diff --git a/src/extensions/proxy/proxyconn.cpp b/src/extensions/proxy/proxyconn.cpp index c572a0ee..548690ae 100644 --- a/src/extensions/proxy/proxyconn.cpp +++ b/src/extensions/proxy/proxyconn.cpp @@ -269,23 +269,17 @@ int ProxyConn::sendReqHeader() pReq->dropReqHeader(HttpHeader::H_TRANSFER_ENCODING); } -#if 1 //always set "Accept-Encoding" header to "gzip" + //Forward the client's own "Accept-Encoding" header to the backend + //as-is, so encodings other than gzip (e.g. "br") aren't clobbered. + //If the client sent none, add "gzip" so the backend<->OLS leg can + //still be compressed; OLS decompresses it before replying to a + //client that didn't ask for gzip (see HttpSession::setupGzipFilter). char *pAE = (char *)pReq->getHeader(HttpHeader::H_ACC_ENCODING); - if (*pAE) - { - int len = pReq->getHeaderLen(HttpHeader::H_ACC_ENCODING); - if (len >= 4) - { - memmove(pAE, "gzip", 4); - memset(pAE + 4, ' ', len - 4); - } - } - else // If accept encoding header does not exist, use predefined. + if (!*pAE) { pExtraHeader = m_extraHeader; headerLen += 23; } -#endif //reconstruct request line if URL has been rewritten if (pReq->getRedirects() > 0) diff --git a/src/http/httpcgitool.cpp b/src/http/httpcgitool.cpp index c110de41..74d30af3 100644 --- a/src/http/httpcgitool.cpp +++ b/src/http/httpcgitool.cpp @@ -108,6 +108,22 @@ int HttpCgiTool::processContentType(HttpSession *pSession, } +int HttpCgiTool::parseContentEncoding(const char *pValue, int valLen) +{ + if (!pValue || valLen <= 0) + return UPSTREAM_ENCODING_OTHER; + if (valLen >= 4 && strncasecmp(pValue, "none", 4) == 0) + return UPSTREAM_ENCODING_NONE; + if (valLen >= 4 && strncasecmp(pValue, "gzip", 4) == 0) + return UPSTREAM_ENCODING_GZIP; + if (valLen >= 7 && strncasecmp(pValue, "deflate", 7) == 0) + return UPSTREAM_ENCODING_DEFLATE; + if (valLen >= 2 && strncasecmp(pValue, "br", 2) == 0) + return UPSTREAM_ENCODING_BR; + return UPSTREAM_ENCODING_OTHER; +} + + int HttpCgiTool::processHeaderLine(HttpExtConnector *pExtConn, const char *pName, int nameLen, const char *pValue, int valLen) @@ -201,20 +217,24 @@ int HttpCgiTool::processHeaderLine2(HttpExtConnector *pExtConn, //HttpCgiTool::processExpires(pReq, pResp, pValue); return processContentType(pExtConn->getHttpSession(), pValue, valLen); case HttpRespHeaders::H_CONTENT_ENCODING: - if (pReq->getStatusCode() == SC_304 - || (valLen >= 4 && strncasecmp(pValue, "none", 4) == 0)) + if (pReq->getStatusCode() == SC_304) + return 0; + switch (parseContentEncoding(pValue, valLen)) + { + case UPSTREAM_ENCODING_NONE: return 0; - if (valLen >= 4 && strncasecmp(pValue, "gzip", 4) == 0) + case UPSTREAM_ENCODING_GZIP: pReq->orGzip(UPSTREAM_GZIP); - else if (valLen >= 7 && strncasecmp(pValue, "deflate", 7) == 0) + break; + case UPSTREAM_ENCODING_DEFLATE: pReq->orGzip(UPSTREAM_DEFLATE); -// if ( !(pReq->gzipAcceptable() & REQ_GZIP_ACCEPT) ) -// return 0; -// } -// else //if ( strncasecmp( pValue, "deflate", 7 ) == 0 ) -// { -// pReq->andGzip( ~GZIP_ENABLED ); -// } + break; + case UPSTREAM_ENCODING_BR: + pReq->orBr(UPSTREAM_BR); + break; + default: + break; + } break; case HttpRespHeaders::H_CONTENT_DISPOSITION: pReq->appendRedirHdr(pName, pValue + valLen - pName); diff --git a/src/http/httpcgitool.h b/src/http/httpcgitool.h index d4d615bd..4fdef003 100644 --- a/src/http/httpcgitool.h +++ b/src/http/httpcgitool.h @@ -27,6 +27,15 @@ class FcgiEnv; class Env; class IEnv; +enum +{ + UPSTREAM_ENCODING_OTHER = 0, + UPSTREAM_ENCODING_NONE, + UPSTREAM_ENCODING_GZIP, + UPSTREAM_ENCODING_DEFLATE, + UPSTREAM_ENCODING_BR, +}; + class HttpCgiTool { HttpCgiTool() {}; @@ -37,6 +46,7 @@ class HttpCgiTool static int processContentType(HttpSession *pSession, const char *pValue, int valLen); static int processExpires(HttpReq *pReq, HttpResp *pResp, const char *pValue); + static int parseContentEncoding(const char *pValue, int valLen); static int processHeaderLine(HttpExtConnector *pExtConn, const char *pName, int nameLen, diff --git a/src/http/httpsession.cpp b/src/http/httpsession.cpp index 8e39a700..bf5fe31a 100644 --- a/src/http/httpsession.cpp +++ b/src/http/httpsession.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -3963,6 +3964,23 @@ int HttpSession::setupGzipFilter() int hkptNogzip = (m_sessionHooks.getFlag(LSI_HKPT_RECV_RESP_BODY) | m_sessionHooks.getFlag(LSI_HKPT_SEND_RESP_BODY)) & LSI_FLAG_DECOMPRESS_REQUIRED; + + if (m_request.brAcceptable() & UPSTREAM_BR) + { + //The body is already brotli encoded. Gzipping it again would emit + //"Content-Encoding: br,gzip" and a body no client can decode, and + //there is no brotli decoder on the response filter path to undo it + //with, so leave the body and its Content-Encoding alone. + //Recording it as a brotli buffer also keeps the cache module from + //re-compressing it and storing the entry as gzip. + setFlag(HSF_RESP_BODY_BRCOMPRESSED); + clearFlag(HSF_RESP_BODY_GZIPCOMPRESSED); + if (!(m_request.brAcceptable() & REQ_BR_ACCEPT)) + LS_DBG_L(getLogSession(), "Upstream returned a brotli encoded " + "body that the client did not ask for, passing through."); + return 0; + } + if (gz & (UPSTREAM_GZIP | UPSTREAM_DEFLATE)) { setFlag(HSF_RESP_BODY_GZIPCOMPRESSED); @@ -5843,14 +5861,19 @@ int HttpSession::updateContentCompressible() int HttpSession::contentEncodingFixup() { - int len; + int len = 0; int requireChunk = 0; const char *pContentEncoding = m_response.getRespHeaders().getHeader( HttpRespHeaders::H_CONTENT_ENCODING, &len); if ((!(m_request.gzipAcceptable() & REQ_GZIP_ACCEPT)) && (!(m_request.brAcceptable() & REQ_BR_ACCEPT))) { - if (pContentEncoding) + //modgzip's decompress filter is zlib only, so it can only undo gzip + //and deflate. Running it over brotli (or any other encoding the + //backend picked) would corrupt the body instead of decoding it. + int encoding = HttpCgiTool::parseContentEncoding(pContentEncoding, len); + if (encoding == UPSTREAM_ENCODING_GZIP + || encoding == UPSTREAM_ENCODING_DEFLATE) { if (addModgzipFilter(1, 0) == -1) return LS_FAIL; diff --git a/test/http/httpcgitooltest.cpp b/test/http/httpcgitooltest.cpp index ce819638..5cb91744 100644 --- a/test/http/httpcgitooltest.cpp +++ b/test/http/httpcgitooltest.cpp @@ -17,6 +17,49 @@ *****************************************************************************/ #ifdef RUN_TEST +#include "httpcgitooltest.h" + +#include +#include "unittest-cpp/UnitTest++.h" +#include + +SUITE(HttpCgiToolTest) +{ + TEST(HttpCgiToolTest_parseContentEncoding) + { + CHECK(UPSTREAM_ENCODING_GZIP + == HttpCgiTool::parseContentEncoding("gzip", 4)); + CHECK(UPSTREAM_ENCODING_GZIP + == HttpCgiTool::parseContentEncoding("GZip", 4)); + CHECK(UPSTREAM_ENCODING_DEFLATE + == HttpCgiTool::parseContentEncoding("deflate", 7)); + CHECK(UPSTREAM_ENCODING_NONE + == HttpCgiTool::parseContentEncoding("none", 4)); + + CHECK(UPSTREAM_ENCODING_BR + == HttpCgiTool::parseContentEncoding("br", 2)); + CHECK(UPSTREAM_ENCODING_BR + == HttpCgiTool::parseContentEncoding("BR", 2)); + + //anything the response filters cannot decode must not be mistaken + //for gzip or deflate + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding("zstd", 4)); + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding("identity", 8)); + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding("", 0)); + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding(NULL, 4)); + + //a truncated value must not match a longer token + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding("gzi", 3)); + CHECK(UPSTREAM_ENCODING_OTHER + == HttpCgiTool::parseContentEncoding("b", 1)); + } +} + // #include "httpcgitooltest.h" // #include // #include