Bug 237623 - introduce "soft" checks for HTTP framing errors, r=mcmanus

--HG--
extra : rebase_source : a920948bc393de14a17d903c4604fd4c61356293

modules/libpref/init/all.js

@@ -1362,7 +1362,8 @@ pref("network.http.tcp_keepalive.short_lived_idle_time", 10);
 pref("network.http.tcp_keepalive.long_lived_connections", true);
 pref("network.http.tcp_keepalive.long_lived_idle_time", 600);
 
-pref("network.http.enforce-framing.http1", false);
+pref("network.http.enforce-framing.http1", false); // should be named "strict"
+pref("network.http.enforce-framing.soft", true);
 
 // default values for FTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,

netwerk/protocol/http/nsHttpHandler.cpp

@@ -208,7 +208,7 @@ nsHttpHandler::nsHttpHandler()
     , mTCPKeepaliveShortLivedIdleTimeS(10)
     , mTCPKeepaliveLongLivedEnabled(false)
     , mTCPKeepaliveLongLivedIdleTimeS(600)
-    , mEnforceH1Framing(false)
+    , mEnforceH1Framing(FRAMECHECK_BARELY)
 {
 #if defined(PR_LOGGING)
     gHttpLog = PR_NewLogModule("nsHttp");
@@ -1524,10 +1524,18 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
                                                       1, kMaxTCPKeepIdle);
     }
 
-    if (PREF_CHANGED(HTTP_PREF("enforce-framing.http1"))) {
+    if (PREF_CHANGED(HTTP_PREF("enforce-framing.http1")) ||
+        PREF_CHANGED(HTTP_PREF("enforce-framing.soft")) ) {
         rv = prefs->GetBoolPref(HTTP_PREF("enforce-framing.http1"), &cVar);
-        if (NS_SUCCEEDED(rv)) {
-            mEnforceH1Framing = cVar;
+        if (NS_SUCCEEDED(rv) && cVar) {
+            mEnforceH1Framing = FRAMECHECK_STRICT;
+        } else {
+            rv = prefs->GetBoolPref(HTTP_PREF("enforce-framing.soft"), &cVar);
+            if (NS_SUCCEEDED(rv) && cVar) {
+                mEnforceH1Framing = FRAMECHECK_BARELY;
+            } else {
+                mEnforceH1Framing = FRAMECHECK_LAX;
+            }
         }
     }
 

netwerk/protocol/http/nsHttpHandler.h

@@ -39,6 +39,12 @@ class nsHttpConnectionInfo;
 class nsHttpTransaction;
 class AltSvcMapping;
 
+enum FrameCheckLevel {
+    FRAMECHECK_LAX,
+    FRAMECHECK_BARELY,
+    FRAMECHECK_STRICT
+};
+
 //-----------------------------------------------------------------------------
 // nsHttpHandler - protocol handler for HTTP and HTTPS
 //-----------------------------------------------------------------------------
@@ -151,8 +157,9 @@ public:
       return mTCPKeepaliveLongLivedIdleTimeS;
     }
 
-    // returns the network.http.enforce-framing.http1 preference
-    bool GetEnforceH1Framing() { return mEnforceH1Framing; }
+    // returns the HTTP framing check level preference, as controlled with
+    // network.http.enforce-framing.http1 and network.http.enforce-framing.soft
+    FrameCheckLevel GetEnforceH1Framing() { return mEnforceH1Framing; }
 
     nsHttpAuthCache     *AuthCache(bool aPrivate) {
         return aPrivate ? &mPrivateAuthCache : &mAuthCache;
@@ -531,7 +538,7 @@ private:
 
     // if true, generate NS_ERROR_PARTIAL_TRANSFER for h1 responses with
     // incorrect content lengths or malformed chunked encodings
-    bool mEnforceH1Framing;
+    FrameCheckLevel mEnforceH1Framing;
 
 private:
     // For Rate Pacing Certain Network Events. Only assign this pointer on

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -125,6 +125,8 @@ nsHttpTransaction::nsHttpTransaction()
     , mResponseTimeoutEnabled(true)
     , mForceRestart(false)
     , mReuseOnRestart(false)
+    , mContentDecoding(false)
+    , mContentDecodingCheck(false)
     , mReportedStart(false)
     , mReportedResponseHeader(false)
     , mForTakeResponseHead(nullptr)
@@ -909,11 +911,17 @@ nsHttpTransaction::Close(nsresult reason)
 
         NS_WARNING("Partial transfer, incomplete HTTP response received");
 
-        if ((mHttpVersion >= NS_HTTP_VERSION_1_1) &&
-            gHttpHandler->GetEnforceH1Framing()) {
-            reason = NS_ERROR_NET_PARTIAL_TRANSFER;
-            LOG(("Partial transfer, incomplete HTTP response received: %s",
-                 mChunkedDecoder ? "broken chunk" : "c-l underrun"));
+        if (mHttpVersion >= NS_HTTP_VERSION_1_1) {
+            FrameCheckLevel clevel = gHttpHandler->GetEnforceH1Framing();
+            if (clevel >= FRAMECHECK_BARELY) {
+                if ((clevel == FRAMECHECK_STRICT) ||
+                    (mChunkedDecoder && mChunkedDecoder->GetChunkRemaining()) ||
+                    (!mChunkedDecoder && !mContentDecoding && mContentDecodingCheck) ) {
+                    reason = NS_ERROR_NET_PARTIAL_TRANSFER;
+                    LOG(("Partial transfer, incomplete HTTP response received: %s",
+                         mChunkedDecoder ? "broken chunk" : "c-l underrun"));
+                }
+            }
         }
 
         if (mConnection) {
@@ -1736,6 +1744,12 @@ nsHttpTransaction::ProcessData(char *buf, uint32_t count, uint32_t *countRead)
             MOZ_ASSERT(mConnection);
             mConnection->PushBack(buf + *countRead, countRemaining);
         }
+
+        if (!mContentDecodingCheck && mResponseHead) {
+            mContentDecoding =
+                !!mResponseHead->PeekHeader(nsHttp::Content_Encoding);
+            mContentDecodingCheck = true;
+        }
     }
 
     return NS_OK;

netwerk/protocol/http/nsHttpTransaction.h

@@ -277,6 +277,8 @@ private:
     bool                            mResponseTimeoutEnabled;
     bool                            mForceRestart;
     bool                            mReuseOnRestart;
+    bool                            mContentDecoding;
+    bool                            mContentDecodingCheck;
 
     // mClosed           := transaction has been explicitly closed
     // mTransactionDone  := transaction ran to completion or was interrupted

netwerk/streamconv/converters/nsHTTPCompressConv.cpp

@@ -12,6 +12,9 @@
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsComponentManagerUtils.h"
+#include "nsThreadUtils.h"
+#include "mozilla/Preferences.h"
+
 
 // nsISupports implementation
 NS_IMPL_ISUPPORTS(nsHTTPCompressConv,
@@ -35,6 +38,13 @@ nsHTTPCompressConv::nsHTTPCompressConv()
     , mSkipCount(0)
     , mFlags(0)
 {
+    if (NS_IsMainThread()) {
+        mFailUncleanStops =
+            (Preferences::GetBool("network.http.enforce-framing.soft", false) ||
+             Preferences::GetBool("network.http.enforce-framing.http", false));
+    } else {
+        mFailUncleanStops = false;
+    }
 }
 
 nsHTTPCompressConv::~nsHTTPCompressConv()
@@ -88,6 +98,14 @@ NS_IMETHODIMP
 nsHTTPCompressConv::OnStopRequest(nsIRequest* request, nsISupports *aContext, 
                                   nsresult aStatus)
 {
+    // Framing integrity is enforced for content-encoding: gzip, but not for
+    // content-encoding: deflate. Note that gzip vs deflate is NOT determined
+    // by content sniffing but only via header.
+    if (!mStreamEnded && NS_SUCCEEDED(aStatus) &&
+        (mFailUncleanStops && (mMode == HTTP_COMPRESS_GZIP)) ) {
+        // This is not a clean end of gzip stream: the transfer is incomplete.
+        aStatus = NS_ERROR_NET_PARTIAL_TRANSFER;
+    }
     return mListener->OnStopRequest(request, aContext, aStatus);
 } 
 

netwerk/streamconv/converters/nsHTTPCompressConv.h

@@ -76,6 +76,7 @@ private:
     bool        mStreamEnded;
     bool        mStreamInitialized;
     bool        mDummyStreamInitialised;
+    bool        mFailUncleanStops;
 
     z_stream d_stream;
     unsigned mLen, hMode, mSkipCount, mFlags;

netwerk/test/unit/test_content_length_underrun.js

@@ -18,12 +18,14 @@ var test_flags = new Array();
 var testPathBase = "/cl_hdrs";
 
 var prefs;
-var enforcePref;
+var enforcePrefStrict;
+var enforcePrefSoft;
 
 function run_test()
 {
   prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
-  enforcePref = prefs.getBoolPref("network.http.enforce-framing.http1");
+  enforcePrefStrict = prefs.getBoolPref("network.http.enforce-framing.http1");
+  enforcePrefSoft = prefs.getBoolPref("network.http.enforce-framing.soft");
   prefs.setBoolPref("network.http.enforce-framing.http1", true);
 
   httpserver.start(-1);
@@ -43,6 +45,42 @@ function run_test_number(num)
                                         channel, flags), null);
 }
 
+function run_gzip_test(num)
+{
+  testPath = testPathBase + num;
+  httpserver.registerPathHandler(testPath, eval("handler" + num));
+
+  var channel = setupChannel(testPath);
+
+  function StreamListener() {}
+
+  StreamListener.prototype = {
+    QueryInterface: function(aIID) {
+      if (aIID.equals(Components.interfaces.nsIStreamListener) ||
+          aIID.equals(Components.interfaces.nsIRequestObserver) ||
+          aIID.equals(Components.interfaces.nsISupports))
+        return this;
+      throw Components.results.NS_ERROR_NO_INTERFACE;
+    },
+
+    onStartRequest: function(aRequest, aContext) {},
+
+    onStopRequest: function(aRequest, aContext, aStatusCode) {
+      // Make sure we catch the error NS_ERROR_NET_PARTIAL_TRANSFER here.
+      do_check_eq(aStatusCode, Components.results.NS_ERROR_NET_PARTIAL_TRANSFER);
+      //  do_test_finished();
+        endTests();
+    },
+
+    onDataAvailable: function(request, context, stream, offset, count) {}
+  };
+
+  let listener = new StreamListener();
+ 
+  channel.asyncOpen(listener, null);
+
+}
+
 function setupChannel(url)
 {
   var ios = Components.classes["@mozilla.org/network/io-service;1"].
@@ -61,7 +99,9 @@ function setupChannel(url)
 
 function endTests()
 {
-  prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
+  // restore the prefs to pre-test values
+  prefs.setBoolPref("network.http.enforce-framing.http1", enforcePrefStrict);
+  prefs.setBoolPref("network.http.enforce-framing.soft", enforcePrefSoft);
   httpserver.stop(do_test_finished);
 }
 
@@ -111,8 +151,9 @@ function completeTest2(request, data, ctx)
 {
   do_check_eq(request.status, Components.results.NS_OK);
 
-  // test 3 requires the pref to be false
+  // test 3 requires the enforce-framing prefs to be false
   prefs.setBoolPref("network.http.enforce-framing.http1", false);
+  prefs.setBoolPref("network.http.enforce-framing.soft", false);
   run_test_number(3);
 }
 
@@ -135,8 +176,84 @@ function handler3(metadata, response)
 
 function completeTest3(request, data, ctx)
 {
-  // reset the pref in case we add more tests
-  prefs.setBoolPref("network.http.enforce-framing.http1", true);
   do_check_eq(request.status, Components.results.NS_OK);
-  endTests();
+  prefs.setBoolPref("network.http.enforce-framing.soft", true);
+  run_test_number(4);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 4: Succeed because a cut off deflate stream can't be detected
+test_flags[4] = CL_IGNORE_CL;
+
+function handler4(metadata, response)
+{
+    // this is the beginning of a deflate compressed response body
+    
+  var body = "\xcd\x57\xcd\x6e\x1b\x37\x10\xbe\x07\xc8\x3b\x0c\x36\x68\x72\xd1" +
+    "\xbf\x92\x22\xb1\x57\x0a\x64\x4b\x6a\x0c\x28\xb6\x61\xa9\x41\x73" +
+    "\x2a\xb8\xbb\x94\x44\x98\xfb\x03\x92\x92\xec\x06\x7d\x97\x1e\xeb" +
+    "\xbe\x86\x5e\xac\xc3\x25\x97\xa2\x64\xb9\x75\x0b\x14\xe8\x69\x87" +
+    "\x33\x9c\x1f\x7e\x33\x9c\xe1\x86\x9f\x66\x9f\x27\xfd\x97\x2f\x20" +
+    "\xfc\x34\x1a\x0c\x35\x01\xa1\x62\x8a\xd3\xfe\xf5\xcd\xd5\xe5\xd5" +
+    "\x6c\x54\x83\x49\xbe\x60\x31\xa3\x1c\x12\x0a\x0b\x2a\x15\xcb\x33" +
+    "\x4d\xae\x19\x05\x19\xe7\x9c\x30\x41\x1b\x61\xd3\x28\x95\xfa\x29" +
+    "\x55\x04\x32\x92\xd2\x5e\x90\x50\x19\x0b\x56\x68\x9d\x00\xe2\x3c" +
+    "\x53\x34\x53\xbd\xc0\x99\x56\xf9\x4a\x51\xe0\x64\xcf\x18\x24\x24" +
+    "\x93\xb0\xca\x40\xd2\x15\x07\x6e\xbd\x37\x60\x82\x3b\x8f\x86\x22" +
+    "\x21\xcb\x15\x95\x35\x20\x91\xa4\x59\xac\xa9\x62\x95\x31\xed\x14" +
+    "\xc9\x98\x2c\x19\x15\x3a\x62\x45\xef\x70\x1b\x50\x05\xa4\x28\xc4" +
+    "\xf6\x21\x66\xa4\xdc\x83\x32\x09\x85\xc8\xe7\x54\xa2\x4b\x81\x74" +
+    "\xbe\x12\xc0\x91\xb9\x7d\x50\x24\xe2\x0c\xd9\x29\x06\x2e\xdd\x79";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Content-Length: 553677\r\n");
+  response.write("Content-Encoding: deflate\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+function completeTest4(request, data, ctx)
+{
+  do_check_eq(request.status, Components.results.NS_OK);
+  run_gzip_test(99);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 99: FAIL because a cut off gzip stream CAN be detected
+
+// Note that test 99 here is run completely different than the other tests in
+// this file so if you add more tests here, consider adding them before this.
+
+function handler99(metadata, response)
+{
+  // this is the beginning of a gzip compressed response body
+    
+  var body = "\x1f\x8b\x08\x00\x80\xb9\x25\x53\x00\x03\xd4\xd9\x79\xb8\x8e\xe5" +
+    "\xba\x00\xf0\x65\x19\x33\x24\x15\x29\xf3\x50\x52\xc6\xac\x85\x10" +
+    "\x8b\x12\x22\x45\xe6\xb6\x21\x9a\x96\x84\x4c\x69\x32\xec\x84\x92" +
+    "\xcc\x99\x6a\xd9\x32\xa5\xd0\x40\xd9\xc6\x14\x15\x95\x28\x62\x9b" +
+    "\x09\xc9\x70\x4a\x25\x53\xec\x8e\x9c\xe5\x1c\x9d\xeb\xfe\x9d\x73" +
+    "\x9d\x3f\xf6\x1f\xe7\xbd\xae\xcf\xf3\xbd\xbf\xef\x7e\x9f\xeb\x79" +
+    "\xef\xf7\x99\xde\xe5\xee\x6e\xdd\x3b\x75\xeb\xd1\xb5\x6c\xb3\xd4" +
+    "\x47\x1f\x48\xf8\x17\x1d\x15\xce\x1d\x55\x92\x93\xcf\x97\xe7\x8e" +
+    "\x8b\xca\xe4\xca\x55\x92\x2a\x54\x4e\x4e\x4e\x4a\xa8\x78\x53\xa5" +
+    "\x8a\x15\x2b\x55\x4a\xfa\xe3\x7b\x85\x8a\x37\x55\x48\xae\x92\x50" +
+    "\xb4\xc2\xbf\xaa\x41\x17\x1f\xbd\x7b\xf6\xba\xaf\x47\xd1\xa2\x09" +
+    "\x3d\xba\x75\xeb\xf5\x3f\xc5\xfd\x6f\xbf\xff\x3f\x3d\xfa\xd7\x6d" +
+    "\x74\x7b\x62\x86\x0c\xff\x79\x9e\x98\x50\x33\xe1\x8f\xb3\x01\xef" +
+    "\xb6\x38\x7f\x9e\x92\xee\xf9\xa7\xee\xcb\x74\x21\x26\x25\xa1\x6a" +
+    "\x42\xf6\x73\xff\x96\x4c\x28\x91\x90\xe5\xdc\x79\xa6\x8b\xe2\x52" +
+    "\xd2\xbf\x5d\x28\x2b\x24\x26\xfc\xa9\xcc\x96\x1e\x97\x31\xfd\xba" +
+    "\xee\xe9\xde\x3d\x31\xe5\x4f\x65\xc1\xf4\xb8\x0b\x65\x86\x8b\xca";
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Content-Length: 553677\r\n");
+  response.write("Content-Encoding: gzip\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
 }

netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js

@@ -63,8 +63,8 @@ var enforcePref;
 
 function run_test()
 {
-  enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.http1");
-  Services.prefs.setBoolPref("network.http.enforce-framing.http1", false);
+  enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.soft");
+  Services.prefs.setBoolPref("network.http.enforce-framing.soft", false);
 
   httpServer = new HttpServer();
   httpServer.registerPathHandler("/content", contentHandler);
@@ -87,6 +87,6 @@ function firstTimeThrough(request, buffer)
 function finish_test(request, buffer)
 {
   do_check_eq(buffer, responseBody);
-  Services.prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
+  Services.prefs.setBoolPref("network.http.enforce-framing.soft", enforcePref);
   httpServer.stop(do_test_finished);
 }