Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway: clarify entity-bytes out of range behavior #460

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lidel
Copy link
Member

@lidel lidel commented Jan 25, 2024

This PR applies cosmetic spec clarification based on discussion that happened in ipfs/boxo#523 and was released in gateway-conformance@v0.5.

The intention here is to make it more clear that it is ok to return HTTP 200 and that truncation is fine too.

cc @rvagg @hannahhoward

Cosmetic clarification based on discussion that happened in 
ipfs/boxo#523
@lidel lidel changed the title gateway: clarify entiry-bytes out of range gateway: clarify entity-bytes out of range behavior Jan 25, 2024
@@ -155,16 +155,16 @@ The following additional values are supported:
A Gateway MUST augment the returned `Etag` based on the passed `entity-bytes`.

A Gateway SHOULD return an HTTP 400 Bad Request error when the requested range
cannot be parsed as valid offset positions.
is outside of valid offset positions in full, and the gateway knows that upfront.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

soft suggestion, my brain tripped up on "in full" when reading this on the first pass.

Suggested change
is outside of valid offset positions in full, and the gateway knows that upfront.
is entirely outside of the entity's byte range and the server is able to determine this
upfront.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants