Allow doing start-end RANGE requests against blobs #163
Conversation
thundergolfer
requested review from
thomastaylor312,
bacongobbler and
flavio
as code owners
|
Oh damn sorry I thought this was compiling because I was pointing at my fork but I had a cached |
thundergolfer
force-pushed
the
main
branch
2 times, most recently
from
4383888 to
0fc29b2
Compare
Signed-off-by: Jonathon Belotti <jonathon@modal.com>
|
@flavio I updated the implementation and added a test. During reimplementation I switch to using I'm not sure how this was working before? Let me know what you think :) |
| StatusCode::PARTIAL_CONTENT => Ok(BlobResponse::Partial(stream_from_response( | ||
| response, &layer, | ||
| response, &layer, false, |
There was a problem hiding this comment.
If the offset was ever non-zero this method should already have been failing on stream verification? I checked registry spec and couldn't see anything about returned specialized digests for partial reads.
There was a problem hiding this comment.
It makes sense that partial pulls can't be validated, so we should probably document the pull_blob_stream_partial that partial pulls by nature cannot be validated and as such should be validated by the caller when complete
There was a problem hiding this comment.
Actually, now that I'm looking at this more, we might need to return the header digest to validate against to the caller on a partial request (probably as a new data member of SizedStream), otherwise they won't have access to that information (though they do have access to the layer digest). If there is a better way to do this that other clients use, that would work as well!
There was a problem hiding this comment.
@thundergolfer If you'd like/are willing to do this, that would be great! If not, let me know and I'll switch this to an approval and I'll change it after merge since it is a preexisting edge case we didn't account for originally
There was a problem hiding this comment.
Hey @thomastaylor312, I've switched focus off the related project at work so I'll be unlikely to get to it soon.
There was a problem hiding this comment.
Ok sounds good. I'll merge this as is and fix up. Thanks for adding this!
|
@thundergolfer thanks for having introduced the test case. @thomastaylor312 can you take a quick look too? |
|
Nice, thanks. Let me know if you need anything else before merge 👌 |
| StatusCode::PARTIAL_CONTENT => Ok(BlobResponse::Partial(stream_from_response( | ||
| response, &layer, | ||
| response, &layer, false, |
There was a problem hiding this comment.
It makes sense that partial pulls can't be validated, so we should probably document the pull_blob_stream_partial that partial pulls by nature cannot be validated and as such should be validated by the caller when complete
|
Sorry forgot to add context when I hit the submit button. This all looks good to me. One minor point around adding some clarification to the docs for a function around the security implications |
In oras-project#163 it was pointed out that we can't use validating streams for partial responses, which makes complete sense. Because of this, we need to pass back the digest header for use in validation once a partial download is complete Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
== What's Changed * client: add support to set https_proxy/no_proxy for client by @Xynnn007 in oras-project#160 * chore(deps): Update testcontainers requirement from 0.20 to 0.22 by @dependabot in oras-project#161 * Docs.rs badge, update crate name. by @bddap in oras-project#166 * chore(deps): Bump actions/checkout from 4.1.7 to 4.2.0 by @dependabot in oras-project#170 * chore(deps): Update testcontainers requirement from 0.22 to 0.23 by @dependabot in oras-project#169 * chore(deps): Update rstest requirement from 0.22.0 to 0.23.0 by @dependabot in oras-project#168 * Allow doing start-end RANGE requests against blobs by @thundergolfer in oras-project#163 * fix(client): Return the digest header for partial responses by @thomastaylor312 in oras-project#171 * remove reference by @flavio in oras-project#164 == New Contributors * @bddap made their first contribution in oras-project#166 * @thundergolfer made their first contribution in oras-project#163 **Full Changelog**: oras-project/rust-oci-client@v0.12.1...v0.13.0 Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Addresses #162