Unverified Commit 2ff56c28 authored by Michael Rossberg's avatar Michael Rossberg Committed by GitHub
Browse files

Fix range requests for GetObject (#128) 



* Fix: GetObject range request

* fix satisfaction check

* content range

---------

Co-authored-by: default avatarNugine <nugine@foxmail.com>
parent 6a042ab1

crates/s3s-fs/src/s3.rs

+11 −3
Original line number Diff line number Diff line
@@ -48,6 +48,11 @@ fn normalize_path(path: &Path, delimiter: &str) -> Option<String> { 
    Some(normalized)

}



/// <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Range>

fn fmt_content_range(start: u64, end_inclusive: u64, size: u64) -> String {

    format!("bytes {start}-{end_inclusive}/{size}")

}



#[async_trait::async_trait]

impl S3 for FileSystem {

    #[tracing::instrument]
@@ -195,11 +200,13 @@ impl S3 for FileSystem { 
        let last_modified = Timestamp::from(try_!(file_metadata.modified()));

        let file_len = file_metadata.len();



        let content_length = match input.range {

            None => file_len,

        let (content_length, content_range) = match input.range {

            None => (file_len, None),

            Some(range) => {

                let file_range = range.check(file_len)?;

                file_range.end - file_range.start

                let content_length = file_range.end - file_range.start;

                let content_range = fmt_content_range(file_range.start, file_range.end - 1, file_len);

                (content_length, Some(content_range))

            }

        };

        let content_length_usize = try_!(usize::try_from(content_length));
@@ -232,6 +239,7 @@ impl S3 for FileSystem { 
        let output = GetObjectOutput {

            body: Some(StreamingBlob::wrap(body)),

            content_length: content_length_i64,

            content_range,

            last_modified: Some(last_modified),

            metadata: object_metadata,

            e_tag: Some(e_tag),

crates/s3s/src/dto/range.rs

+47 −13
Original line number Diff line number Diff line
@@ -103,31 +103,37 @@ impl Range { 
    }



    /// Checks if the range is satisfiable

    ///  according to [RFC9110](https://www.rfc-editor.org/rfc/rfc9110.html#name-byte-ranges).

    /// # Errors

    /// Returns an error if the range is not satisfiable

    #[allow(clippy::range_plus_one)] // cannot be fixed

    pub fn check(&self, full_length: u64) -> Result<ops::Range<u64>, RangeNotSatisfiable> {

        let err = || RangeNotSatisfiable { _priv: () };

        match *self {

            Range::Int { first, last } => match last {

            Range::Int { first, last } => {

                if first >= full_length {

                    return Err(err());

                }

                // 0 <= first < full_length



                match last {

                    Some(last) => {

                    if first > last || last >= full_length {

                        let last = last.min(full_length - 1);

                        if first > last {

                            return Err(err());

                        }

                    // first <= last < full_length

                        // 0 <= first <= last < full_length

                        Ok(first..last + 1)

                    }

                None => {

                    if first > full_length {

                        return Err(err());

                    // 0 <= first < full_length

                    None => Ok(first..full_length),

                }

                    Ok(first..full_length)

            }

            },

            Range::Suffix { length } => {

                if length > full_length {

                if length == 0 {

                    return Err(err());

                }

                let length = length.min(full_length);

                Ok((full_length - length)..full_length)

            }

        }
@@ -193,6 +199,8 @@ mod tests { 
            ("bytes=-500 ", Err(())),

            ("bytes=-+500", Err(())),

            ("bytes=-1000000000000000000000000", Err(())),

            ("bytes=-0", Ok(range_suffix(0))),

            ("bytes=-000", Ok(range_suffix(0))),

        ];



        for (input, expected) in &cases {
@@ -203,4 +211,30 @@ mod tests { 
            }

        }

    }



    #[test]

    fn satisfiable() {

        let cases = [

            (10000, range_int_from(9500), Ok(9500..10000)),

            (10000, range_int_from(10000), Err(())),

            (10000, range_int_inclusive(0, 499), Ok(0..500)),

            (10000, range_int_inclusive(0, 0), Ok(0..1)),

            (10000, range_int_inclusive(9500, 50000), Ok(9500..10000)),

            (10000, range_int_inclusive(10000, 10000), Err(())),

            (10000, range_suffix(500), Ok(9500..10000)),

            (10000, range_suffix(10000), Ok(0..10000)),

            (10000, range_suffix(0), Err(())),

            (0, range_int_from(0), Err(())),

            (0, range_suffix(1), Ok(0..0)),

            (0, range_suffix(0), Err(())),

        ];



        for &(full_length, ref range, ref expected) in &cases {

            let output = range.check(full_length);

            match expected {

                Ok(expected) => assert_eq!(output.unwrap(), *expected),

                Err(()) => assert!(output.is_err(), "{full_length:?}, {range:?}"),

            }

        }

    }

}