mirror of
https://github.com/servo/servo.git
synced 2025-08-03 12:40:06 +01:00
Auto merge of #29627 - CYBAI:respect-headers-ct, r=mukilan
Respect MIME type from headers instead of caching it Based on the spec, > A [Request](https://fetch.spec.whatwg.org/#request) object’s [MIME type](https://fetch.spec.whatwg.org/#concept-body-mime-type) is to return the result of [extracting a MIME type](https://fetch.spec.whatwg.org/#concept-header-extract-mime-type) from its [request](https://fetch.spec.whatwg.org/#concept-request-request)’s [header list](https://fetch.spec.whatwg.org/#concept-request-header-list). request and response should always return the MIME type from their header list. As we're currently caching the mime type directly, this PR will help to remove the caching field and always retrieve from headers instead. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable) - [x] There are tests for these changes
This commit is contained in:
bors-servo
GitHub
commit
4251159513
4 changed files with 85 additions and 148 deletions
|
|
@ -272,10 +272,6 @@ impl Headers {
|
|||
self.guard.get()
|
||||
}
|
||||
|
||||
pub fn empty_header_list(&self) {
|
||||
*self.header_list.borrow_mut() = HyperHeaders::new();
|
||||
}
|
||||
|
||||
pub fn set_headers(&self, hyper_headers: HyperHeaders) {
|
||||
*self.header_list.borrow_mut() = hyper_headers;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -50,7 +50,6 @@ pub struct Request {
|
|||
request: DomRefCell<NetTraitsRequest>,
|
||||
body_stream: MutNullableDom<ReadableStream>,
|
||||
headers: MutNullableDom<Headers>,
|
||||
mime_type: DomRefCell<Vec<u8>>,
|
||||
}
|
||||
|
||||
impl Request {
|
||||
|
|
@ -60,7 +59,6 @@ impl Request {
|
|||
request: DomRefCell::new(net_request_from_global(global, url)),
|
||||
body_stream: MutNullableDom::new(None),
|
||||
headers: Default::default(),
|
||||
mime_type: DomRefCell::new("".to_string().into_bytes()),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -81,69 +79,71 @@ impl Request {
|
|||
// Step 2
|
||||
let mut fallback_mode: Option<NetTraitsRequestMode> = None;
|
||||
|
||||
// Step 3
|
||||
// FIXME(cybai): As the spec changed in https://github.com/whatwg/fetch/pull/1153,
|
||||
// we will need to change the default value of credentials for
|
||||
// NetTraitsRequest and then remove fallback here.
|
||||
let mut fallback_credentials: Option<NetTraitsRequestCredentials> = None;
|
||||
|
||||
// Step 4
|
||||
// Step 3
|
||||
let base_url = global.api_base_url();
|
||||
|
||||
// Step 5 TODO: "Let signal be null."
|
||||
// Step 4 TODO: "Let signal be null."
|
||||
|
||||
match input {
|
||||
// Step 6
|
||||
// Step 5
|
||||
RequestInfo::USVString(USVString(ref usv_string)) => {
|
||||
// Step 6.1
|
||||
// Step 5.1
|
||||
let parsed_url = base_url.join(&usv_string);
|
||||
// Step 6.2
|
||||
// Step 5.2
|
||||
if parsed_url.is_err() {
|
||||
return Err(Error::Type("Url could not be parsed".to_string()));
|
||||
}
|
||||
// Step 6.3
|
||||
// Step 5.3
|
||||
let url = parsed_url.unwrap();
|
||||
if includes_credentials(&url) {
|
||||
return Err(Error::Type("Url includes credentials".to_string()));
|
||||
}
|
||||
// Step 6.4
|
||||
// Step 5.4
|
||||
temporary_request = net_request_from_global(global, url);
|
||||
// Step 6.5
|
||||
// Step 5.5
|
||||
fallback_mode = Some(NetTraitsRequestMode::CorsMode);
|
||||
// Step 6.6
|
||||
// FIXME(cybai): remove this line when we can remove the fallback of credentials
|
||||
fallback_credentials = Some(NetTraitsRequestCredentials::CredentialsSameOrigin);
|
||||
},
|
||||
// Step 7
|
||||
// Step 6
|
||||
RequestInfo::Request(ref input_request) => {
|
||||
// This looks like Step 38
|
||||
// TODO do this in the right place to not mask other errors
|
||||
if request_is_disturbed(input_request) || request_is_locked(input_request) {
|
||||
return Err(Error::Type("Input is disturbed or locked".to_string()));
|
||||
}
|
||||
// Step 7.1
|
||||
// Step 6.1
|
||||
temporary_request = input_request.request.borrow().clone();
|
||||
// Step 7.2 TODO: "Set signal to input's signal."
|
||||
// Step 6.2 TODO: "Set signal to input's signal."
|
||||
},
|
||||
}
|
||||
|
||||
// Step 8
|
||||
// Step 7
|
||||
// TODO: `entry settings object` is not implemented yet.
|
||||
let origin = base_url.origin();
|
||||
|
||||
// Step 9
|
||||
// Step 8
|
||||
let mut window = Window::Client;
|
||||
|
||||
// Step 10
|
||||
// Step 9
|
||||
// TODO: `environment settings object` is not implemented in Servo yet.
|
||||
|
||||
// Step 11
|
||||
// Step 10
|
||||
if !init.window.handle().is_null_or_undefined() {
|
||||
return Err(Error::Type("Window is present and is not null".to_string()));
|
||||
}
|
||||
|
||||
// Step 12
|
||||
// Step 11
|
||||
if !init.window.handle().is_undefined() {
|
||||
window = Window::NoWindow;
|
||||
}
|
||||
|
||||
// Step 13
|
||||
// Step 12
|
||||
let mut request: NetTraitsRequest;
|
||||
request = net_request_from_global(global, temporary_request.current_url());
|
||||
request.method = temporary_request.method;
|
||||
|
|
@ -160,7 +160,7 @@ impl Request {
|
|||
request.redirect_mode = temporary_request.redirect_mode;
|
||||
request.integrity_metadata = temporary_request.integrity_metadata;
|
||||
|
||||
// Step 14
|
||||
// Step 13
|
||||
if init.body.is_some() ||
|
||||
init.cache.is_some() ||
|
||||
init.credentials.is_some() ||
|
||||
|
|
@ -173,33 +173,33 @@ impl Request {
|
|||
init.referrerPolicy.is_some() ||
|
||||
!init.window.handle().is_undefined()
|
||||
{
|
||||
// Step 14.1
|
||||
// Step 13.1
|
||||
if request.mode == NetTraitsRequestMode::Navigate {
|
||||
request.mode = NetTraitsRequestMode::SameOrigin;
|
||||
}
|
||||
// Step 14.2 TODO: "Unset request's reload-navigation flag."
|
||||
// Step 14.3 TODO: "Unset request's history-navigation flag."
|
||||
// Step 14.4
|
||||
// Step 13.2 TODO: "Unset request's reload-navigation flag."
|
||||
// Step 13.3 TODO: "Unset request's history-navigation flag."
|
||||
// Step 13.4
|
||||
request.referrer = global.get_referrer();
|
||||
// Step 14.5
|
||||
// Step 13.5
|
||||
request.referrer_policy = None;
|
||||
}
|
||||
|
||||
// Step 15
|
||||
// Step 14
|
||||
if let Some(init_referrer) = init.referrer.as_ref() {
|
||||
// Step 15.1
|
||||
// Step 14.1
|
||||
let ref referrer = init_referrer.0;
|
||||
// Step 15.2
|
||||
// Step 14.2
|
||||
if referrer.is_empty() {
|
||||
request.referrer = NetTraitsRequestReferrer::NoReferrer;
|
||||
} else {
|
||||
// Step 15.3.1
|
||||
// Step 14.3.1
|
||||
let parsed_referrer = base_url.join(referrer);
|
||||
// Step 15.3.2
|
||||
// Step 14.3.2
|
||||
if parsed_referrer.is_err() {
|
||||
return Err(Error::Type("Failed to parse referrer url".to_string()));
|
||||
}
|
||||
// Step 15.3.3
|
||||
// Step 14.3.3
|
||||
if let Ok(parsed_referrer) = parsed_referrer {
|
||||
if (parsed_referrer.cannot_be_a_base() &&
|
||||
parsed_referrer.scheme() == "about" &&
|
||||
|
|
@ -208,55 +208,54 @@ impl Request {
|
|||
{
|
||||
request.referrer = global.get_referrer();
|
||||
} else {
|
||||
// Step 15.3.4
|
||||
// Step 14.3.4
|
||||
request.referrer = NetTraitsRequestReferrer::ReferrerUrl(parsed_referrer);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Step 16
|
||||
// Step 15
|
||||
if let Some(init_referrerpolicy) = init.referrerPolicy.as_ref() {
|
||||
let init_referrer_policy = init_referrerpolicy.clone().into();
|
||||
request.referrer_policy = Some(init_referrer_policy);
|
||||
}
|
||||
|
||||
// Step 17
|
||||
// Step 16
|
||||
let mode = init
|
||||
.mode
|
||||
.as_ref()
|
||||
.map(|m| m.clone().into())
|
||||
.or(fallback_mode);
|
||||
|
||||
// Step 18
|
||||
// Step 17
|
||||
if let Some(NetTraitsRequestMode::Navigate) = mode {
|
||||
return Err(Error::Type("Request mode is Navigate".to_string()));
|
||||
}
|
||||
|
||||
// Step 19
|
||||
// Step 18
|
||||
if let Some(m) = mode {
|
||||
request.mode = m;
|
||||
}
|
||||
|
||||
// Step 20
|
||||
// Step 19
|
||||
let credentials = init
|
||||
.credentials
|
||||
.as_ref()
|
||||
.map(|m| m.clone().into())
|
||||
.or(fallback_credentials);
|
||||
|
||||
// Step 21
|
||||
if let Some(c) = credentials {
|
||||
request.credentials_mode = c;
|
||||
}
|
||||
|
||||
// Step 22
|
||||
// Step 20
|
||||
if let Some(init_cache) = init.cache.as_ref() {
|
||||
let cache = init_cache.clone().into();
|
||||
request.cache_mode = cache;
|
||||
}
|
||||
|
||||
// Step 23
|
||||
// Step 21
|
||||
if request.cache_mode == NetTraitsRequestCache::OnlyIfCached {
|
||||
if request.mode != NetTraitsRequestMode::SameOrigin {
|
||||
return Err(Error::Type(
|
||||
|
|
@ -265,44 +264,46 @@ impl Request {
|
|||
}
|
||||
}
|
||||
|
||||
// Step 24
|
||||
// Step 22
|
||||
if let Some(init_redirect) = init.redirect.as_ref() {
|
||||
let redirect = init_redirect.clone().into();
|
||||
request.redirect_mode = redirect;
|
||||
}
|
||||
|
||||
// Step 25
|
||||
// Step 23
|
||||
if let Some(init_integrity) = init.integrity.as_ref() {
|
||||
let integrity = init_integrity.clone().to_string();
|
||||
request.integrity_metadata = integrity;
|
||||
}
|
||||
|
||||
// Step 26 TODO: "If init["keepalive"] exists..."
|
||||
// Step 24 TODO: "If init["keepalive"] exists..."
|
||||
|
||||
// Step 27.1
|
||||
// Step 25.1
|
||||
if let Some(init_method) = init.method.as_ref() {
|
||||
// Step 27.2
|
||||
if !is_method(&init_method) {
|
||||
return Err(Error::Type("Method is not a method".to_string()));
|
||||
}
|
||||
// Step 25.2
|
||||
if is_forbidden_method(&init_method) {
|
||||
return Err(Error::Type("Method is forbidden".to_string()));
|
||||
}
|
||||
// Step 27.3
|
||||
// Step 25.3
|
||||
let method = match init_method.as_str() {
|
||||
Some(s) => normalize_method(s)
|
||||
.map_err(|e| Error::Type(format!("Method is not valid: {:?}", e)))?,
|
||||
None => return Err(Error::Type("Method is not a valid UTF8".to_string())),
|
||||
};
|
||||
// Step 27.4
|
||||
// Step 25.4
|
||||
request.method = method;
|
||||
}
|
||||
|
||||
// Step 28 TODO: "If init["signal"] exists..."
|
||||
// Step 26 TODO: "If init["signal"] exists..."
|
||||
// Step 27 TODO: "If init["priority"] exists..."
|
||||
|
||||
// Step 29
|
||||
// Step 28
|
||||
let r = Request::from_net_request(global, request);
|
||||
|
||||
// Step 29 TODO: "Set this's signal to new AbortSignal object..."
|
||||
// Step 30 TODO: "If signal is not null..."
|
||||
|
||||
// Step 31
|
||||
|
|
@ -310,7 +311,7 @@ impl Request {
|
|||
// hasn't had any other way to initialize its headers
|
||||
r.headers.or_init(|| Headers::for_request(&r.global()));
|
||||
|
||||
// Step 32 - but spec says this should only be when non-empty init?
|
||||
// Step 33 - but spec says this should only be when non-empty init?
|
||||
let headers_copy = init
|
||||
.headers
|
||||
.as_ref()
|
||||
|
|
@ -323,30 +324,30 @@ impl Request {
|
|||
},
|
||||
});
|
||||
|
||||
// Step 32.3
|
||||
// Step 33.3
|
||||
// We cannot empty `r.Headers().header_list` because
|
||||
// we would undo the Step 27 above. One alternative is to set
|
||||
// we would undo the Step 25 above. One alternative is to set
|
||||
// `headers_copy` as a deep copy of `r.Headers()`. However,
|
||||
// `r.Headers()` is a `DomRoot<T>`, and therefore it is difficult
|
||||
// to obtain a mutable reference to `r.Headers()`. Without the
|
||||
// mutable reference, we cannot mutate `r.Headers()` to be the
|
||||
// deep copied headers in Step 27.
|
||||
// deep copied headers in Step 25.
|
||||
|
||||
// Step 32.4
|
||||
// Step 32
|
||||
if r.request.borrow().mode == NetTraitsRequestMode::NoCors {
|
||||
let borrowed_request = r.request.borrow();
|
||||
// Step 32.4.1
|
||||
// Step 32.1
|
||||
if !is_cors_safelisted_method(&borrowed_request.method) {
|
||||
return Err(Error::Type(
|
||||
"The mode is 'no-cors' but the method is not a cors-safelisted method"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
// Step 32.4.2
|
||||
// Step 32.2
|
||||
r.Headers().set_guard(Guard::RequestNoCors);
|
||||
}
|
||||
|
||||
// Step 32.5
|
||||
// Step 33.5
|
||||
match headers_copy {
|
||||
None => {
|
||||
// This is equivalent to the specification's concept of
|
||||
|
|
@ -360,11 +361,11 @@ impl Request {
|
|||
Some(headers_copy) => r.Headers().fill(Some(headers_copy))?,
|
||||
}
|
||||
|
||||
// Step 32.5-6 depending on how we got here
|
||||
// Step 33.5 depending on how we got here
|
||||
// Copy the headers list onto the headers of net_traits::Request
|
||||
r.request.borrow_mut().headers = r.Headers().get_headers_list();
|
||||
|
||||
// Step 33
|
||||
// Step 34
|
||||
let mut input_body = if let RequestInfo::Request(ref mut input_request) = input {
|
||||
let mut input_request_request = input_request.request.borrow_mut();
|
||||
input_request_request.body.take()
|
||||
|
|
@ -372,7 +373,7 @@ impl Request {
|
|||
None
|
||||
};
|
||||
|
||||
// Step 34
|
||||
// Step 35
|
||||
if let Some(init_body_option) = init.body.as_ref() {
|
||||
if init_body_option.is_some() || input_body.is_some() {
|
||||
let req = r.request.borrow();
|
||||
|
|
@ -393,14 +394,14 @@ impl Request {
|
|||
}
|
||||
}
|
||||
|
||||
// Step 35-36
|
||||
// Step 36-37
|
||||
if let Some(Some(ref init_body)) = init.body {
|
||||
// Step 36.2 TODO "If init["keepalive"] exists and is true..."
|
||||
// Step 37.1 TODO "If init["keepalive"] exists and is true..."
|
||||
|
||||
// Step 36.3
|
||||
// Step 37.2
|
||||
let mut extracted_body = init_body.extract(global)?;
|
||||
|
||||
// Step 36.4
|
||||
// Step 37.3
|
||||
if let Some(contents) = extracted_body.content_type.take() {
|
||||
let ct_header_name = b"Content-Type";
|
||||
if !r
|
||||
|
|
@ -414,6 +415,7 @@ impl Request {
|
|||
ByteString::new(ct_header_val.to_vec()),
|
||||
)?;
|
||||
|
||||
// Step 37.4
|
||||
// In Servo r.Headers's header list isn't a pointer to
|
||||
// the same actual list as r.request's, and so we need to
|
||||
// append to both lists to keep them in sync.
|
||||
|
|
@ -431,21 +433,16 @@ impl Request {
|
|||
input_body = Some(net_body);
|
||||
}
|
||||
|
||||
// Step 37 "TODO if body is non-null and body's source is null..."
|
||||
// Step 38 is done earlier
|
||||
|
||||
// Step 39 "TODO if body is non-null and body's source is null..."
|
||||
// This looks like where we need to set the use-preflight flag
|
||||
// if the request has a body and nothing else has set the flag.
|
||||
|
||||
// Step 38 is done earlier
|
||||
|
||||
// Step 39
|
||||
// TODO: `ReadableStream` object is not implemented in Servo yet.
|
||||
|
||||
// Step 40
|
||||
r.request.borrow_mut().body = input_body;
|
||||
// Step 40 is done earlier
|
||||
|
||||
// Step 41
|
||||
let extracted_mime_type = r.Headers().extract_mime_type();
|
||||
*r.mime_type.borrow_mut() = extracted_mime_type;
|
||||
r.request.borrow_mut().body = input_body;
|
||||
|
||||
// Step 42
|
||||
Ok(r)
|
||||
|
|
@ -462,7 +459,6 @@ impl Request {
|
|||
fn clone_from(r: &Request) -> Fallible<DomRoot<Request>> {
|
||||
let req = r.request.borrow();
|
||||
let url = req.url();
|
||||
let mime_type = r.mime_type.borrow().clone();
|
||||
let headers_guard = r.Headers().get_guard();
|
||||
let r_clone = Request::new(&r.global(), url);
|
||||
r_clone.request.borrow_mut().pipeline_id = req.pipeline_id;
|
||||
|
|
@ -471,7 +467,6 @@ impl Request {
|
|||
borrowed_r_request.origin = req.origin.clone();
|
||||
}
|
||||
*r_clone.request.borrow_mut() = req.clone();
|
||||
*r_clone.mime_type.borrow_mut() = mime_type;
|
||||
r_clone.Headers().copy_from_headers(r.Headers())?;
|
||||
r_clone.Headers().set_guard(headers_guard);
|
||||
Ok(r_clone)
|
||||
|
|
@ -682,7 +677,8 @@ impl BodyMixin for Request {
|
|||
}
|
||||
|
||||
fn get_mime_type(&self) -> Vec<u8> {
|
||||
self.mime_type.borrow().clone()
|
||||
let headers = self.Headers();
|
||||
headers.extract_mime_type()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -37,7 +37,6 @@ use url::Position;
|
|||
pub struct Response {
|
||||
reflector_: Reflector,
|
||||
headers_reflector: MutNullableDom<Headers>,
|
||||
mime_type: DomRefCell<Vec<u8>>,
|
||||
/// `None` can be considered a StatusCode of `0`.
|
||||
#[ignore_malloc_size_of = "Defined in hyper"]
|
||||
status: DomRefCell<Option<StatusCode>>,
|
||||
|
|
@ -62,7 +61,6 @@ impl Response {
|
|||
Response {
|
||||
reflector_: Reflector::new(),
|
||||
headers_reflector: Default::default(),
|
||||
mime_type: DomRefCell::new("".to_string().into_bytes()),
|
||||
status: DomRefCell::new(Some(StatusCode::OK)),
|
||||
raw_status: DomRefCell::new(Some((200, b"".to_vec()))),
|
||||
response_type: DomRefCell::new(DOMResponseType::Default),
|
||||
|
|
@ -79,6 +77,7 @@ impl Response {
|
|||
reflect_dom_object(Box::new(Response::new_inherited(global)), global)
|
||||
}
|
||||
|
||||
// https://fetch.spec.whatwg.org/#initialize-a-response
|
||||
pub fn Constructor(
|
||||
global: &GlobalScope,
|
||||
body: Option<BodyInit>,
|
||||
|
|
@ -100,34 +99,29 @@ impl Response {
|
|||
));
|
||||
}
|
||||
|
||||
// Step 3
|
||||
let r = Response::new(global);
|
||||
|
||||
// Step 4
|
||||
// Step 3
|
||||
*r.status.borrow_mut() = Some(StatusCode::from_u16(init.status).unwrap());
|
||||
|
||||
// Step 5
|
||||
// Step 4
|
||||
*r.raw_status.borrow_mut() = Some((init.status, init.statusText.clone().into()));
|
||||
|
||||
// Step 6
|
||||
// Step 5
|
||||
if let Some(ref headers_member) = init.headers {
|
||||
// Step 6.1
|
||||
r.Headers().empty_header_list();
|
||||
|
||||
// Step 6.2
|
||||
r.Headers().fill(Some(headers_member.clone()))?;
|
||||
}
|
||||
|
||||
// Step 7
|
||||
// Step 6
|
||||
if let Some(ref body) = body {
|
||||
// Step 7.1
|
||||
// Step 6.1
|
||||
if is_null_body_status(init.status) {
|
||||
return Err(Error::Type(
|
||||
"Body is non-null but init's status member is a null body status".to_string(),
|
||||
));
|
||||
};
|
||||
|
||||
// Step 7.3
|
||||
// Step 6.2
|
||||
let ExtractedBody {
|
||||
stream,
|
||||
total_bytes: _,
|
||||
|
|
@ -137,7 +131,7 @@ impl Response {
|
|||
|
||||
r.body_stream.set(Some(&*stream));
|
||||
|
||||
// Step 7.4
|
||||
// Step 6.3
|
||||
if let Some(content_type_contents) = content_type {
|
||||
if !r
|
||||
.Headers()
|
||||
|
|
@ -152,16 +146,6 @@ impl Response {
|
|||
};
|
||||
}
|
||||
|
||||
// Step 8
|
||||
*r.mime_type.borrow_mut() = r.Headers().extract_mime_type();
|
||||
|
||||
// Step 9
|
||||
// TODO: `entry settings object` is not implemented in Servo yet.
|
||||
|
||||
// Step 10
|
||||
// TODO: Write this step once Promises are merged in
|
||||
|
||||
// Step 11
|
||||
Ok(r)
|
||||
}
|
||||
|
||||
|
|
@ -242,7 +226,8 @@ impl BodyMixin for Response {
|
|||
}
|
||||
|
||||
fn get_mime_type(&self) -> Vec<u8> {
|
||||
self.mime_type.borrow().clone()
|
||||
let headers = self.Headers();
|
||||
headers.extract_mime_type()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -404,7 +389,6 @@ impl Response {
|
|||
Some(hyper_headers) => hyper_headers.into_inner(),
|
||||
None => HyperHeaders::new(),
|
||||
});
|
||||
*self.mime_type.borrow_mut() = self.Headers().extract_mime_type();
|
||||
}
|
||||
|
||||
pub fn set_raw_status(&self, status: Option<(u16, Vec<u8>)>) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue