Bug 1134609 -Make Fetch Request constructor less destructive to input on error r=bkelly
authorAntonio de Luna Lopez <tonydelun@gmail.com>
Mon, 10 Aug 2015 12:06:00 -0700
changeset 258770 8e4ca69ff980b055495a80a40991d42d8244e93f
parent 258769 c6f579ae99a69318270ecb3f18dd3b9dcdeb594c
child 258771 935545bab63273f0118a581030abfe1b895b66bf
push id29263
push userryanvm@gmail.com
push dateSun, 23 Aug 2015 21:18:49 +0000
treeherdermozilla-central@4ccdd06e51d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1134609
milestone43.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 1134609 -Make Fetch Request constructor less destructive to input on error r=bkelly
dom/fetch/InternalRequest.h
dom/fetch/Request.cpp
dom/fetch/Request.h
dom/tests/mochitest/fetch/test_request.js
--- a/dom/fetch/InternalRequest.h
+++ b/dom/fetch/InternalRequest.h
@@ -314,17 +314,17 @@ public:
   {
     mSameOriginDataURL = false;
   }
 
   void
   SetBody(nsIInputStream* aStream)
   {
     // A request's body may not be reset once set.
-    MOZ_ASSERT(!mBodyStream);
+    MOZ_ASSERT_IF(aStream, !mBodyStream);
     mBodyStream = aStream;
   }
 
   // Will return the original stream!
   // Use a tee or copy if you don't want to erase the original.
   void
   GetBody(nsIInputStream** aStream)
   {
--- a/dom/fetch/Request.cpp
+++ b/dom/fetch/Request.cpp
@@ -158,33 +158,31 @@ GetRequestURLFromWorker(const GlobalObje
 
 } // namespace
 
 /*static*/ already_AddRefed<Request>
 Request::Constructor(const GlobalObject& aGlobal,
                      const RequestOrUSVString& aInput,
                      const RequestInit& aInit, ErrorResult& aRv)
 {
+  nsCOMPtr<nsIInputStream> temporaryBody;
   nsRefPtr<InternalRequest> request;
-  bool inputRequestHasBody = false;
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aGlobal.GetAsSupports());
 
   if (aInput.IsRequest()) {
     nsRefPtr<Request> inputReq = &aInput.GetAsRequest();
     nsCOMPtr<nsIInputStream> body;
     inputReq->GetBody(getter_AddRefs(body));
     if (body) {
-      inputRequestHasBody = true;
       if (inputReq->BodyUsed()) {
         aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
         return nullptr;
-      } else {
-        inputReq->SetBodyUsed();
       }
+      temporaryBody = body;
     }
 
     request = inputReq->GetInternalRequest();
   } else {
     request = new InternalRequest();
   }
 
   request = request->GetRequestConstructorCopy(global, aRv);
@@ -322,17 +320,17 @@ Request::Constructor(const GlobalObject&
     }
   }
 
   requestHeaders->Fill(*headers, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
-  if (aInit.mBody.WasPassed() || inputRequestHasBody) {
+  if (aInit.mBody.WasPassed() || temporaryBody) {
     // HEAD and GET are not allowed to have a body.
     nsAutoCString method;
     request->GetMethod(method);
     // method is guaranteed to be uppercase due to step 14.2 above.
     if (method.EqualsLiteral("HEAD") || method.EqualsLiteral("GET")) {
       aRv.ThrowTypeError(MSG_NO_BODY_ALLOWED_FOR_GET_AND_HEAD);
       return nullptr;
     }
@@ -342,32 +340,45 @@ Request::Constructor(const GlobalObject&
     const OwningArrayBufferOrArrayBufferViewOrBlobOrFormDataOrUSVStringOrURLSearchParams& bodyInit = aInit.mBody.Value();
     nsCOMPtr<nsIInputStream> stream;
     nsAutoCString contentType;
     aRv = ExtractByteStreamFromBody(bodyInit,
                                     getter_AddRefs(stream), contentType);
     if (NS_WARN_IF(aRv.Failed())) {
       return nullptr;
     }
-    request->ClearCreatedByFetchEvent();
-    request->SetBody(stream);
+
+    temporaryBody = stream;
 
     if (!contentType.IsVoid() &&
         !requestHeaders->Has(NS_LITERAL_CSTRING("Content-Type"), aRv)) {
       requestHeaders->Append(NS_LITERAL_CSTRING("Content-Type"),
                              contentType, aRv);
     }
 
-    if (aRv.Failed()) {
+    if (NS_WARN_IF(aRv.Failed())) {
       return nullptr;
     }
+
+    request->ClearCreatedByFetchEvent();
+    request->SetBody(temporaryBody);
   }
 
   nsRefPtr<Request> domRequest = new Request(global, request);
   domRequest->SetMimeType();
+
+  if (aInput.IsRequest()) {
+    nsRefPtr<Request> inputReq = &aInput.GetAsRequest();
+    nsCOMPtr<nsIInputStream> body;
+    inputReq->GetBody(getter_AddRefs(body));
+    if (body) {
+      inputReq->SetBody(nullptr);
+      inputReq->SetBodyUsed();
+    }
+  }
   return domRequest.forget();
 }
 
 already_AddRefed<Request>
 Request::Clone(ErrorResult& aRv) const
 {
   if (BodyUsed()) {
     aRv.ThrowTypeError(MSG_FETCH_BODY_CONSUMED_ERROR);
--- a/dom/fetch/Request.h
+++ b/dom/fetch/Request.h
@@ -100,16 +100,19 @@ public:
     return mRequest->Headers();
   }
 
   Headers* Headers_();
 
   void
   GetBody(nsIInputStream** aStream) { return mRequest->GetBody(aStream); }
 
+  void
+  SetBody(nsIInputStream* aStream) { return mRequest->SetBody(aStream); }
+
   static already_AddRefed<Request>
   Constructor(const GlobalObject& aGlobal, const RequestOrUSVString& aInput,
               const RequestInit& aInit, ErrorResult& rv);
 
   nsIGlobalObject* GetParentObject() const
   {
     return mOwner;
   }
--- a/dom/tests/mochitest/fetch/test_request.js
+++ b/dom/tests/mochitest/fetch/test_request.js
@@ -451,25 +451,37 @@ function testBug1154268() {
       var r2 = new Request(r1, { method: method });
       ok(false, method + " Request copied from POST Request with body should fail.");
     } catch (e) {
       is(e.name, "TypeError", method + " Request copied from POST Request with body should fail.");
     }
   });
 }
 
+function testRequestConsumedByFailedConstructor(){
+  var r1 = new Request('http://example.com', { method: 'POST', body: 'hello world' });
+  try{
+    var r2 = new Request(r1, { method: 'GET' });
+    ok(false, 'GET Request copied from POST Request with body should fail.');
+  } catch(e) {
+    ok(true, 'GET Request copied from POST Request with body should fail.');
+  }
+  ok(!r1.bodyUsed, 'Initial request should not be consumed by failed Request constructor');
+}
+
 function runTest() {
   testDefaultCtor();
   testSimpleUrlParse();
   testUrlFragment();
   testMethod();
   testBug1109574();
   testHeaderGuard();
   testModeCorsPreflightEnumValue();
   testBug1154268();
+  testRequestConsumedByFailedConstructor();
 
   return Promise.resolve()
     .then(testBodyCreation)
     .then(testBodyUsed)
     .then(testBodyExtraction)
     .then(testFormDataBodyCreation)
     .then(testFormDataBodyExtraction)
     .then(testUsedRequest)