Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-09-27 07:10:19 +01:00
Code Issues Projects Releases Packages Wiki Activity

Fully implement request constructor body handling (#39514)

Browse source 
This aligns the implementation with the spec, where both input body and
init body are now set. In doing so, it fixes a fetch abort test, since
the stream was missing for the input body.

It also introduces the `unusable` method, as that's the one the spec
uses. The other two getters no longer exist in the spec.

Fixes #39448

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This commit is contained in:
Tim van der Lippe 2025-09-27 03:52:01 +02:00 • committed by GitHub
parent 043425cae5
commit 4c25039d35
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 96 additions and 113 deletions
Download patch file Download diff file Expand all files Collapse all files

10
components/script/body.rs
View file

@ -617,7 +617,7 @@ pub(crate) fn consume_body<T: BodyMixin + DomObject>(
let promise = Promise::new_in_current_realm(comp, can_gc); let promise = Promise::new_in_current_realm(comp, can_gc);
// If object is unusable, then return a promise rejected with a TypeError. // If object is unusable, then return a promise rejected with a TypeError.
if object.is_disturbed() || object.is_locked() { if object.is_unusable() {
promise.reject_error( promise.reject_error(
Error::Type("The body's stream is disturbed or locked".to_string()), Error::Type("The body's stream is disturbed or locked".to_string()),
can_gc, can_gc,
@ -874,12 +874,12 @@ pub(crate) fn decode_to_utf16_with_bom_removal(
/// <https://fetch.spec.whatwg.org/#body> /// <https://fetch.spec.whatwg.org/#body>
pub(crate) trait BodyMixin { pub(crate) trait BodyMixin {
/// <https://fetch.spec.whatwg.org/#concept-body-disturbed> /// <https://fetch.spec.whatwg.org/#dom-body-bodyused>
fn is_disturbed(&self) -> bool; fn is_body_used(&self) -> bool;
/// <https://fetch.spec.whatwg.org/#body-unusable>
fn is_unusable(&self) -> bool;
/// <https://fetch.spec.whatwg.org/#dom-body-body> /// <https://fetch.spec.whatwg.org/#dom-body-body>
fn body(&self) -> Option<DomRoot<ReadableStream>>; fn body(&self) -> Option<DomRoot<ReadableStream>>;
/// <https://fetch.spec.whatwg.org/#concept-body-locked>
fn is_locked(&self) -> bool;
/// <https://fetch.spec.whatwg.org/#concept-body-mime-type> /// <https://fetch.spec.whatwg.org/#concept-body-mime-type>
fn get_mime_type(&self, can_gc: CanGc) -> Vec<u8>; fn get_mime_type(&self, can_gc: CanGc) -> Vec<u8>;
} }

103
components/script/dom/request.rs
View file

@ -111,6 +111,9 @@ impl Request {
// Step 4. Let signal be null. // Step 4. Let signal be null.
let mut signal: Option<DomRoot<AbortSignal>> = None; let mut signal: Option<DomRoot<AbortSignal>> = None;
// Required later for step 41.1
let mut input_body_is_unusable = false;
match input { match input {
// Step 5. If input is a string, then: // Step 5. If input is a string, then:
RequestInfo::USVString(USVString(ref usv_string)) => { RequestInfo::USVString(USVString(ref usv_string)) => {
@ -133,11 +136,8 @@ impl Request {
// Step 6. Otherwise: // Step 6. Otherwise:
// Step 6.1. Assert: input is a Request object. // Step 6.1. Assert: input is a Request object.
RequestInfo::Request(ref input_request) => { RequestInfo::Request(ref input_request) => {
// This looks like Step 38 // Preparation for step 41.1
// TODO do this in the right place to not mask other errors input_body_is_unusable = input_request.is_unusable();
if request_is_disturbed(input_request) || request_is_locked(input_request) {
return Err(Error::Type("Input is disturbed or locked".to_string()));
}
// Step 6.2. Set request to input’s request. // Step 6.2. Set request to input’s request.
temporary_request = input_request.request.borrow().clone(); temporary_request = input_request.request.borrow().clone();
// Step 6.3. Set signal to input’s signal. // Step 6.3. Set signal to input’s signal.
@ -419,8 +419,9 @@ impl Request {
r.request.borrow_mut().headers = r.Headers(can_gc).get_headers_list(); r.request.borrow_mut().headers = r.Headers(can_gc).get_headers_list();
// Step 34. Let inputBody be input’s request’s body if input is a Request object; otherwise null. // Step 34. Let inputBody be input’s request’s body if input is a Request object; otherwise null.
let mut input_body = if let RequestInfo::Request(ref mut input_request) = input { let input_body = if let RequestInfo::Request(ref mut input_request) = input {
let mut input_request_request = input_request.request.borrow_mut(); let mut input_request_request = input_request.request.borrow_mut();
r.body_stream.set(input_request.body().as_deref());
input_request_request.body.take() input_request_request.body.take()
} else { } else {
None None
@ -428,8 +429,7 @@ impl Request {
// Step 35. If either init["body"] exists and is non-null or inputBody is non-null, // Step 35. If either init["body"] exists and is non-null or inputBody is non-null,
// and request’s method is `GET` or `HEAD`, then throw a TypeError. // and request’s method is `GET` or `HEAD`, then throw a TypeError.
if let Some(init_body_option) = init.body.as_ref() { if init.body.as_ref().is_some_and(|body| body.is_some()) || input_body.is_some() {
if init_body_option.is_some() || input_body.is_some() {
let req = r.request.borrow(); let req = r.request.borrow();
let req_method = &req.method; let req_method = &req.method;
match *req_method { match *req_method {
@ -446,19 +446,16 @@ impl Request {
_ => {}, _ => {},
} }
} }
}
// Step 36. Let initBody be null. // Step 36. Let initBody be null.
let mut init_body = None;
// Step 37. If init["body"] exists and is non-null, then: // Step 37. If init["body"] exists and is non-null, then:
if let Some(Some(ref init_body)) = init.body { if let Some(Some(ref input_init_body)) = init.body {
// Step 37.1. Let bodyWithType be the result of extracting init["body"], with keepalive set to request’s keepalive. // Step 37.1. Let bodyWithType be the result of extracting init["body"], with keepalive set to request’s keepalive.
// TODO let mut body_with_type = input_init_body.extract(global, can_gc)?;
// Step 37.2. Set initBody to bodyWithType’s body.
let mut extracted_body = init_body.extract(global, can_gc)?;
// Step 37.3. Let type be bodyWithType’s type. // Step 37.3. Let type be bodyWithType’s type.
if let Some(contents) = extracted_body.content_type.take() { if let Some(contents) = body_with_type.content_type.take() {
let ct_header_name = b"Content-Type"; let ct_header_name = b"Content-Type";
// Step 37.4. If type is non-null and this’s headers’s header list // Step 37.4. If type is non-null and this’s headers’s header list
// does not contain `Content-Type`, then append (`Content-Type`, type) to this’s headers. // does not contain `Content-Type`, then append (`Content-Type`, type) to this’s headers.
@ -485,31 +482,52 @@ impl Request {
} }
} }
let (net_body, stream) = extracted_body.into_net_request_body(); // Step 37.2. Set initBody to bodyWithType’s body.
let (net_body, stream) = body_with_type.into_net_request_body();
r.body_stream.set(Some(&*stream)); r.body_stream.set(Some(&*stream));
input_body = Some(net_body); init_body = Some(net_body);
} }
// Step 38. Let inputOrInitBody be initBody if it is non-null; otherwise inputBody. // Step 38. Let inputOrInitBody be initBody if it is non-null; otherwise inputBody.
// Step 40. Let finalBody be inputOrInitBody.
// Step 41.2. Set finalBody to the result of creating a proxy for inputBody.
//
// There are multiple reassignments to similar values. In the end, all end up as
// final_body. Therefore, final_body is equivalent to inputOrInitBody
let final_body = init_body.or(input_body);
// Step 39. If inputOrInitBody is non-null and inputOrInitBody’s source is null, then: // Step 39. If inputOrInitBody is non-null and inputOrInitBody’s source is null, then:
if final_body
.as_ref()
.is_some_and(|body| body.source_is_null())
{
// Step 39.1. If initBody is non-null and init["duplex"] does not exist, then throw a TypeError.
// TODO // TODO
// This looks like where we need to set the use-preflight flag // Step 39.2. If this’s request’s mode is neither "same-origin" nor "cors", then throw a TypeError.
// if the request has a body and nothing else has set the flag. let request_mode = &r.request.borrow().mode;
if *request_mode != NetTraitsRequestMode::CorsMode &&
// Step 40. Let finalBody be inputOrInitBody. *request_mode != NetTraitsRequestMode::SameOrigin
// {
// is done earlier return Err(Error::Type(
"Request mode must be Cors or SameOrigin".to_string(),
));
}
// Step 39.3. Set this’s request’s use-CORS-preflight flag.
// TODO
}
// Step 41. If initBody is null and inputBody is non-null, then: // Step 41. If initBody is null and inputBody is non-null, then:
// TODO // Step 41.1. If inputBody is unusable, then throw a TypeError.
// Step 41.1. If input is unusable, then throw a TypeError. //
// TODO // We only perform this check on input_body. However, we already
// Step 41.2. Set finalBody to the result of creating a proxy for inputBody. // processed the input body. Therefore, we check it all the way
// TODO // above and throw the error at the last possible moment
if input_body_is_unusable {
return Err(Error::Type("Input body is unusable".to_string()));
}
// Step 42. Set this’s request’s body to finalBody. // Step 42. Set this’s request’s body to finalBody.
r.request.borrow_mut().body = input_body; r.request.borrow_mut().body = final_body;
Ok(r) Ok(r)
} }
@ -582,16 +600,6 @@ fn includes_credentials(input: &ServoUrl) -> bool {
!input.username().is_empty() || input.password().is_some() !input.username().is_empty() || input.password().is_some()
} }
// https://fetch.spec.whatwg.org/#concept-body-disturbed
fn request_is_disturbed(input: &Request) -> bool {
input.is_disturbed()
}
// https://fetch.spec.whatwg.org/#concept-body-locked
fn request_is_locked(input: &Request) -> bool {
input.is_locked()
}
impl RequestMethods<crate::DomTypeHolder> for Request { impl RequestMethods<crate::DomTypeHolder> for Request {
// https://fetch.spec.whatwg.org/#dom-request // https://fetch.spec.whatwg.org/#dom-request
fn Constructor( fn Constructor(
@ -681,7 +689,7 @@ impl RequestMethods<crate::DomTypeHolder> for Request {
// https://fetch.spec.whatwg.org/#dom-body-bodyused // https://fetch.spec.whatwg.org/#dom-body-bodyused
fn BodyUsed(&self) -> bool { fn BodyUsed(&self) -> bool {
self.is_disturbed() self.is_body_used()
} }
/// <https://fetch.spec.whatwg.org/#dom-request-signal> /// <https://fetch.spec.whatwg.org/#dom-request-signal>
@ -694,11 +702,8 @@ impl RequestMethods<crate::DomTypeHolder> for Request {
// https://fetch.spec.whatwg.org/#dom-request-clone // https://fetch.spec.whatwg.org/#dom-request-clone
fn Clone(&self, can_gc: CanGc) -> Fallible<DomRoot<Request>> { fn Clone(&self, can_gc: CanGc) -> Fallible<DomRoot<Request>> {
// Step 1. If this is unusable, then throw a TypeError. // Step 1. If this is unusable, then throw a TypeError.
if request_is_locked(self) { if self.is_unusable() {
return Err(Error::Type("Request is locked".to_string())); return Err(Error::Type("Request is unusable".to_string()));
}
if request_is_disturbed(self) {
return Err(Error::Type("Request is disturbed".to_string()));
} }
// Step 2. Let clonedRequest be the result of cloning this’s request. // Step 2. Let clonedRequest be the result of cloning this’s request.
@ -750,16 +755,18 @@ impl RequestMethods<crate::DomTypeHolder> for Request {
} }
impl BodyMixin for Request { impl BodyMixin for Request {
fn is_disturbed(&self) -> bool { fn is_body_used(&self) -> bool {
let body_stream = self.body_stream.get(); let body_stream = self.body_stream.get();
body_stream body_stream
.as_ref() .as_ref()
.is_some_and(|stream| stream.is_disturbed()) .is_some_and(|stream| stream.is_disturbed())
} }
fn is_locked(&self) -> bool { fn is_unusable(&self) -> bool {
let body_stream = self.body_stream.get(); let body_stream = self.body_stream.get();
body_stream.is_some_and(|stream| stream.is_locked()) body_stream
.as_ref()
.is_some_and(|stream| stream.is_disturbed() || stream.is_locked())
} }
fn body(&self) -> Option<DomRoot<ReadableStream>> { fn body(&self) -> Option<DomRoot<ReadableStream>> {

35
components/script/dom/response.rs
View file

@ -98,19 +98,31 @@ impl Response {
body.error_native(error, can_gc); body.error_native(error, can_gc);
} }
} }
}
impl BodyMixin for Response { pub(crate) fn is_disturbed(&self) -> bool {
fn is_disturbed(&self) -> bool { let body_stream = self.body_stream.get();
self.body_stream body_stream
.get() .as_ref()
.is_some_and(|stream| stream.is_disturbed()) .is_some_and(|stream| stream.is_disturbed())
} }
fn is_locked(&self) -> bool { pub(crate) fn is_locked(&self) -> bool {
let body_stream = self.body_stream.get();
body_stream
.as_ref()
.is_some_and(|stream| stream.is_locked())
}
}
impl BodyMixin for Response {
fn is_body_used(&self) -> bool {
self.is_disturbed()
}
fn is_unusable(&self) -> bool {
self.body_stream self.body_stream
.get() .get()
.is_some_and(|stream| stream.is_locked()) .is_some_and(|stream| stream.is_disturbed() || stream.is_locked())
} }
fn body(&self) -> Option<DomRoot<ReadableStream>> { fn body(&self) -> Option<DomRoot<ReadableStream>> {
@ -300,7 +312,7 @@ impl ResponseMethods<crate::DomTypeHolder> for Response {
/// <https://fetch.spec.whatwg.org/#dom-response-clone> /// <https://fetch.spec.whatwg.org/#dom-response-clone>
fn Clone(&self, can_gc: CanGc) -> Fallible<DomRoot<Response>> { fn Clone(&self, can_gc: CanGc) -> Fallible<DomRoot<Response>> {
// Step 1 // Step 1
if self.is_locked() || self.is_disturbed() { if self.is_unusable() {
return Err(Error::Type("cannot clone a disturbed response".to_string())); return Err(Error::Type("cannot clone a disturbed response".to_string()));
} }
@ -341,8 +353,7 @@ impl ResponseMethods<crate::DomTypeHolder> for Response {
/// <https://fetch.spec.whatwg.org/#dom-body-bodyused> /// <https://fetch.spec.whatwg.org/#dom-body-bodyused>
fn BodyUsed(&self) -> bool { fn BodyUsed(&self) -> bool {
// bodyUsed returns true only if body is non-null !self.is_body_empty.get() && self.is_body_used()
!self.is_body_empty() && self.is_disturbed()
} }
/// <https://fetch.spec.whatwg.org/#dom-body-body> /// <https://fetch.spec.whatwg.org/#dom-body-body>
@ -537,8 +548,4 @@ impl Response {
stream_consumer.stream_end(); stream_consumer.stream_end();
} }
} }
pub(crate) fn is_body_empty(&self) -> bool {
self.is_body_empty.get()
}
} }

1
components/script/script_runtime.rs
View file

@ -59,7 +59,6 @@ use script_bindings::script_runtime::{mark_runtime_dead, runtime_is_alive};
use servo_config::{opts, pref}; use servo_config::{opts, pref};
use style::thread_state::{self, ThreadState}; use style::thread_state::{self, ThreadState};
use crate::body::BodyMixin;
use crate::dom::bindings::codegen::Bindings::PromiseBinding::PromiseJobCallback; use crate::dom::bindings::codegen::Bindings::PromiseBinding::PromiseJobCallback;
use crate::dom::bindings::codegen::Bindings::ResponseBinding::Response_Binding::ResponseMethods; use crate::dom::bindings::codegen::Bindings::ResponseBinding::Response_Binding::ResponseMethods;
use crate::dom::bindings::codegen::Bindings::ResponseBinding::ResponseType as DOMResponseType; use crate::dom::bindings::codegen::Bindings::ResponseBinding::ResponseType as DOMResponseType;

6
tests/wpt/meta/fetch/api/abort/general.any.js.ini vendored
View file

@ -5,9 +5,6 @@
expected: ERROR expected: ERROR
[general.any.html] [general.any.html]
[Request is still 'used' if signal is aborted before fetching]
expected: FAIL
[response.arrayBuffer() rejects if already aborted] [response.arrayBuffer() rejects if already aborted]
expected: FAIL expected: FAIL
@ -31,9 +28,6 @@
[general.any.worker.html] [general.any.worker.html]
[Request is still 'used' if signal is aborted before fetching]
expected: FAIL
[response.arrayBuffer() rejects if already aborted] [response.arrayBuffer() rejects if already aborted]
expected: FAIL expected: FAIL

24
tests/wpt/meta/fetch/api/abort/request.any.js.ini vendored
View file

@ -1,22 +1,10 @@
[request.any.html] [request.any.html]
[Calling arrayBuffer() on an aborted consumed nonempty request]
expected: FAIL
[Calling blob() on an aborted consumed nonempty request]
expected: FAIL
[Calling formData() on an aborted request] [Calling formData() on an aborted request]
expected: FAIL expected: FAIL
[Aborting a request after calling formData()] [Aborting a request after calling formData()]
expected: FAIL expected: FAIL
[Calling json() on an aborted consumed nonempty request]
expected: FAIL
[Calling text() on an aborted consumed nonempty request]
expected: FAIL
[request.any.serviceworker.html] [request.any.serviceworker.html]
expected: ERROR expected: ERROR
@ -25,20 +13,8 @@
expected: ERROR expected: ERROR
[request.any.worker.html] [request.any.worker.html]
[Calling arrayBuffer() on an aborted consumed nonempty request]
expected: FAIL
[Calling blob() on an aborted consumed nonempty request]
expected: FAIL
[Calling formData() on an aborted request] [Calling formData() on an aborted request]
expected: FAIL expected: FAIL
[Aborting a request after calling formData()] [Aborting a request after calling formData()]
expected: FAIL expected: FAIL
[Calling json() on an aborted consumed nonempty request]
expected: FAIL
[Calling text() on an aborted consumed nonempty request]
expected: FAIL