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

feat(selectors): negative values for slice matcher's From and To #530

Merged
merged 5 commits into from
Aug 9, 2023

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jul 4, 2023

Ref: ipld/ipld#289

Builds on the basic fixes and tests in: #529

Note particularly the nuances in the test cases for negative values as they relate to what I outlined in ipld/ipld#289.

One implication of the way this is implemented is that we can't do the "your range didn't match, so we'll give you the full file". We end up with "didn't match, you get nothing (just the root)". Which seems like logical behaviour to me, but @willscott you did bring up the http spec saying that you could return the whole lot if your range is OOB or invalid in some way.

@willscott
Copy link
Member

Either behavior can be okay - if we can support the semantics of negative range or otherwise have enough confidence that the range requested doesn't match a portion of the file, not returning anything past the root is i believe the expected semantics (and would result in a render returning an http 416 'range not satisfiable' response)

@rvagg rvagg force-pushed the rvagg/slice-matcher-fixes branch from 319ff19 to 43637d8 Compare July 4, 2023 07:36
@rvagg rvagg force-pushed the rvagg/slice-matcher-negatives branch from cebd293 to f6610a1 Compare July 4, 2023 07:42
@rvagg
Copy link
Member Author

rvagg commented Jul 5, 2023

Made "from" allowed to underflow and be reset to 0. Put comments about the expected behaviour with the boundaries and whether things match or not in the spec docs @ ipld/ipld#289

Base automatically changed from rvagg/slice-matcher-fixes to master July 7, 2023 09:08
@rvagg rvagg force-pushed the rvagg/slice-matcher-negatives branch from d4d5de8 to d033706 Compare August 9, 2023 02:32
@rvagg rvagg merged commit a5beedf into master Aug 9, 2023
22 checks passed
@rvagg rvagg deleted the rvagg/slice-matcher-negatives branch August 9, 2023 02:47
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.

2 participants