Commit 3cbb8c9a authored by Dong-hee Na's avatar Dong-hee Na Committed by Commit Bot

Fetch: Throw TypeError for response from disturbed/locked streams

If the Response body object is a readable stream then check whether
the object is disturbed or locked.
If the condition is true then throw TypeError.

refernce: https://fetch.spec.whatwg.org/#concept-bodyinit-extract

Bug: 878281
Change-Id: I0593d048e7bd146863efdc9f1d355bc70857cdc0
Reviewed-on: https://chromium-review.googlesource.com/1226718Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#592402}
parent bc2b383e
This is a testharness.js-based test.
FAIL Constructing a Response with a stream on which getReader() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() and releaseLock() are called assert_throws: function "() => new Response(stream)" did not throw
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Constructing a Response with a stream on which getReader() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() and releaseLock() are called assert_throws: function "() => new Response(stream)" did not throw
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Constructing a Response with a stream on which getReader() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() and releaseLock() are called assert_throws: function "() => new Response(stream)" did not throw
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Constructing a Response with a stream on which getReader() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() is called assert_throws: function "() => new Response(stream)" did not throw
FAIL Constructing a Response with a stream on which read() and releaseLock() are called assert_throws: function "() => new Response(stream)" did not throw
Harness: the test ran to completion.
...@@ -6,6 +6,6 @@ ...@@ -6,6 +6,6 @@
test(() => { test(() => {
let response; let response;
fillStackAndRun(() => response = new Response('hi')); fillStackAndRun(() => response = new Response('hi'));
assert_equals(response.body, undefined, 'response body should be undefined'); assert_equals(response.body instanceof ReadableStream, true, 'response body should be ReadableStream');
}, 'stack overflow in Response constructor should lead to an undefined body'); }, 'stack overflow in Response constructor should not crash the browser');
</script> </script>
...@@ -6,8 +6,6 @@ ...@@ -6,8 +6,6 @@
test(() => { test(() => {
const rs = new ReadableStream(); const rs = new ReadableStream();
const response = new Response(rs); const response = new Response(rs);
assert_throws("InvalidStateError", fillStackAndRun(() => response.bodyUsed)
() => fillStackAndRun(() => response.bodyUsed),
'getter should throw a DOMException');
}, 'stack overflow in bodyUsed should not crash the browser'); }, 'stack overflow in bodyUsed should not crash the browser');
</script> </script>
...@@ -480,6 +480,14 @@ test(() => { ...@@ -480,6 +480,14 @@ test(() => {
assert_equals(response.body, stream); assert_equals(response.body, stream);
}, 'Response constructed with a stream'); }, 'Response constructed with a stream');
test(() => {
var controller;
var stream = new ReadableStream({start: c => controller = c});
stream.getReader();
assert_throws(TypeError(), () => new Response(stream),
'Response constructor should throw TypeError');
}, 'Response constructed with a locked stream');
promise_test(() => { promise_test(() => {
var controller; var controller;
var stream = new ReadableStream({start: c => controller = c}); var stream = new ReadableStream({start: c => controller = c});
...@@ -539,14 +547,6 @@ promise_test(t => { ...@@ -539,14 +547,6 @@ promise_test(t => {
return promise_rejects(t, TypeError(), response.text()); return promise_rejects(t, TypeError(), response.text());
}, 'Response constructed with an errored stream'); }, 'Response constructed with an errored stream');
promise_test(t => {
var controller;
var stream = new ReadableStream({start: c => controller = c});
stream.getReader();
var response = new Response(stream);
return promise_rejects(t, TypeError(), response.text());
}, 'Response constructed with a locked stream');
promise_test(t => { promise_test(t => {
var controller; var controller;
var stream = new ReadableStream({start: c => controller = c}); var stream = new ReadableStream({start: c => controller = c});
......
...@@ -230,7 +230,7 @@ Response* Response::Create(ScriptState* script_state, ...@@ -230,7 +230,7 @@ Response* Response::Create(ScriptState* script_state,
unsigned short status = init.status(); unsigned short status = init.status();
// "1. If |init|'s status member is not in the range 200 to 599, inclusive, // "1. If |init|'s status member is not in the range 200 to 599, inclusive,
// throw a RangeError." // throw a RangeError."
if (status < 200 || 599 < status) { if (status < 200 || 599 < status) {
exception_state.ThrowRangeError( exception_state.ThrowRangeError(
ExceptionMessages::IndexOutsideRange<unsigned>( ExceptionMessages::IndexOutsideRange<unsigned>(
...@@ -246,17 +246,18 @@ Response* Response::Create(ScriptState* script_state, ...@@ -246,17 +246,18 @@ Response* Response::Create(ScriptState* script_state,
return nullptr; return nullptr;
} }
// "3. Let |r| be a new Response object, associated with a new response, // "3. Let |r| be a new Response object, associated with a new response.
// Headers object, and Body object." // "4. Set |r|'s headers to a new Headers object whose list is
// |r|'s response's header list, and guard is "response" "
Response* r = new Response(ExecutionContext::From(script_state)); Response* r = new Response(ExecutionContext::From(script_state));
// "5. Set |r|'s response's status to |init|'s status member."
// "4. Set |r|'s response's status to |init|'s status member."
r->response_->SetStatus(init.status()); r->response_->SetStatus(init.status());
// "5. Set |r|'s response's status message to |init|'s statusText member." // "6. Set |r|'s response's status message to |init|'s statusText member."
r->response_->SetStatusMessage(AtomicString(init.statusText())); r->response_->SetStatusMessage(AtomicString(init.statusText()));
// "6. If |init|'s headers member is present, run these substeps:" // "7. If |init|'s headers exists, then fill |r|’s headers with
// |init|'s headers"
if (init.hasHeaders()) { if (init.hasHeaders()) {
// "1. Empty |r|'s response's header list." // "1. Empty |r|'s response's header list."
r->response_->HeaderList()->ClearList(); r->response_->HeaderList()->ClearList();
...@@ -266,36 +267,56 @@ Response* Response::Create(ScriptState* script_state, ...@@ -266,36 +267,56 @@ Response* Response::Create(ScriptState* script_state,
if (exception_state.HadException()) if (exception_state.HadException())
return nullptr; return nullptr;
} }
// "7. If body is given, run these substeps:" // "8. If body is non-null, then:"
if (body) { if (body) {
// "1. If |init|'s status member is a null body status, throw a // "1. If |init|'s status is a null body status, then throw a TypeError."
// 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)) { if (IsNullBodyStatus(status)) {
exception_state.ThrowTypeError( exception_state.ThrowTypeError(
"Response with null body status cannot have body"); "Response with null body status cannot have body");
return nullptr; return nullptr;
} }
// "2. Let |Content-Type| be null."
// "3. Set |r|'s response's body and |Content-Type|
// to the result of extracting body."
// https://fetch.spec.whatwg.org/#concept-bodyinit-extract
// Step 5, Blob:
// "If object's type attribute is not the empty byte sequence, set
// Content-Type to its value."
r->response_->ReplaceBodyStreamBuffer(body); r->response_->ReplaceBodyStreamBuffer(body);
// https://fetch.spec.whatwg.org/#concept-bodyinit-extract
// Step 5, ReadableStream:
// "If object is disturbed or locked, then throw a TypeError."
// If the BodyStreamBuffer was not constructed from a ReadableStream
// then IsStreamLocked and IsStreamDisturbed will always be false.
// So we don't have to check BodyStreamBuffer is a ReadableStream
// or not.
if (body->IsStreamLocked(exception_state).value_or(true) ||
body->IsStreamDisturbed(exception_state).value_or(true)) {
if (!exception_state.HadException()) {
exception_state.ThrowTypeError(
"Response body object should not be disturbed or locked");
}
return nullptr;
}
// "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."
if (!content_type.IsEmpty() && if (!content_type.IsEmpty() &&
!r->response_->HeaderList()->Has("Content-Type")) !r->response_->HeaderList()->Has("Content-Type"))
r->response_->HeaderList()->Append("Content-Type", content_type); r->response_->HeaderList()->Append("Content-Type", content_type);
} }
// "8. Set |r|'s MIME type to the result of extracting a MIME type // "9. Set |r|'s MIME type to the result of extracting a MIME type
// from |r|'s response's header list." // from |r|'s response's header list."
r->response_->SetMIMEType(r->response_->HeaderList()->ExtractMIMEType()); r->response_->SetMIMEType(r->response_->HeaderList()->ExtractMIMEType());
// "9. Return |r|." // "10. Set |r|'s response’s HTTPS state to current settings object's"
// HTTPS state."
// "11. Resolve |r|'s trailer promise with a new Headers object whose
// guard is "immutable"."
// "12. Return |r|."
return r; return r;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment