Bug 1088850 - Disable http/1 framing enforcement from bug 237623. r=bagder, a=sledru
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 24 Oct 2014 16:19:40 -0400
changeset 218151 b4f797f3cd52
parent 218150 d49ad0a834a8
child 218152 8bb03fe2c2dd
child 218154 e0486ab5ac2c
push id555
push userryanvm@gmail.com
push date2014-10-29 17:07 +0000
treeherdermozilla-release@b4f797f3cd52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder, sledru
bugs1088850, 237623
milestone33.0.2
Bug 1088850 - Disable http/1 framing enforcement from bug 237623. r=bagder, a=sledru
modules/libpref/src/init/all.js
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/test/unit/test_content_length_underrun.js
netwerk/test/unit/test_gzipped_206.js
netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js
toolkit/components/jsdownloads/test/unit/common_test_Download.js
--- a/modules/libpref/src/init/all.js
+++ b/modules/libpref/src/init/all.js
@@ -1163,16 +1163,18 @@ pref("network.http.tcp_keepalive.short_l
 // Max time from initial request during which conns are considered short-lived.
 pref("network.http.tcp_keepalive.short_lived_time", 60);
 // Idle time of TCP connection until first keepalive probe sent.
 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);
+
 // default values for FTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,
 // Section 4.8 "High-Throughput Data Service Class", and 80 (0x50, or AF22)
 // per Section 4.7 "Low-Latency Data Service Class".
 pref("network.ftp.data.qos", 0);
 pref("network.ftp.control.qos", 0);
 
 // </http>
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -200,16 +200,17 @@ nsHttpHandler::nsHttpHandler()
     , mRequestTokenBucketMinParallelism(6)
     , mRequestTokenBucketHz(100)
     , mRequestTokenBucketBurst(32)
     , mTCPKeepaliveShortLivedEnabled(false)
     , mTCPKeepaliveShortLivedTimeS(60)
     , mTCPKeepaliveShortLivedIdleTimeS(10)
     , mTCPKeepaliveLongLivedEnabled(false)
     , mTCPKeepaliveLongLivedIdleTimeS(600)
+    , mEnforceH1Framing(false)
 {
 #if defined(PR_LOGGING)
     gHttpLog = PR_NewLogModule("nsHttp");
 #endif
 
     LOG(("Creating nsHttpHandler [this=%p].\n", this));
 
     RegisterStrongMemoryReporter(new SpdyZlibReporter());
@@ -1451,16 +1452,23 @@ nsHttpHandler::PrefsChanged(nsIPrefBranc
     if (PREF_CHANGED(HTTP_PREF("tcp_keepalive.long_lived_idle_time"))) {
         rv = prefs->GetIntPref(
             HTTP_PREF("tcp_keepalive.long_lived_idle_time"), &val);
         if (NS_SUCCEEDED(rv) && val > 0)
             mTCPKeepaliveLongLivedIdleTimeS = clamped(val,
                                                       1, kMaxTCPKeepIdle);
     }
 
+    if (PREF_CHANGED(HTTP_PREF("enforce-framing.http1"))) {
+        rv = prefs->GetBoolPref(HTTP_PREF("enforce-framing.http1"), &cVar);
+        if (NS_SUCCEEDED(rv)) {
+            mEnforceH1Framing = cVar;
+        }
+    }
+
     // Enable HTTP response timeout if TCP Keepalives are disabled.
     mResponseTimeoutEnabled = !mTCPKeepaliveShortLivedEnabled &&
                               !mTCPKeepaliveLongLivedEnabled;
 
 #undef PREF_CHANGED
 #undef MULTI_PREF_CHANGED
 }
 
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -142,16 +142,19 @@ public:
       return mTCPKeepaliveLongLivedEnabled;
     }
     // Returns time (secs) before first TCP keepalive probes should be sent;
     // same time used between successful keepalive probes.
     int32_t GetTCPKeepaliveLongLivedIdleTime() {
       return mTCPKeepaliveLongLivedIdleTimeS;
     }
 
+    // returns the network.http.enforce-framing.http1 preference
+    bool GetEnforceH1Framing() { return mEnforceH1Framing; }
+
     nsHttpAuthCache     *AuthCache(bool aPrivate) {
         return aPrivate ? &mPrivateAuthCache : &mAuthCache;
     }
     nsHttpConnectionMgr *ConnMgr()   { return mConnMgr; }
 
     // cache support
     bool UseCache() const { return mUseCache; }
     uint32_t GenerateUniqueID() { return ++mLastUniqueID; }
@@ -506,16 +509,20 @@ private:
     // Time (secs) before first keepalive probe; between successful probes.
     int32_t mTCPKeepaliveShortLivedIdleTimeS;
 
     // True if TCP keepalive is enabled for long-lived conns.
     bool mTCPKeepaliveLongLivedEnabled;
     // Time (secs) before first keepalive probe; between successful probes.
     int32_t mTCPKeepaliveLongLivedIdleTimeS;
 
+    // if true, generate NS_ERROR_PARTIAL_TRANSFER for h1 responses with
+    // incorrect content lengths or malformed chunked encodings
+    bool mEnforceH1Framing;
+
 private:
     // For Rate Pacing Certain Network Events. Only assign this pointer on
     // socket thread.
     void MakeNewRequestTokenBucket();
     nsRefPtr<EventTokenBucket> mRequestTokenBucket;
 
 public:
     // Socket thread only
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -875,23 +875,32 @@ nsHttpTransaction::Close(nsresult reason
             gHttpHandler->ConnMgr()->PipelineFeedbackInfo(
                 mConnInfo, nsHttpConnectionMgr::RedCorruptedContent, nullptr, 0);
             if (NS_SUCCEEDED(RestartInProgress()))
                 return;
         }
     }
 
     if ((mChunkedDecoder || (mContentLength >= int64_t(0))) &&
-        (mHttpVersion >= NS_HTTP_VERSION_1_1)) {
+        (NS_SUCCEEDED(reason) && !mResponseIsComplete)) {
+
+        NS_WARNING("Partial transfer, incomplete HTTP response received");
 
-        if (NS_SUCCEEDED(reason) && !mResponseIsComplete) {
+        if ((mHttpVersion >= NS_HTTP_VERSION_1_1) &&
+            gHttpHandler->GetEnforceH1Framing()) {
             reason = NS_ERROR_NET_PARTIAL_TRANSFER;
-            LOG(("Partial transfer, incomplete HTTP responese received: %s",
+            LOG(("Partial transfer, incomplete HTTP response received: %s",
                  mChunkedDecoder ? "broken chunk" : "c-l underrun"));
         }
+
+        if (mConnection) {
+            // whether or not we generate an error for the transaction
+            // bad framing means we don't want a pconn
+            mConnection->DontReuse();
+        }
     }
 
     bool relConn = true;
     if (NS_SUCCEEDED(reason)) {
         if (!mResponseIsComplete) {
             // The response has not been delimited with a high-confidence
             // algorithm like Content-Length or Chunked Encoding. We
             // need to use a strong framing mechanism to pipeline.
--- a/netwerk/test/unit/test_content_length_underrun.js
+++ b/netwerk/test/unit/test_content_length_underrun.js
@@ -11,18 +11,25 @@ XPCOMUtils.defineLazyGetter(this, "URL",
   return "http://localhost:" + httpserver.identity.primaryPort;
 });
 
 var httpserver = new HttpServer();
 var index = 0;
 var test_flags = new Array();
 var testPathBase = "/cl_hdrs";
 
+var prefs;
+var enforcePref;
+
 function run_test()
 {
+  prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+  enforcePref = prefs.getBoolPref("network.http.enforce-framing.http1");
+  prefs.setBoolPref("network.http.enforce-framing.http1", true);
+
   httpserver.start(-1);
 
   do_test_pending();
   run_test_number(1);
 }
 
 function run_test_number(num)
 {
@@ -41,16 +48,17 @@ function setupChannel(url)
                        getService(Ci.nsIIOService);
   var chan = ios.newChannel(URL + url, "", null);
   var httpChan = chan.QueryInterface(Components.interfaces.nsIHttpChannel);
   return httpChan;
 }
 
 function endTests()
 {
+  prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
   httpserver.stop(do_test_finished);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Test 1: FAIL because of Content-Length underrun with HTTP 1.1
 test_flags[1] = CL_EXPECT_LATE_FAILURE;
 
 function handler1(metadata, response)
@@ -89,10 +97,38 @@ function handler2(metadata, response)
   response.write("\r\n");
   response.write(body);
   response.finish();
 }
 
 function completeTest2(request, data, ctx)
 {
   do_check_eq(request.status, Components.results.NS_OK);
+
+  // test 3 requires the pref to be false
+  prefs.setBoolPref("network.http.enforce-framing.http1", false);
+  run_test_number(3);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 3: SUCCEED with bad Content-Length because pref allows it
+test_flags[3] = CL_IGNORE_CL;
+
+function handler3(metadata, response)
+{
+  var body = "blablabla";
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n");
+  response.write("Content-Type: text/plain\r\n");
+  response.write("Content-Length: 556677\r\n");
+  response.write("\r\n");
+  response.write(body);
+  response.finish();
+}
+
+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();
 }
--- a/netwerk/test/unit/test_gzipped_206.js
+++ b/netwerk/test/unit/test_gzipped_206.js
@@ -1,9 +1,10 @@
 Cu.import("resource://testing-common/httpd.js");
+Cu.import("resource://gre/modules/Services.jsm");
 
 var httpserver = null;
 
 // testString = "This is a slightly longer test\n";
 const responseBody = [0x1f, 0x8b, 0x08, 0x08, 0xef, 0x70, 0xe6, 0x4c, 0x00, 0x03, 0x74, 0x65, 0x78, 0x74, 0x66, 0x69,
                      0x6c, 0x65, 0x2e, 0x74, 0x78, 0x74, 0x00, 0x0b, 0xc9, 0xc8, 0x2c, 0x56, 0x00, 0xa2, 0x44, 0x85,
                      0xe2, 0x9c, 0xcc, 0xf4, 0x8c, 0x92, 0x9c, 0x4a, 0x85, 0x9c, 0xfc, 0xbc, 0xf4, 0xd4, 0x22, 0x85,
                      0x92, 0xd4, 0xe2, 0x12, 0x2e, 0x2e, 0x00, 0x00, 0xe5, 0xe6, 0xf0, 0x20, 0x00, 0x00, 0x00];
@@ -58,30 +59,36 @@ function cachedHandler(metadata, respons
 
 function continue_test(request, data) {
   do_check_eq(17, data.length);
   var chan = make_channel("http://localhost:" +
                           httpserver.identity.primaryPort + "/cached/test.gz");
   chan.asyncOpen(new ChannelListener(finish_test, null, CL_EXPECT_GZIP), null);
 }
 
+var enforcePref;
+
 function finish_test(request, data, ctx) {
   do_check_eq(request.status, 0);
   do_check_eq(data.length, responseBody.length);
   for (var i = 0; i < data.length; ++i) {
     do_check_eq(data.charCodeAt(i), responseBody[i]);
   }
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
   httpserver.stop(do_test_finished);
 }
 
 function run_test() {
+  enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.http1");
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", false);
+
   httpserver = new HttpServer();
   httpserver.registerPathHandler("/cached/test.gz", cachedHandler);
   httpserver.start(-1);
 
   // wipe out cached content
   evict_cache_entries();
 
   var chan = make_channel("http://localhost:" +
                           httpserver.identity.primaryPort + "/cached/test.gz");
-  chan.asyncOpen(new ChannelListener(continue_test, null, CL_EXPECT_GZIP|CL_EXPECT_LATE_FAILURE), null);
+  chan.asyncOpen(new ChannelListener(continue_test, null, CL_EXPECT_GZIP | CL_IGNORE_CL), null);
   do_test_pending();
 }
--- a/netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js
+++ b/netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js
@@ -47,33 +47,39 @@ function contentHandler(metadata, respon
       (responseBody.length - 1).toString() + "/" +
       (responseBody.length).toString());
 
     response.setHeader("Content-Length", slice.length + "");
     response.bodyOutputStream.write(slice, slice.length);
   }
 }
 
+var enforcePref;
+
 function run_test()
 {
+  enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.http1");
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", false);
+
   httpServer = new HttpServer();
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
   var chan = make_channel(URL + "/content");
-  chan.asyncOpen(new ChannelListener(firstTimeThrough, null, CL_EXPECT_LATE_FAILURE), null);
+  chan.asyncOpen(new ChannelListener(firstTimeThrough, null, CL_IGNORE_CL), null);
   do_test_pending();
 }
 
 function firstTimeThrough(request, buffer)
 {
   // Change single cache entry limit to 1 kb.  This emulates smart size change.
   Services.prefs.setIntPref("browser.cache.disk.max_entry_size", 1);
 
   var chan = make_channel(URL + "/content");
   chan.asyncOpen(new ChannelListener(finish_test, null), null);
 }
 
 function finish_test(request, buffer)
 {
   do_check_eq(buffer, responseBody);
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
   httpServer.stop(do_test_finished);
 }
--- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js
+++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js
@@ -1089,16 +1089,24 @@ add_task(function test_error_source()
 /**
  * Ensures a download error is reported when receiving less bytes than what was
  * specified in the Content-Length header.
  */
 add_task(function test_error_source_partial()
 {
   let sourceUrl = httpUrl("shorter-than-content-length-http-1-1.txt");
 
+  let enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.http1");
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", true);
+
+  function cleanup() {
+    Services.prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
+  }
+  do_register_cleanup(cleanup);
+
   let download;
   try {
     if (!gUseLegacySaver) {
       // When testing DownloadCopySaver, we want to check that the promise
       // returned by the "start" method is rejected.
       download = yield promiseNewDownload(sourceUrl);
 
       do_check_true(download.error === null);