properly handle EOF in BufReader::peek

[lolbinarycat]

previously this would cause an infinite loop due to it being unable to read n bytes.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[workingjubilee]

@lolbinarycat Can we get a test with the code that would cause the infinite loop?

[Amanieu]

I think a better algorithm would be to remove the loop and do this instead:

• If enough bytes are already available in the buffer, return those immediately without doing any backshift.
• Otherwise backshift the buffer and then perform a single read operation on the underlying reader.
• If enough bytes were read to satisfy the peek, return those, otherwise return a shorter slice with the bytes that are available.

Always backshifting before reading will minimize the number of reads on the underlying reader, and we can avoid backshifting if enough bytes are already available in the buffer.

If the user really wants to issue multiple reads to the underlying reader then we should have a separate peek_exact method which does this.

[lolbinarycat]

@workingjubilee I actually added such a test to the end of the doctest, trying to peek once EOF has been reached would trigger the bug.

@Amanieu performing multiple reads is the entire purpose of this feature. if you don't care about robustness against short reads, you can just hackily use fill_buf. programs generally should not have their behavior be dependent on read boundaries.

[lolbinarycat]

any status on moving this forward? either we should merge this fix, or mark bufreader_peek as an incomplete feature, because as it stands, any program using this is will hang as soon as it encounters EOF, and there is no possible workaround, unless you want to wrap every seek with alarm calls using unsafe ffi.

if we want to change the behavior or api of this feature, that should be discussed in a separate issue.

[Amanieu]

Sure, let's just fix this for now, we can discuss the API separately.

@bors r+

[bors]

📌 Commit dfdbf63 has been approved by Amanieu

It is now in the queue for this repository.

[leo60228]

"the" should be capitalized, shouldn't it?

[lolbinarycat]

yeah, nice catch. i plan on doing another pass at the documentation once the finer details of the behavior are worked out, possibly earlier.

however, this is a very minor typo, and i don't think it's worth blocking such a critical fix on such a minor typo.

[bors]

⌛ Testing commit dfdbf63 with merge bc486f3...

[workingjubilee]

chill, bors.
@bors r-

[workingjubilee]

@bors r-

[workingjubilee]

@bors retry

[workingjubilee]

@bors r-