Allow range requests to blob URLs with negative offsets

components/net/fetch/methods.rs

@@ -4,7 +4,7 @@
 
 use crate::data_loader::decode;
 use crate::fetch::cors_cache::CorsCache;
-use crate::filemanager_thread::{fetch_file_in_chunks, FILE_CHUNK_SIZE, FileManager};
+use crate::filemanager_thread::{fetch_file_in_chunks, FileManager, FILE_CHUNK_SIZE};
 use crate::http_loader::{determine_request_referrer, http_fetch, HttpState};
 use crate::http_loader::{set_default_accept, set_default_accept_language};
 use crate::subresource_integrity::is_response_integrity_valid;
@@ -26,7 +26,7 @@ use net_traits::response::{Response, ResponseBody, ResponseType};
 use net_traits::{FetchTaskTarget, NetworkError, ReferrerPolicy, ResourceFetchTiming};
 use servo_url::ServoUrl;
 use std::borrow::Cow;
-use std::fs::{File, Metadata};
+use std::fs::File;
 use std::io::{BufReader, Seek, SeekFrom};
 use std::mem;
 use std::ops::Bound;
@@ -490,10 +490,34 @@ fn wait_for_response(response: &mut Response, target: Target, done_chan: &mut Do
     }
 }
 
+/// Range header start and end values.
+pub enum RangeRequestBounds {
+    /// The range bounds are known and set to final values.
+    Final(RelativePos),
+    /// We need extra information to set the range bounds.
+    /// i.e. buffer or file size.
+    Pending(u64),
+}
+
+impl RangeRequestBounds {
+    pub fn get_final(&self, len: Option<u64>) -> RelativePos {
+        match self {
+            RangeRequestBounds::Final(pos) => pos.clone(),
+            RangeRequestBounds::Pending(offset) => RelativePos::from_opts(
+                if let Some(len) = len {
+                    Some((len - u64::min(len, *offset)) as i64)
+                } else {
+                    Some(0)
+                },
+                None,
+            ),
+        }
+    }
+}
+
 /// Get the range bounds if the `Range` header is present.
-fn get_range_bounds(range: Option<Range>, metadata: Option<Metadata>) -> RelativePos {
+fn get_range_request_bounds(range: Option<Range>) -> RangeRequestBounds {
-    if let Some(ref range) = range
+    if let Some(ref range) = range {
-    {
         let (start, end) = match range
             .iter()
             .collect::<Vec<(Bound<u64>, Bound<u64>)>>()
@@ -505,18 +529,13 @@ fn get_range_bounds(range: Option<Range>, metadata: Option<Metadata>) -> Relativ
                 (start, Some(i64::max(start as i64, end as i64)))
             },
             Some(&(Bound::Unbounded, Bound::Included(offset))) => {
-                if let Some(metadata) = metadata {
+                return RangeRequestBounds::Pending(offset);
-                    // `offset` cannot be bigger than the file size.
-                    (metadata.len() - u64::min(metadata.len(), offset), None)
-                } else {
-                    (0, None)
-                }
             },
             _ => (0, None),
         };
-        RelativePos::from_opts(Some(start as i64), end)
+        RangeRequestBounds::Final(RelativePos::from_opts(Some(start as i64), end))
     } else {
-        RelativePos::from_opts(Some(0), None)
+        RangeRequestBounds::Final(RelativePos::from_opts(Some(0), None))
     }
 }
 
@@ -569,12 +588,17 @@ fn scheme_fetch(
 
                     // Get range bounds (if any) and try to seek to the requested offset.
                     // If seeking fails, bail out with a NetworkError.
-                    let range = get_range_bounds(request.headers.typed_get::<Range>(), file.metadata().ok());
+                    let file_size = match file.metadata() {
+                        Ok(metadata) => Some(metadata.len()),
+                        Err(_) => None,
+                    };
+                    let range = get_range_request_bounds(request.headers.typed_get::<Range>());
+                    let range = range.get_final(file_size);
                     let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file);
                     if reader.seek(SeekFrom::Start(range.start as u64)).is_err() {
                         *response.body.lock().unwrap() = ResponseBody::Done(vec![]);
                         return Response::network_error(NetworkError::Internal(
-                            "Unexpected method for blob".into(),
+                            "Unexpected method for file".into(),
                         ));
                     }
 
@@ -588,11 +612,13 @@ fn scheme_fetch(
                     *done_chan = Some((done_sender.clone(), done_receiver));
                     *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]);
 
-                    fetch_file_in_chunks(done_sender,
+                    fetch_file_in_chunks(
-                                         reader,
+                        done_sender,
-                                         response.body.clone(),
+                        reader,
-                                         context.cancellation_listener.clone(),
+                        response.body.clone(),
-                                         range);
+                        context.cancellation_listener.clone(),
+                        range,
+                    );
 
                     response
                 } else {
@@ -614,12 +640,14 @@ fn scheme_fetch(
                 ));
             }
 
-            let range = get_range_bounds(request.headers.typed_get::<Range>(), None);
+            let range = get_range_request_bounds(request.headers.typed_get::<Range>());
 
             let (id, origin) = match parse_blob_url(&url) {
                 Ok((id, origin)) => (id, origin),
                 Err(()) => {
-                    return Response::network_error(NetworkError::Internal("Invalid blob url".into()));
+                    return Response::network_error(NetworkError::Internal(
+                        "Invalid blob url".into(),
+                    ));
                 },
             };
 
@@ -628,14 +656,15 @@ fn scheme_fetch(
             *done_chan = Some((done_sender.clone(), done_receiver));
             *response.body.lock().unwrap() = ResponseBody::Receiving(vec![]);
             let check_url_validity = true;
-            if let Err(err) = context.filemanager.fetch_file(&done_sender,
+            if let Err(err) = context.filemanager.fetch_file(
-                                                             context.cancellation_listener.clone(),
+                &done_sender,
-                                                             id,
+                context.cancellation_listener.clone(),
-                                                             check_url_validity,
+                id,
-                                                             origin,
+                check_url_validity,
-                                                             &mut response,
+                origin,
-                                                             range)
+                &mut response,
-            {
+                range,
+            ) {
                 let _ = done_sender.send(Data::Done);
                 return Response::network_error(NetworkError::Internal(err));
             };

components/net/filemanager_thread.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use crate::fetch::methods::{CancellationListener, Data};
+use crate::fetch::methods::{CancellationListener, Data, RangeRequestBounds};
 use embedder_traits::{EmbedderMsg, EmbedderProxy, FilterPattern};
 use headers_ext::{ContentLength, ContentType, HeaderMap, HeaderMapExt};
 use http::header::{self, HeaderValue};
@@ -113,7 +113,7 @@ impl FileManager {
         check_url_validity: bool,
         origin: FileOrigin,
         response: &mut Response,
-        range: RelativePos
+        range: RangeRequestBounds,
     ) -> Result<(), String> {
         self.store
             .fetch_blob_buf(
@@ -532,13 +532,14 @@ impl FileManagerStore {
         cancellation_listener: Arc<Mutex<CancellationListener>>,
         id: &Uuid,
         origin_in: &FileOrigin,
-        range: RelativePos,
+        range: RangeRequestBounds,
         check_url_validity: bool,
         response: &mut Response,
     ) -> Result<(), BlobURLStoreError> {
         let file_impl = self.get_impl(id, origin_in, check_url_validity)?;
         match file_impl {
             FileImpl::Memory(buf) => {
+                let range = range.get_final(Some(buf.size));
                 let range = range.to_abs_range(buf.size as usize);
                 let len = range.len() as u64;
 
@@ -567,9 +568,12 @@ impl FileManagerStore {
                 let file = File::open(&metadata.path)
                     .map_err(|e| BlobURLStoreError::External(e.to_string()))?;
 
+                let range = range.get_final(Some(metadata.size));
                 let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file);
                 if reader.seek(SeekFrom::Start(range.start as u64)).is_err() {
-                    return Err(BlobURLStoreError::External("Unexpected method for blob".into()));
+                    return Err(BlobURLStoreError::External(
+                        "Unexpected method for blob".into(),
+                    ));
                 }
 
                 let filename = metadata
@@ -585,11 +589,13 @@ impl FileManagerStore {
                     filename,
                 );
 
-                fetch_file_in_chunks(done_sender.clone(),
+                fetch_file_in_chunks(
-                                     reader,
+                    done_sender.clone(),
-                                     response.body.clone(),
+                    reader,
-                                     cancellation_listener,
+                    response.body.clone(),
-                                     range);
+                    cancellation_listener,
+                    range,
+                );
 
                 Ok(())
             },
@@ -601,7 +607,7 @@ impl FileManagerStore {
                     cancellation_listener,
                     &parent_id,
                     origin_in,
-                    range.slice_inner(&inner_rel_pos),
+                    RangeRequestBounds::Final(range.get_final(None).slice_inner(&inner_rel_pos)),
                     false,
                     response,
                 );
@@ -794,9 +800,7 @@ pub fn fetch_file_in_chunks(
                 let length = {
                     let buffer = reader.fill_buf().unwrap().to_vec();
                     let mut buffer_len = buffer.len();
-                    if let ResponseBody::Receiving(ref mut body) =
+                    if let ResponseBody::Receiving(ref mut body) = *res_body.lock().unwrap() {
-                        *res_body.lock().unwrap()
-                    {
                         let offset = usize::min(
                             {
                                 if let Some(end) = range.end {
@@ -824,9 +828,7 @@ pub fn fetch_file_in_chunks(
                 if length == 0 {
                     let mut body = res_body.lock().unwrap();
                     let completed_body = match *body {
-                        ResponseBody::Receiving(ref mut body) => {
+                        ResponseBody::Receiving(ref mut body) => mem::replace(body, vec![]),
-                            mem::replace(body, vec![])
-                        },
                         _ => vec![],
                     };
                     *body = ResponseBody::Done(completed_body);

tests/wpt/mozilla/meta/MANIFEST.json

@@ -27246,7 +27246,7 @@
    "testharness"
   ],
   "mozilla/range_request_blob_url.html": [
-   "4e8b55d0b836dda9de14faead2ee1d6da354e8a4",
+   "b41f0b5382e3994db35f38b2c358a41b75551fe2",
    "testharness"
   ],
   "mozilla/range_request_file_url.html": [