Refuse to provide partial response from earlier ranged request to API that did not make a range request (#36227)

Part of https://github.com/servo/servo/issues/33616

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-03-31 12:34:32 +02:00 committed by GitHub
parent 272da2981d
commit bc898da5de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 51 additions and 13 deletions

View file

@ -14,7 +14,7 @@ use crossbeam_channel::Sender;
use devtools_traits::DevtoolsControlMsg; use devtools_traits::DevtoolsControlMsg;
use embedder_traits::resources::{self, Resource}; use embedder_traits::resources::{self, Resource};
use headers::{AccessControlExposeHeaders, ContentType, HeaderMapExt}; use headers::{AccessControlExposeHeaders, ContentType, HeaderMapExt};
use http::header::{self, HeaderMap, HeaderName}; use http::header::{self, HeaderMap, HeaderName, RANGE};
use http::{HeaderValue, Method, StatusCode}; use http::{HeaderValue, Method, StatusCode};
use ipc_channel::ipc; use ipc_channel::ipc;
use log::{debug, trace, warn}; use log::{debug, trace, warn};
@ -47,6 +47,9 @@ use crate::protocols::ProtocolRegistry;
use crate::request_interceptor::RequestInterceptor; use crate::request_interceptor::RequestInterceptor;
use crate::subresource_integrity::is_response_integrity_valid; use crate::subresource_integrity::is_response_integrity_valid;
const PARTIAL_RESPONSE_TO_NON_RANGE_REQUEST_ERROR: &str = "Refusing to provide partial response\
from earlier ranged request to API that did not make a range request";
pub type Target<'a> = &'a mut (dyn FetchTaskTarget + Send); pub type Target<'a> = &'a mut (dyn FetchTaskTarget + Send);
#[derive(Clone, Deserialize, Serialize)] #[derive(Clone, Deserialize, Serialize)]
@ -484,21 +487,28 @@ pub async fn main_fetch(
.get_network_error() .get_network_error()
.cloned() .cloned()
.map(Response::network_error); .map(Response::network_error);
// Step 15. Let internalResponse be response, if response is a network error;
// otherwise responses internal response.
let response_type = response.response_type.clone(); // Needed later after the mutable borrow
let internal_response = if let Some(error_response) = network_error_response.as_mut() { let internal_response = if let Some(error_response) = network_error_response.as_mut() {
error_response error_response
} else { } else {
response.actual_response_mut() response.actual_response_mut()
}; };
// Step 16. // Step 16. If internalResponses URL list is empty, then set it to a clone of requests URL list.
if internal_response.url_list.is_empty() { if internal_response.url_list.is_empty() {
internal_response.url_list.clone_from(&request.url_list) internal_response.url_list.clone_from(&request.url_list)
} }
// Step 17. // Step 19. If response is not a network error and any of the following returns blocked
// TODO: handle blocking as mixed content. // TODO: * should internalResponse to request be blocked as mixed content
// TODO: handle blocking by content security policy. // TODO: * should internalResponse to request be blocked by Content Security Policy
let blocked_error_response; // * should internalResponse to request be blocked due to its MIME type
// * should internalResponse to request be blocked due to nosniff
let mut blocked_error_response;
let internal_response = if should_replace_with_nosniff_error { let internal_response = if should_replace_with_nosniff_error {
// Defer rebinding result // Defer rebinding result
blocked_error_response = blocked_error_response =
@ -513,9 +523,27 @@ pub async fn main_fetch(
internal_response internal_response
}; };
// Step 18. // Step 20. If responses type is "opaque", internalResponses status is 206, internalResponses
// We check `internal_response` since we did not mutate `response` // range-requested flag is set, and requests header list does not contain `Range`, then set
// in the previous step. // response and internalResponse to a network error.
let internal_response = if response_type == ResponseType::Opaque &&
internal_response.status.code() == StatusCode::PARTIAL_CONTENT &&
internal_response.range_requested &&
!request.headers.contains_key(RANGE)
{
// Defer rebinding result
blocked_error_response = Response::network_error(NetworkError::Internal(
PARTIAL_RESPONSE_TO_NON_RANGE_REQUEST_ERROR.into(),
));
&blocked_error_response
} else {
internal_response
};
// Step 21. If response is not a network error and either requests method is `HEAD` or `CONNECT`,
// or internalResponses status is a null body status, set internalResponses body to null and
// disregard any enqueuing toward it (if any).
// NOTE: We check `internal_response` since we did not mutate `response` in the previous steps.
let not_network_error = !response_is_network_error && !internal_response.is_network_error(); let not_network_error = !response_is_network_error && !internal_response.is_network_error();
if not_network_error && if not_network_error &&
(is_null_body_status(&internal_response.status) || (is_null_body_status(&internal_response.status) ||

View file

@ -26,7 +26,7 @@ use headers::{
}; };
use http::header::{ use http::header::{
self, ACCEPT, AUTHORIZATION, CONTENT_ENCODING, CONTENT_LANGUAGE, CONTENT_LOCATION, self, ACCEPT, AUTHORIZATION, CONTENT_ENCODING, CONTENT_LANGUAGE, CONTENT_LOCATION,
CONTENT_TYPE, HeaderValue, CONTENT_TYPE, HeaderValue, RANGE,
}; };
use http::{HeaderMap, Method, Request as HyperRequest, StatusCode}; use http::{HeaderMap, Method, Request as HyperRequest, StatusCode};
use http_body_util::combinators::BoxBody; use http_body_util::combinators::BoxBody;
@ -1123,7 +1123,7 @@ pub async fn http_redirect_fetch(
fetch_response fetch_response
} }
/// [HTTP network or cache fetch](https://fetch.spec.whatwg.org#http-network-or-cache-fetch) /// [HTTP network or cache fetch](https://fetch.spec.whatwg.org/#concept-http-network-or-cache-fetch)
#[async_recursion] #[async_recursion]
async fn http_network_or_cache_fetch( async fn http_network_or_cache_fetch(
fetch_params: &mut FetchParams, fetch_params: &mut FetchParams,
@ -1598,8 +1598,12 @@ async fn http_network_or_cache_fetch(
} }
// TODO(#33616): Step 11. Set responses URL list to a clone of httpRequests URL list. // TODO(#33616): Step 11. Set responses URL list to a clone of httpRequests URL list.
// TODO(#33616): Step 12. If httpRequests header list contains `Range`,
// then set responses range-requested flag. // Step 12. If httpRequests header list contains `Range`, then set responses range-requested flag.
if http_request.headers.contains_key(RANGE) {
response.range_requested = true;
}
// TODO(#33616): Step 13 Set responses request-includes-credentials to includeCredentials. // TODO(#33616): Step 13 Set responses request-includes-credentials to includeCredentials.
// Step 14. If responses status is 401, httpRequests response tainting is not "cors", // Step 14. If responses status is 401, httpRequests response tainting is not "cors",

View file

@ -54,6 +54,7 @@ impl ProtocolHandler for BlobProtocolHander {
response.status = HttpStatus::default(); response.status = HttpStatus::default();
if is_range_request { if is_range_request {
response.range_requested = true;
partial_content(&mut response); partial_content(&mut response);
} }

View file

@ -120,6 +120,9 @@ pub struct Response {
/// track network metrics /// track network metrics
#[ignore_malloc_size_of = "Mutex heap size undefined"] #[ignore_malloc_size_of = "Mutex heap size undefined"]
pub resource_timing: Arc<Mutex<ResourceFetchTiming>>, pub resource_timing: Arc<Mutex<ResourceFetchTiming>>,
/// <https://fetch.spec.whatwg.org/#concept-response-range-requested-flag>
pub range_requested: bool,
} }
impl Response { impl Response {
@ -142,6 +145,7 @@ impl Response {
return_internal: true, return_internal: true,
aborted: Arc::new(AtomicBool::new(false)), aborted: Arc::new(AtomicBool::new(false)),
resource_timing: Arc::new(Mutex::new(resource_timing)), resource_timing: Arc::new(Mutex::new(resource_timing)),
range_requested: false,
} }
} }
@ -175,6 +179,7 @@ impl Response {
resource_timing: Arc::new(Mutex::new(ResourceFetchTiming::new( resource_timing: Arc::new(Mutex::new(ResourceFetchTiming::new(
ResourceTimingType::Error, ResourceTimingType::Error,
))), ))),
range_requested: false,
} }
} }