MindFlavor/AzureSDKForRust

Blob Streaming doesn't work correctly with multiple parts

kodieg opened this issue · 2 comments

kodieg commented

There are few issues I found:

  1. Some bytes are overlapping when actually few chunks are downloaded (last byte of chunk i and first byte of chunk i+1)
  2. There is no way to set chunk size (and actually default chunk size is 1MB + 1 byte accidentally).
  3. One need to pass range, but ranges are inclusive which may be not obvious that one have to pass Range(0, blob.content_size - 1)
  4. If one uses Range(0, blob.content_size) and content_size is dividable by chunk_size, than one can get following error: UnexpectedHTTPResult(UnexpectedHTTPResult { expected: [206], received: 416, body: "\u{feff}<?xml version=\"1.0\" encoding=\"utf-8\"?><Error><Code>InvalidRange</Code><Message>The range specified is invalid for the current size of the resource.\nRequestId:...\n...</Message></Error>" })

I fixed first two errors in PR. Regarding other issues, there is a question whether Range should be inclusive? Whether library should support Ranges that are technically invalid (one byte off). In practice, library could read blob size from the content-range header and adjust requested range accordingly.

MindFlavor commented

Hi and thank you for the PR!
As for the different way range is implemented I agree with you, it should not be different from std (in fact, maybe there would be better to use the range from std).

MindFlavor commented

Regarding other issues, there is a question whether Range should be inclusive?

Unfortunately it's how it is in the SDK (and it seems from the specs too: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Range) use it. I was thinking about correcting the skew in the Range struct with something like:

impl fmt::Display for Range {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "bytes={}-{}", self.start, self.end - 1)
    }
}

Se the Range will be directly mappable from std::ops::Range

impl From<std::ops::Range<u64>> for Range {
    fn from(r: std::ops::Range<u64>) -> Self {
        Self {
            start: r.start,
            end: r.end,
        }
    }
}

keeping Rust semantics but also being correct interacting with the Azure REST API.

  • 👍1