Skip to content

Conversation

squeek502
Copy link
Member

Previously, the logic in peekDelimiterInclusive (when the delimiter was not found in the existing buffer) used the n returned from r.vtable.stream as the length of the slice to check, but it's valid for vtable.stream implementations to return 0 if they wrote to the buffer instead of w. In that scenario, the indexOfScalarPos would be given a 0-length slice so it would never be able to find the delimiter.

This commit changes the logic to assume that r.vtable.stream can both:

  • return 0, and
  • modify seek/end (i.e. it's also valid for a vtable.stream implementation to rebase)

Also introduces std.testing.ReaderIndirect which helps in being able to test against Reader implementations that return 0 from stream/readVec

Fixes #25428


Before this PR, the added "takeDelimiterInclusive on an indirect reader when it rebases" test would fail with error.StreamTooLong since it'd always be searching for the delimiter in a 0-length slice.


This is a very targeted fix. It does not address any of the following:

It also assumes that the stream ambiguity described in #25170 should be resolved in the "writing to w cannot modify the buffer" way (i.e. this fix assumes that no implementation will try to write to both buffer and w in the same stream call).

(see also #25169 for some extra context around those issues)


The added "readSliceShort with indirect reader" test is just to verify the std.testing.ReaderIndirect implementation around rebasing within stream (without the rebase within stream, that test would enter an infinite loop).

…ns that return 0

Previously, the logic in peekDelimiterInclusive (when the delimiter was not found in the existing buffer) used the `n` returned from `r.vtable.stream` as the length of the slice to check, but it's valid for `vtable.stream` implementations to return 0 if they wrote to the buffer instead of `w`. In that scenario, the `indexOfScalarPos` would be given a 0-length slice so it would never be able to find the delimiter.

This commit changes the logic to assume that `r.vtable.stream` can both:
- return 0, and
- modify seek/end (i.e. it's also valid for a `vtable.stream` implementation to rebase)

Also introduces `std.testing.ReaderIndirect` which helps in being able to test against Reader implementations that return 0 from `stream`/`readVec`

Fixes ziglang#25428
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.

std.crypto.tls.Client hangs on reading application data
1 participant