bug 1133156 backout 7abb1cb9739e (bug 237623) for regression r=backout
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 13 Feb 2015 22:57:03 -0500
changeset 229143 008aa6494f9002a026f01fac2c81038c4bd56266
parent 229142 4158bdaf7894be26a44e44aa29f99fd25cc14250
child 229144 edd04b167f471ad8c3374bbed2d1ed1d45404088
push id55602
push usermcmanus@ducksong.com
push dateSat, 14 Feb 2015 03:57:18 +0000
treeherdermozilla-inbound@008aa6494f90 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1133156, 237623
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1133156 backout 7abb1cb9739e (bug 237623) for regression r=backout
modules/libpref/init/all.js
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/protocol/http/nsHttpTransaction.h
netwerk/streamconv/converters/nsHTTPCompressConv.cpp
netwerk/streamconv/converters/nsHTTPCompressConv.h
netwerk/test/unit/test_content_length_underrun.js
netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1345,18 +1345,17 @@ 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); // should be named "strict"
-pref("network.http.enforce-framing.soft", true);
+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);
 
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -203,17 +203,17 @@ nsHttpHandler::nsHttpHandler()
     , mRequestTokenBucketMinParallelism(6)
     , mRequestTokenBucketHz(100)
     , mRequestTokenBucketBurst(32)
     , mTCPKeepaliveShortLivedEnabled(false)
     , mTCPKeepaliveShortLivedTimeS(60)
     , mTCPKeepaliveShortLivedIdleTimeS(10)
     , mTCPKeepaliveLongLivedEnabled(false)
     , mTCPKeepaliveLongLivedIdleTimeS(600)
-    , mEnforceH1Framing(FRAMECHECK_BARELY)
+    , mEnforceH1Framing(false)
 {
 #if defined(PR_LOGGING)
     gHttpLog = PR_NewLogModule("nsHttp");
 #endif
 
     LOG(("Creating nsHttpHandler [this=%p].\n", this));
 
     RegisterStrongMemoryReporter(new SpdyZlibReporter());
@@ -1519,28 +1519,20 @@ 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")) ||
-        PREF_CHANGED(HTTP_PREF("enforce-framing.soft")) ) {
+    if (PREF_CHANGED(HTTP_PREF("enforce-framing.http1"))) {
         rv = prefs->GetBoolPref(HTTP_PREF("enforce-framing.http1"), &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;
-            }
+        if (NS_SUCCEEDED(rv)) {
+            mEnforceH1Framing = cVar;
         }
     }
 
     // Enable HTTP response timeout if TCP Keepalives are disabled.
     mResponseTimeoutEnabled = !mTCPKeepaliveShortLivedEnabled &&
                               !mTCPKeepaliveLongLivedEnabled;
 
 #undef PREF_CHANGED
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -34,22 +34,16 @@ namespace net {
 class ATokenBucketEvent;
 class EventTokenBucket;
 class Tickler;
 class nsHttpConnection;
 class nsHttpConnectionInfo;
 class nsHttpTransaction;
 class AltSvcMapping;
 
-enum FrameCheckLevel {
-    FRAMECHECK_LAX,
-    FRAMECHECK_BARELY,
-    FRAMECHECK_STRICT
-};
-
 //-----------------------------------------------------------------------------
 // nsHttpHandler - protocol handler for HTTP and HTTPS
 //-----------------------------------------------------------------------------
 
 class nsHttpHandler MOZ_FINAL : public nsIHttpProtocolHandler
                               , public nsIObserver
                               , public nsSupportsWeakReference
                               , public nsISpeculativeConnect
@@ -152,19 +146,18 @@ 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 HTTP framing check level preference, as controlled with
-    // network.http.enforce-framing.http1 and network.http.enforce-framing.soft
-    FrameCheckLevel GetEnforceH1Framing() { return mEnforceH1Framing; }
+    // 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; }
@@ -533,17 +526,17 @@ private:
 
     // 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
-    FrameCheckLevel mEnforceH1Framing;
+    bool mEnforceH1Framing;
 
 private:
     // For Rate Pacing Certain Network Events. Only assign this pointer on
     // socket thread.
     void MakeNewRequestTokenBucket();
     nsRefPtr<EventTokenBucket> mRequestTokenBucket;
 
 public:
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -121,18 +121,16 @@ nsHttpTransaction::nsHttpTransaction()
     , mProxyConnectFailed(false)
     , mHttpResponseMatched(false)
     , mPreserveStream(false)
     , mDispatchedAsBlocking(false)
     , mResponseTimeoutEnabled(true)
     , mDontRouteViaWildCard(false)
     , mForceRestart(false)
     , mReuseOnRestart(false)
-    , mContentDecoding(false)
-    , mContentDecodingCheck(false)
     , mReportedStart(false)
     , mReportedResponseHeader(false)
     , mForTakeResponseHead(nullptr)
     , mResponseHeadTaken(false)
     , mSubmittedRatePacing(false)
     , mPassedRatePacing(false)
     , mSynchronousRatePaceRequest(false)
     , mCountRecv(0)
@@ -906,27 +904,21 @@ nsHttpTransaction::Close(nsresult reason
         }
     }
 
     if ((mChunkedDecoder || (mContentLength >= int64_t(0))) &&
         (NS_SUCCEEDED(reason) && !mResponseIsComplete)) {
 
         NS_WARNING("Partial transfer, incomplete HTTP response received");
 
-        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 ((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 (mConnection) {
             // whether or not we generate an error for the transaction
             // bad framing means we don't want a pconn
             mConnection->DontReuse();
         }
     }
@@ -1739,22 +1731,16 @@ nsHttpTransaction::ProcessData(char *buf
         rv = HandleContent(buf, count, countRead, &countRemaining);
         if (NS_FAILED(rv)) return rv;
         // we may have read more than our share, in which case we must give
         // the excess bytes back to the connection
         if (mResponseIsComplete && countRemaining) {
             MOZ_ASSERT(mConnection);
             mConnection->PushBack(buf + *countRead, countRemaining);
         }
-
-        if (!mContentDecodingCheck && mResponseHead) {
-            mContentDecoding =
-                !!mResponseHead->PeekHeader(nsHttp::Content_Encoding);
-            mContentDecodingCheck = true;
-        }
     }
 
     return NS_OK;
 }
 
 void
 nsHttpTransaction::CancelPipeline(uint32_t reason)
 {
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -280,18 +280,16 @@ private:
     bool                            mProxyConnectFailed;
     bool                            mHttpResponseMatched;
     bool                            mPreserveStream;
     bool                            mDispatchedAsBlocking;
     bool                            mResponseTimeoutEnabled;
     bool                            mDontRouteViaWildCard;
     bool                            mForceRestart;
     bool                            mReuseOnRestart;
-    bool                            mContentDecoding;
-    bool                            mContentDecodingCheck;
 
     // mClosed           := transaction has been explicitly closed
     // mTransactionDone  := transaction ran to completion or was interrupted
     // mResponseComplete := transaction ran to completion
 
     // For Restart-In-Progress Functionality
     bool                            mReportedStart;
     bool                            mReportedResponseHeader;
--- a/netwerk/streamconv/converters/nsHTTPCompressConv.cpp
+++ b/netwerk/streamconv/converters/nsHTTPCompressConv.cpp
@@ -7,18 +7,16 @@
 #include "nsHTTPCompressConv.h"
 #include "nsMemory.h"
 #include "plstr.h"
 #include "nsCOMPtr.h"
 #include "nsError.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsComponentManagerUtils.h"
-#include "nsThreadUtils.h"
-#include "mozilla/Preferences.h"
 
 // nsISupports implementation
 NS_IMPL_ISUPPORTS(nsHTTPCompressConv,
                   nsIStreamConverter,
                   nsIStreamListener,
                   nsIRequestObserver)
 
 // nsFTPDirListingConv methods
@@ -32,23 +30,16 @@ nsHTTPCompressConv::nsHTTPCompressConv()
     , mCheckHeaderDone(false)
     , mStreamEnded(false)
     , mStreamInitialized(false)
     , mLen(0)
     , hMode(0)
     , 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()
 {
     NS_IF_RELEASE(mListener);
 
     if (mInpBuffer)
         nsMemory::Free(mInpBuffer);
@@ -92,21 +83,16 @@ nsHTTPCompressConv::OnStartRequest(nsIRe
 {
     return mListener->OnStartRequest(request, aContext);
 } 
 
 NS_IMETHODIMP
 nsHTTPCompressConv::OnStopRequest(nsIRequest* request, nsISupports *aContext, 
                                   nsresult aStatus)
 {
-    if (!mStreamEnded && NS_SUCCEEDED(aStatus) && mFailUncleanStops) {
-        // This is not a clean end of stream, the transfer is incomplete.
-        aStatus = NS_ERROR_NET_PARTIAL_TRANSFER;
-    }
-
     return mListener->OnStopRequest(request, aContext, aStatus);
 } 
 
 NS_IMETHODIMP
 nsHTTPCompressConv::OnDataAvailable(nsIRequest* request, 
                                     nsISupports *aContext, 
                                     nsIInputStream *iStr, 
                                     uint64_t aSourceOffset, 
--- a/netwerk/streamconv/converters/nsHTTPCompressConv.h
+++ b/netwerk/streamconv/converters/nsHTTPCompressConv.h
@@ -71,17 +71,16 @@ private:
     nsresult do_OnDataAvailable (nsIRequest *request, nsISupports *aContext,
                                  uint64_t aSourceOffset, const char *buffer,
                                  uint32_t aCount);
 
     bool        mCheckHeaderDone;
     bool        mStreamEnded;
     bool        mStreamInitialized;
     bool        mDummyStreamInitialised;
-    bool        mFailUncleanStops;
 
     z_stream d_stream;
     unsigned mLen, hMode, mSkipCount, mFlags;
 
     uint32_t check_header (nsIInputStream *iStr, uint32_t streamLen, nsresult *rv);
 };
 
 
--- a/netwerk/test/unit/test_content_length_underrun.js
+++ b/netwerk/test/unit/test_content_length_underrun.js
@@ -13,24 +13,22 @@ XPCOMUtils.defineLazyGetter(this, "URL",
 });
 
 var httpserver = new HttpServer();
 var index = 0;
 var test_flags = new Array();
 var testPathBase = "/cl_hdrs";
 
 var prefs;
-var enforcePrefStrict;
-var enforcePrefSoft;
+var enforcePref;
 
 function run_test()
 {
   prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
-  enforcePrefStrict = prefs.getBoolPref("network.http.enforce-framing.http1");
-  enforcePrefSoft = prefs.getBoolPref("network.http.enforce-framing.soft");
+  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);
 }
 
@@ -58,19 +56,17 @@ function setupChannel(url)
                              Ci.nsILoadInfo.SEC_NORMAL,
                              Ci.nsIContentPolicy.TYPE_OTHER);
   var httpChan = chan.QueryInterface(Components.interfaces.nsIHttpChannel);
   return httpChan;
 }
 
 function endTests()
 {
-  // restore the prefs to pre-test values
-  prefs.setBoolPref("network.http.enforce-framing.http1", enforcePrefStrict);
-  prefs.setBoolPref("network.http.enforce-framing.soft", enforcePrefSoft);
+  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)
@@ -110,19 +106,18 @@ function handler2(metadata, response)
   response.write(body);
   response.finish();
 }
 
 function completeTest2(request, data, ctx)
 {
   do_check_eq(request.status, Components.results.NS_OK);
 
-  // test 3 requires the enforce-framing prefs to be false
+  // test 3 requires the pref to be false
   prefs.setBoolPref("network.http.enforce-framing.http1", false);
-  prefs.setBoolPref("network.http.enforce-framing.soft", 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)
@@ -135,13 +130,13 @@ function handler3(metadata, response)
   response.write("Content-Length: 556677\r\n");
   response.write("\r\n");
   response.write(body);
   response.finish();
 }
 
 function completeTest3(request, data, ctx)
 {
-  // reset the strict pref in case we add more tests
+  // 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_partial_response_entry_size_smart_shrink.js
+++ b/netwerk/test/unit/test_partial_response_entry_size_smart_shrink.js
@@ -58,18 +58,18 @@ function contentHandler(metadata, respon
     response.bodyOutputStream.write(slice, slice.length);
   }
 }
 
 var enforcePref;
 
 function run_test()
 {
-  enforcePref = Services.prefs.getBoolPref("network.http.enforce-framing.soft");
-  Services.prefs.setBoolPref("network.http.enforce-framing.soft", false);
+  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_IGNORE_CL), null);
   do_test_pending();
@@ -82,11 +82,11 @@ function firstTimeThrough(request, buffe
 
   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.soft", enforcePref);
+  Services.prefs.setBoolPref("network.http.enforce-framing.http1", enforcePref);
   httpServer.stop(do_test_finished);
 }