bug 1196882 - dont enforce h1 framing on non 2xx r=bagder a=sylvestre
authorPatrick McManus <mcmanus@ducksong.com>
Fri, 28 Aug 2015 10:57:16 -0400
changeset 289109 422d0883290ede7f6853b1a148cb24b45e20cb44
parent 289108 6a1945ebef4166ab8517d765a06657d749e3805d
child 289110 5b331e32276013f99cbe7034f06eb2fbd1e64c80
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder, sylvestre
bugs1196882
milestone42.0a2
bug 1196882 - dont enforce h1 framing on non 2xx r=bagder a=sylvestre
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/protocol/http/nsHttpTransaction.h
netwerk/test/unit/test_content_length_underrun.js
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -102,16 +102,17 @@ nsHttpTransaction::nsHttpTransaction()
     , mStatus(NS_OK)
     , mPriority(0)
     , mRestartCount(0)
     , mCaps(0)
     , mClassification(CLASS_GENERAL)
     , mPipelinePosition(0)
     , mCapsToClear(0)
     , mHttpVersion(NS_HTTP_VERSION_UNKNOWN)
+    , mHttpResponseCode(0)
     , mClosed(false)
     , mConnected(false)
     , mHaveStatusLine(false)
     , mHaveAllHeaders(false)
     , mTransactionDone(false)
     , mResponseIsComplete(false)
     , mDidContentStart(false)
     , mNoContent(false)
@@ -976,17 +977,18 @@ 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) {
+        if ((mHttpResponseCode / 100 == 2) &&
+            (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"));
@@ -1529,16 +1531,17 @@ nsHttpTransaction::HandleContentStart()
             mResponseHead->Flatten(headers, false);
             LogHeaders(headers.get());
             LOG3(("]\n"));
         }
 
         // Save http version, mResponseHead isn't available anymore after
         // TakeResponseHead() is called
         mHttpVersion = mResponseHead->Version();
+        mHttpResponseCode = mResponseHead->Status();
 
         // notify the connection, give it a chance to cause a reset.
         bool reset = false;
         if (!mRestartInProgressVerifier.IsSetup())
             mConnection->OnHeadersAvailable(this, mRequestHead, mResponseHead, &reset);
 
         // looks like we should ignore this response, resetting...
         if (reset) {
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -268,16 +268,17 @@ private:
     // redundant requests on the network. The member itself is atomic, but
     // access to it from the networking thread may happen either before or
     // after the main thread modifies it. To deal with raciness, only unsetting
     // bitfields should be allowed: 'lost races' will thus err on the
     // conservative side, e.g. by going ahead with a 2nd DNS refresh.
     Atomic<uint32_t>                mCapsToClear;
 
     nsHttpVersion                   mHttpVersion;
+    uint16_t                        mHttpResponseCode;
 
     // state flags, all logically boolean, but not packed together into a
     // bitfield so as to avoid bitfield-induced races.  See bug 560579.
     bool                            mClosed;
     bool                            mConnected;
     bool                            mHaveStatusLine;
     bool                            mHaveAllHeaders;
     bool                            mTransactionDone;
--- a/netwerk/test/unit/test_content_length_underrun.js
+++ b/netwerk/test/unit/test_content_length_underrun.js
@@ -121,16 +121,39 @@ function handler1(metadata, response)
   response.write(body);
   response.finish();
 }
 
 function completeTest1(request, data, ctx)
 {
   do_check_eq(request.status, Components.results.NS_ERROR_NET_PARTIAL_TRANSFER);
 
+  run_test_number(11);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Test 11: PASS because of Content-Length underrun with HTTP 1.1 but non 2xx
+test_flags[11] = CL_IGNORE_CL;
+
+function handler11(metadata, response)
+{
+  var body = "blablabla";
+
+  response.seizePower();
+  response.write("HTTP/1.1 404 NotOK\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 completeTest11(request, data, ctx)
+{
+  do_check_eq(request.status, Components.results.NS_OK);
   run_test_number(2);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Test 2: Succeed because Content-Length underrun is with HTTP 1.0
 
 test_flags[2] = CL_IGNORE_CL;