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

Add tests for HTTP Range requests #154

Open
7 tasks
lidel opened this issue Aug 29, 2023 · 1 comment · May be fixed by #213
Open
7 tasks

Add tests for HTTP Range requests #154

lidel opened this issue Aug 29, 2023 · 1 comment · May be fixed by #213
Labels
test:coverage Improves the spec covered by this test suite

Comments

@lidel
Copy link
Member

lidel commented Aug 29, 2023

cc ipfs/boxo#176, as would be good to have this before we merge that, or shortly after

TLDR

We don't want to be too strict here, but also we want to be able to write a test that limits the possibility space to the above paths.
If someone implements Ranges, we want to ensure the response is valid.
If they don't, we also want to ensure the response is valid.

Background

Coverage of range behaviors is bare minimum for UnixFS right now.
We should improve it and both relax requirements (allowing implementers to choose what to support) and make asserts more explicit (if you choose to implement something optional, you should behave A, if you don't, we expect fallback to be either B or C etc).

When it comes to range requests, HTTP specs (RFC9110) make them OPTIONAL. Supporting HTTP Ranges is useful for streaming chunks of deserialized videos, and that is what some IPFS Gateways support when a client asks for deserialized data (a single range).

Missing tests and tooling

  • ability to enable/disable all range request tests as a profile/flag/unit
  • make sure that if (multiple) ranges are implemented by gateway, they work correctly
    • ok if range is not supported at all, confirm Accept-Ranges none is present (HEAD/GET) and Range request returns HTTP 200 with the whole file
    • test if a single range is supported, confirm Range returned HTTP 206 with bytes for the slice
    • multiple ranges
      • this is not used in practice, browsers streaming videos ask for a single range, Kubo >=0.23 does not support this for unixfs, but if someone implements this, we should confirm it works ok
      • ok if returned HTTP 206 with content type multipart/byteranges and ALL requested ranges
      • ok if returned HTTP 206 with with bytes for the first of the ranges

A file could be any type of deserialized response, but in practice, end users only care about UnixFS file, and the test should use a chunked(!) UnixFS file, ideally in a HAMT directory, making sure Ranges work there too.

What we need in DSL

It is most likely that we need to extend test DSL with something like AnyOf described in #153

@lidel lidel added the test:coverage Improves the spec covered by this test suite label Aug 29, 2023
@lidel
Copy link
Member Author

lidel commented Mar 20, 2024

We have tests for trustless ?format=car&entity-bytes=0:0 but we are lacking a similar test for deserialized responses with HTTP header (Range: bytes=0-0).

  • HTTP Range request works and does overfetch
    • Reuse gateway-conformance/fixtures/trustless_gateway_car/file-3k-and-3-blocks-missing-block.car: A file chunked into 3 blocks, each 1024 bytes. The middle block is missing to ensure range request only triggers fetch of necessary blocks.
    • HTTP Range: bytes=0-0 request for /ipfs/QmYhmPjhFjYFyaoiuNzYv8WGavpSRDwdHWe5B4M5du5Rtk, expect response to be HTTP 206 with body equal to the first byte of the file
    • HTTP Range: bytes=2200- request for /ipfs/QmYhmPjhFjYFyaoiuNzYv8WGavpSRDwdHWe5B4M5du5Rtk, expect response to be HTTP 206 with body equal to the expected tail of the file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:coverage Improves the spec covered by this test suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant