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

http/message: Split request line as bytes to avoid splitting on 0x0A. Fixes #1577 #1578

Merged
merged 2 commits into from Sep 2, 2017

Conversation

Projects
None yet
3 participants
@rslinckx
Copy link
Contributor

rslinckx commented Aug 25, 2017

No description provided.

yield bytes(d.encode("latin1"))
for d in self.data:
if six.PY3:
yield bytes([d])

This comment has been minimized.

@tilgovi

tilgovi Aug 27, 2017

Collaborator

I don't think the cast is necessary anymore, since we open the file in binary mode.

This comment has been minimized.

@rslinckx

rslinckx Aug 28, 2017

Author Contributor

I'm not sure what was the intention there, but it's reading the data byte-by-byte and yielding byte-by-byte.

In py2, iterating the data str will yield individual bytes as str
In py3, iterating the data bytes will yield individual bytes as int, which then need to be returned as new bytes using bytes([d])

Previous code was just plain wrong under py2 as str(self.data.decode("latin1")) will try to re-encode an unicode string in ascii

This comment has been minimized.

@tilgovi

tilgovi Aug 31, 2017

Collaborator

Thanks for the explanation.

@tilgovi

This comment has been minimized.

Copy link
Collaborator

tilgovi commented Aug 27, 2017

Looks good. Thank you!

@tilgovi

This comment has been minimized.

Copy link
Collaborator

tilgovi commented Aug 31, 2017

@berkerpeksag, @benoitc do you see any problem with this?

@benoitc benoitc merged commit c171c15 into benoitc:master Sep 2, 2017

5 of 8 checks passed

couverture-io/py26 Not all changes are covered
Details
couverture-io/py36 Not all changes are covered
Details
couverture-io/py37 Not all changes are covered
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
couverture-io/py27 Code coverage OK
Details
couverture-io/py34 Code coverage OK
Details
couverture-io/py35 Code coverage OK
Details
couverture-io/py36-dev Code coverage OK
Details
@benoitc

This comment has been minimized.

Copy link
Owner

benoitc commented Sep 2, 2017

@rslinckx thanks

@tilgovi merged :)

fofanov pushed a commit to fofanov/gunicorn that referenced this pull request Mar 16, 2018

Merge pull request benoitc#1578 from rslinckx/master
http/message: Split request line as bytes to avoid splitting on 0x0A. Fixes benoitc#1577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.