[Fetch] Request with GET/HEAD method cannot have body (issue 882383002 by hiroshige@chromium.org)

hiro...@chromium.org

unread,

Jan 29, 2015, 8:05:58 AM1/29/15

to yhi...@chromium.org, blink-...@chromium.org

Reviewers: yhirano,

Message:
Seems spec was updated after the code.

Description:
[Fetch] Request with GET/HEAD method cannot have body

BUG=444493

Please review this at https://codereview.chromium.org/882383002/

Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Affected files (+32, -7 lines):
M LayoutTests/http/tests/fetch/script-tests/request.js
M Source/modules/fetch/Request.cpp

Index: LayoutTests/http/tests/fetch/script-tests/request.js
diff --git a/LayoutTests/http/tests/fetch/script-tests/request.js
b/LayoutTests/http/tests/fetch/script-tests/request.js
index
9be837f8afe11800549a8f6e862972d3196bfc08..14364fe0710462198ab0fb185fd7576ac0750d88
100644
--- a/LayoutTests/http/tests/fetch/script-tests/request.js
+++ b/LayoutTests/http/tests/fetch/script-tests/request.js
@@ -384,11 +384,31 @@ test(function() {
},
'Request construction behavior regarding "used" body flag and
exceptions.');

+
+// Spec: https://fetch.spec.whatwg.org/#dom-request
+// Step 21:
+// If request's method is `GET` or `HEAD`, throw a TypeError.
+promise_test(function() {
+ var headers = new Headers;
+ headers.set('Content-Language', 'ja');
+ ['GET', 'HEAD'].forEach(function(method) {
+ assert_throws(
+ {name: 'TypeError'},
+ function() {
+ new Request(URL,
+ {method: method,
+ body: new Blob(['Test Blob'], {type: 'test/type'})
+ });
+ },
+ 'Request of GET/HEAD method cannot have RequestInit body.');
+ });
+ }, 'Request of GET/HEAD method cannot have RequestInit body.');
+
promise_test(function() {
var headers = new Headers;
headers.set('Content-Language', 'ja');
var req = new Request(URL, {
- method: 'GET',
+ method: 'POST',
headers: headers,
body: new Blob(['Test Blob'], {type: 'test/type'})
});
Index: Source/modules/fetch/Request.cpp
diff --git a/Source/modules/fetch/Request.cpp
b/Source/modules/fetch/Request.cpp
index
9e4ace193ce20ce3e8e98febb5a0c4aff5288b24..b6ae9409ceda7c69d3b77896e3aef837578f6a94
100644
--- a/Source/modules/fetch/Request.cpp
+++ b/Source/modules/fetch/Request.cpp
@@ -179,13 +179,18 @@ Request*
Request::createRequestWithRequestOrString(ExecutionContext* context, Re

// "21. If |init|'s body member is present, run these substeps:"
if (init.bodyBlobHandle) {
- // "1. Let |stream| and |Content-Type| be the result of extracting
- // |init|'s body member."
- // "2. Set |r|'s request's body to |stream|."
- // "3.If |Content-Type| is non-null and |r|'s request's header list
+ // "1. If request's method is `GET` or `HEAD`, throw a TypeError."
+ // "2. Let |stream| and |Content-Type| be the result of extracting
+ // |init|'s body member."
+ // "3. Set |r|'s request's body to |stream|."
+ // "4. If |Content-Type| is non-null and |r|'s request's header
list
// contains no header named `Content-Type`, append
- // `Content-Type`/|Content-Type| to |r|'s Headers object. Rethrow
any
- // exception."
+ // `Content-Type`/|Content-Type| to |r|'s Headers object. Rethrow
any
+ // exception."
+ if (request->method() == "GET" || request->method() == "HEAD") {
+ exceptionState.throwTypeError("Request with GET/HEAD method
cannot have body.");
+ return 0;
+ }
r->setBodyBlobHandle(init.bodyBlobHandle);
if (!init.bodyBlobHandle->type().isEmpty()
&& !r->headers()->has("Content-Type", exceptionState)) {
r->headers()->append("Content-Type",
init.bodyBlobHandle->type(), exceptionState);

hiro...@chromium.org

unread,

Jan 29, 2015, 9:28:08 AM1/29/15

to yhi...@chromium.org, ho...@chromium.org, blink-...@chromium.org

+horo@.

I updated the code according to:
https://fetch.spec.whatwg.org/#dom-request
Step 21.1:

If request's method is `GET` or `HEAD`, throw a TypeError.

However, this CL make the following serviceworker tests:
serviceworker/cache-delete.html
serviceworker/cache-put.html
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/44247/layout-test-results/results.html
(or also
serviceworker/cache-add.html
serviceworker/cache-match.html
locally).

At least some tests use Request with 'GET' and body, but I'm not sure how
to fix
them correctly.

Any thoughts?

https://codereview.chromium.org/882383002/

ho...@chromium.org

unread,

Feb 2, 2015, 4:32:34 AM2/2/15

to hiro...@chromium.org, yhi...@chromium.org, blink-...@chromium.org

Currently cache.add is not supported. And cache.put only supports "GET"
method.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/serviceworkers/Cache.cpp&l=361

So plase change the meshod to POST in the tests (ex: new
Request('simple.txt',
{body: 'Hello', method: 'POST'});), and change *-expected.txt
adding "FAIL ...".

https://codereview.chromium.org/882383002/