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

Fix HTTP::ChunkedContent#peek return empty bytes at EOF #5943

Merged

Conversation

straight-shoota
Copy link
Member

After #5928 was merged I noticed the implementation of #peek was incorrect: it is supposed to return an empty slice at EOF but returned nil instead (which would mean the IO is not peekable).

That was already broken before, so it was not recognized in the PR.

@ysbaddaden
Copy link
Contributor

I don't understand. If you can't peek, shouldn't it raise? Just like trying to seek will raise. Other cases such as gets return a nilable. It's confusing :/

@straight-shoota
Copy link
Member Author

@ysbaddaden The docs for IO#peek says it should return:

  • nil if this IO isn't peekable
  • an empty slice if it is, but EOF was reached
  • a non-empty slice if some data can be peeked

@ysbaddaden
Copy link
Contributor

OK.

@sdogruyol sdogruyol merged commit e3dfb47 into crystal-lang:master Apr 12, 2018
@sdogruyol sdogruyol added this to the Next milestone Apr 12, 2018
@straight-shoota straight-shoota deleted the jm/fix/chunked-peek-empty branch April 12, 2018 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants