Fix broken header parsing in logging

[mattbillenstein]

I found this by mixing flask-websocket in a flask app -- the parsing logic here leaves whitespace in the header values (including \r\n) so it's not suitable for extracting headers into logs.

[tilgovi]

Can you explain how this fixes the problem? It's not at all obvious from the change.

[mattbillenstein]

Sorry, could have made that more clear -- pywsgi provides an object that is dict-like and wraps the headers -- the headers property on that object:

        @property
        def headers(self):
            for key, value in self._headers:
                yield '%s: %s\r\n' % (key, value)

adds whitespace for some reason, it's much cleaner to just call .items():

import sys
if sys.version_info.major == 2:
    from StringIO import StringIO
else:
    from io import BytesIO as StringIO

import gevent.pywsgi

raw_headers = b'Foo: bar\r\nBar: baz\r\nBaz: boo\r\n\r\n'
fp = StringIO(raw_headers)
headers = gevent.pywsgi.headers_factory(fp)

print([h.split(":", 1) for h in headers.headers])
# [['Foo', ' bar\r\n'], ['Bar', ' baz\r\n'], ['Baz', ' boo\r\n']]

print(headers.items())
# [('baz', 'boo'), ('foo', 'bar'), ('bar', 'baz')]

(tve)mattb@matt:~ $ python3 header_fix.py
[['Foo', ' bar\r\n'], ['Bar', ' baz\r\n'], ['Baz', ' boo\r\n']]
[('Foo', 'bar'), ('Bar', 'baz'), ('Baz', 'boo')]

(tve)mattb@matt:~ $ python2 header_fix.py
[['Foo', ' bar\r\n'], ['Bar', ' baz\r\n'], ['Baz', ' boo\r\n']]
[('baz', 'boo'), ('foo', 'bar'), ('bar', 'baz')]

I'm not sure what the actual interface for the headers attribute is supposed to be between the various servers and frameworks -- not sure how much test coverage you have around this; I think this code path isn't even exercised when using the gevent worker, I only have this problem when also using flask-sockets and geventwebsocket.

[tilgovi]

I checked this out and it seems fine.

We don't have any coverage right now for anything around this, but in gevent 0.13.x up through the most recent versions this object is mimetools.Message or the wrapper on Python 3 that has the code you copy/pasted. In all cases, .items() works fine.

@berkerpeksag should we do anything more here or just merge this?

[berkerpeksag]

Shouldn't this be changed to {}?

[tilgovi]

Nope. It's been a list of tuples and that isn't being changed here.

[berkerpeksag]

I don't know much about this part of Gunicorn, but this looks good to me. Thank you!

I think current behavior comes from the mimetools module (which is an ancient Python 2 module) so I don't know why mimetools behaved like that. In gevent/gevent@198f239, they make the behavior same under Python 3. There is also a check for ' ' and '\t' at https://github.com/gevent/gevent/blob/98c2c15fbbee0c6ab6fab0ba747fd0cdceca2ba8/src/gevent/pywsgi.py#L1036 but this detail is probably not important in our case.

I'll just ping @jamadden in case we missed something.

[jamadden]

I'm not sure what you're asking, but calling items should be fine. I don't see that changing.