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

Two WSGI environ bug fixes. #19

Merged
merged 2 commits into from Oct 4, 2012
Merged

Two WSGI environ bug fixes. #19

merged 2 commits into from Oct 4, 2012

Conversation

dowski
Copy link
Contributor

@dowski dowski commented Oct 1, 2012

The Content-Type header was not being correctly handled. If it wasn't sent exactly as "CONTENT-TYPE" it was getting recorded in the environ as "HTTP_CONTENT_TYPE" which is against PEP-333.

In the pure-Python parser, the SCRIPT_NAME field was being mishandled due to mutation of the original environ. The code was looking for HTTP_SCRIPT_NAME which was already popped from the environ dictionary if it existed.

The line immediately above this one pops HTTP_SCRIPT_NAME from environ
if it exists. Thus, it can't ever be in environ. The correct behavior is
to look for the value that it is renamed to: SCRIPT_NAME.
Clients may send the header names in any case combination. They are
already normalized to upper-case but the "special" header fields are
matched against the non-normalized name directly from the client.
@dowski
Copy link
Contributor Author

dowski commented Oct 2, 2012

Here's an interpreter session that shows the problem:

>>> req = """GET /index HTTP/1.1\r
... Host: foo\r
... Content-Type: bar\r
... \r
... """
>>> parser = HttpParser()
>>> parser.execute(req, len(req))
53
>>> parser.get_wsgi_environ()
{'RAW_URI': '/index', 'SCRIPT_NAME': '', 'REQUEST_METHOD': 'GET', 'HTTP_HOST': 'foo', 'PATH_INFO': '/index', 'SERVER_PROTOCOL': 'HTTP/1.1', 'QUERY_STRING': '', 'HTTP_CONTENT_TYPE': 'bar'}

Notice how you get HTTP_CONTENT_TYPE in the returned environ dictionary, not CONTENT_TYPE.

@dowski
Copy link
Contributor Author

dowski commented Oct 4, 2012

Any thoughts on this pull request? It seems like it addresses a real and fundamental bug to me, but maybe I'm misusing the API or something?

For reference, we're using http-parser in diesel (http://diesel.io/) and we'd like to be able to count on the official release version and not have to bundle a fixed version to work around this (apparent) issue.

benoitc added a commit that referenced this pull request Oct 4, 2012
Two WSGI environ bug fixes.
@benoitc benoitc merged commit 1971993 into benoitc:master Oct 4, 2012
@dowski
Copy link
Contributor Author

dowski commented Oct 4, 2012

Thank you!

@benoitc
Copy link
Owner

benoitc commented Oct 4, 2012

On Thu, Oct 4, 2012 at 3:27 PM, Christian Wyglendowski <
notifications@github.com> wrote:

Thank you!

Sorry for the delay. I'm just starting to recover but I generally release
faster :)

  • benoit

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.

None yet

2 participants