Forward the client's Accept-Encoding to the proxy backend, and handle a brotli response (#265)

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)

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);

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,

src/http/httpsession.cpp

@@ -32,6 +32,7 @@
 #include <http/handlertype.h>
 #include <http/hiochainstream.h>
 #include <http/htauth.h>
+#include <http/httpcgitool.h>
 #include <http/httphandler.h>
 #include <http/httplog.h>
 #include <http/httpmethod.h>
@@ -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;

test/http/httpcgitooltest.cpp

@@ -17,6 +17,49 @@
 *****************************************************************************/
 #ifdef RUN_TEST
 
+#include "httpcgitooltest.h"
+
+#include <http/httpcgitool.h>
+#include "unittest-cpp/UnitTest++.h"
+#include <string.h>
+
+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 <http/httpsession.h>
 // #include <http/httpextconnector.h>