Make the Response constructor throw when status is a null body status and body is non-null

Source/modules/fetch/Response.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/fetch/Response.h"
 
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMArrayBufferView.h"
 #include "core/fileapi/Blob.h"
 #include "core/html/DOMFormData.h"
 #include "modules/fetch/BodyStreamBuffer.h"
 #include "modules/fetch/FetchBlobDataConsumerHandle.h"
 #include "modules/fetch/ResponseInit.h"
 #include "platform/network/FormData.h"
 #include "platform/network/HTTPHeaderMap.h"
 #include "public/platform/WebServiceWorkerResponse.h"
 #include "wtf/RefPtr.h"
 
+// Check whether |status| is a null body status

[hiroshige, 2015/07/30 11:20:40]

nit: s/Check/Checks/, and append a period at the last of the statement.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Comm...

+// Spec: https://fetch.spec.whatwg.org/#statuses

[hiroshige, 2015/07/30 11:20:40]

nit: https://fetch.spec.whatwg.org/#null-body-status is better.

[shiva.jm, 2015/07/30 11:47:16]

Done.

+bool isNullBodyStatus(unsigned short status)
+{
+    if (status == 101 || status == 204 || status == 205 || status == 304)
+        return true;
+
+    return false;
+}
+
 namespace blink {
 
 namespace {
 

[hiroshige, 2015/07/30 11:20:40]

Please move isNullBodyStatus() here (or somewhere in "namespace blink {
namespace { ..."), rather than exposing it in the global namespace.

[shiva.jm, 2015/07/30 11:47:16]

Done.

 FetchResponseData* createFetchResponseDataFromWebResponse(ExecutionContext* executionContext, const WebServiceWorkerResponse& webResponse)
 {
     FetchResponseData* response = 0;
     if (webResponse.status() > 0)
         response = FetchResponseData::create();
     else
         response = FetchResponseData::createNetworkErrorResponse();
 
     response->setURL(webResponse.url());
     response->setStatus(webResponse.status());
@@ skipping 110 matching lines @@
         const long long length = blobData->length();
         Blob* blob = Blob::create(BlobDataHandle::create(blobData.release(), length));
         return create(context, blob, ResponseInit(responseInit, exceptionState), exceptionState);
     }
     ASSERT_NOT_REACHED();
     return nullptr;
 }
 
 Response* Response::create(ExecutionContext* context, Blob* body, const ResponseInit& responseInit, ExceptionState& exceptionState)
 {
-    // "1. If |init|'s status member is not in the range 200 to 599, throw a
+    unsigned short status = responseInit.status;
+
+    // "1. If |init|'s status member is not in the range 200 to 599, inclusive, throw a
     // RangeError."
-    if (responseInit.status < 200 || 599 < responseInit.status) {
+    if (status < 200 || 599 < status) {
         exceptionState.throwRangeError("Invalid status");

[hiroshige, 2015/07/30 11:20:40]

How about applying this?
https://codereview.chromium.org/1258933002/diff/1/Source/modules/fetch/Respon...
On 2015/07/27 11:36:08, philipj wrote:

[shiva.jm, 2015/07/30 11:47:16]

Done.

         return 0;
     }
 
     // "2. If |init|'s statusText member does not match the Reason-Phrase
     // token production, throw a TypeError."
     if (!isValidReasonPhrase(responseInit.statusText)) {
         exceptionState.throwTypeError("Invalid statusText");
         return 0;
     }
 
@@ skipping 21 matching lines @@
         // "1. Empty |r|'s response's header list."
         r->m_response->headerList()->clearList();
         // "2. Fill |r|'s Headers object with |init|'s headers member. Rethrow
         // any exceptions."
         r->m_headers->fillWith(responseInit.headersDictionary, exceptionState);
         if (exceptionState.hadException())
             return 0;
     }
     // "7. If body is given, run these substeps:"
     if (body) {
-        // "1. Let |stream| and |Content-Type| be the result of extracting body."
-        // "2. Set |r|'s response's body to |stream|."
-        // "3. If |Content-Type| is non-null and |r|'s response's header list
+        // "1. If |init|'s status member is a null body status, throw a TypeError."
+        // "2. Let |stream| and |Content-Type| be the result of extracting body."
+        // "3. Set |r|'s response's body to |stream|."
+        // "4. If |Content-Type| is non-null and |r|'s response's header list
         // contains no header named `Content-Type`, append `Content-Type`/
         // |Content-Type| to |r|'s response's header list."
         // https://fetch.spec.whatwg.org/#concept-bodyinit-extract
         // Step 3, Blob:
         // "If object's type attribute is not the empty byte sequence, set
         // Content-Type to its value."
+        if (isNullBodyStatus(status)) {
+            exceptionState.throwTypeError("Response with null body status cannot have body");
+            return 0;
+        }
         r->m_response->replaceBodyStreamBuffer(BodyStreamBuffer::create(FetchBlobDataConsumerHandle::create(context, body->blobDataHandle())));
         r->refreshBody();
         if (!body->type().isEmpty() && !r->m_response->headerList()->has("Content-Type"))
             r->m_response->headerList()->append("Content-Type", body->type());
     }
 
     // "8. Set |r|'s MIME type to the result of extracting a MIME type
     // from |r|'s response's header list."
     r->m_response->setMIMEType(r->m_response->headerList()->extractMIMEType());
 
@@ skipping 214 matching lines @@
 }
 
 DEFINE_TRACE(Response)
 {
     Body::trace(visitor);
     visitor->trace(m_response);
     visitor->trace(m_headers);
 }
 
 } // namespace blink