svenvc / zinc

Zinc HTTP Components is an open-source Smalltalk framework to deal with the HTTP networking protocol.
MIT License
97 stars 57 forks source link

ZnBufferedReadStream can read incorrectly after moving position back #148

Closed Rinzwind closed 2 months ago

Rinzwind commented 2 months ago

In the following example, the last #next message unexpectedly answers 1 rather than 2:

(byteArray := ByteArray new: 1 + 65535 + 32768 withAll: 0)
    at: 1 put: 1;
    at: 1 + 32768 put: 2.
(ZnBufferedReadStream on: byteArray readStream)
    next;
    next: 65535 + 32768;
    position: 32768;
    next

This is because #position: assumes that the buffer’s end corresponds to the current position of the stream, which is invalidated by the case in which #shouldBufferReadOfCount: answers false in #readInto:startingAt:count: as that advances the stream while not reading into the buffer. So it would seem that either the first method should be fixed to use the buffer’s end as determined when the buffer was read, or the case in the second method should be made to reset the buffer.

svenvc commented 2 months ago

Hi Kris,

Great catch, nice analysis, good reproducible example, super fix suggestion.

This is what I ended up with (basically just the highlighted addition). I made the example/test a bit simpler/smaller.

2024-08-30_18-08-00_BlElement-1024152576

If you agree I will commit this.

Rinzwind commented 2 months ago

Thanks, seems fine!

svenvc commented 2 months ago

Thanks, Kris, the fix was pushed