)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9fcbf16d9873b3099d2f666e5b3c8e59e3775afe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1b191117_9a181bdb","updated":"2023-01-24 17:41:29.000000000","message":"I\u0027m still not sure whether this is actually a direction worth pursuing...","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3a16c92c_a272f4c9","updated":"2022-12-08 18:54:27.000000000","message":"This seems interesting to me!  I guess once we decide we can\u0027t depend on upstream CPython to own our http parser anymore it seems weird to NOT at least TRY and make it more reasonable.  And now that I\u0027m looking at it, the existing py2/py3 MessageClass split is pretty nuts!\n\nI hear https://github.com/MagicStack/httptools is pretty fast (but maybe lacks PROXY support https://github.com/MagicStack/httptools/issues/61) and https://github.com/python-hyper/h11 claims to be very compliant.  Maybe we could steal code/tests or even add something (different/better?) as a dependency!","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"}],"swift/common/http_protocol.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        # for py3:"},{"line_number":63,"context_line":"        def get_default_type(self):"},{"line_number":64,"context_line":"            \u0027\u0027\u0027If the client didn\u0027t provide a content type, leave it blank.\u0027\u0027\u0027"},{"line_number":65,"context_line":"            return \u0027\u0027"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def parse_request(self):"},{"line_number":68,"context_line":"        \"\"\"Parse a request (inlined from cpython@7e293984)."}],"source_content_type":"text/x-python","patch_set":1,"id":"88c20a91_4b2ed437","side":"PARENT","line":65,"updated":"2022-12-08 18:54:27.000000000","message":"ok, i forgot about this - this is gross/weird","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dabd4e0ab5fa488367005f2e778c5b9c0967cd6c","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        # for py3:"},{"line_number":63,"context_line":"        def get_default_type(self):"},{"line_number":64,"context_line":"            \u0027\u0027\u0027If the client didn\u0027t provide a content type, leave it blank.\u0027\u0027\u0027"},{"line_number":65,"context_line":"            return \u0027\u0027"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def parse_request(self):"},{"line_number":68,"context_line":"        \"\"\"Parse a request (inlined from cpython@7e293984)."}],"source_content_type":"text/x-python","patch_set":1,"id":"fde91793_e7861fb1","side":"PARENT","line":65,"in_reply_to":"88c20a91_4b2ed437","updated":"2022-12-19 21:54:22.000000000","message":"I maintain it was better than it used to be: https://review.opendev.org/c/openstack/swift/+/640552\n\nBut I much prefer the type property we\u0027ve got above, now.","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                )"},{"line_number":157,"context_line":"                return False"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"            header_payload \u003d self.headers.get_payload()"},{"line_number":160,"context_line":"            if isinstance(header_payload, list) and len(header_payload) \u003d\u003d 1:"},{"line_number":161,"context_line":"                header_payload \u003d header_payload[0].get_payload()"},{"line_number":162,"context_line":"            if header_payload:"}],"source_content_type":"text/x-python","patch_set":1,"id":"27d7d0fb_8778c8c9","side":"PARENT","line":159,"updated":"2022-12-08 18:54:27.000000000","message":"our new headers object has no \"get_payload\" method","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dabd4e0ab5fa488367005f2e778c5b9c0967cd6c","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                )"},{"line_number":157,"context_line":"                return False"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"            header_payload \u003d self.headers.get_payload()"},{"line_number":160,"context_line":"            if isinstance(header_payload, list) and len(header_payload) \u003d\u003d 1:"},{"line_number":161,"context_line":"                header_payload \u003d header_payload[0].get_payload()"},{"line_number":162,"context_line":"            if header_payload:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f9075d8_d1c46041","side":"PARENT","line":159,"in_reply_to":"27d7d0fb_8778c8c9","updated":"2022-12-19 21:54:22.000000000","message":"Nor does it need one. We\u0027re the only folks crazy enough to go poking at that as an HTTP server, at least that I\u0027m aware of.","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":true,"context_lines":[{"line_number":174,"context_line":"                    # whether the client said the connection should close;"},{"line_number":175,"context_line":"                    # see https://github.com/eventlet/eventlet/blob/v0.25.0/"},{"line_number":176,"context_line":"                    # eventlet/wsgi.py#L504"},{"line_number":177,"context_line":"                    self.headers.add_header(header, value)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        conntype \u003d self.headers.get(\u0027Connection\u0027, \"\")"},{"line_number":180,"context_line":"        if conntype.lower() \u003d\u003d \u0027close\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cfa5e673_de4ae1dc","side":"PARENT","line":177,"updated":"2022-12-08 18:54:27.000000000","message":"so following the bug, this is like special py3-only \"crazy-utf8\u0027 headers handling\n\nI assume the new impl just \"does the right thing\" in all cases?\n\nDid you lift the parser from anywhere specific, like https://github.com/benoitc/http-parser/blob/master/http_parser/pyparser.py#L327 (that project has a cython-impl too)","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dabd4e0ab5fa488367005f2e778c5b9c0967cd6c","unresolved":true,"context_lines":[{"line_number":174,"context_line":"                    # whether the client said the connection should close;"},{"line_number":175,"context_line":"                    # see https://github.com/eventlet/eventlet/blob/v0.25.0/"},{"line_number":176,"context_line":"                    # eventlet/wsgi.py#L504"},{"line_number":177,"context_line":"                    self.headers.add_header(header, value)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        conntype \u003d self.headers.get(\u0027Connection\u0027, \"\")"},{"line_number":180,"context_line":"        if conntype.lower() \u003d\u003d \u0027close\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"dc6bb2e1_6f5ee16c","side":"PARENT","line":177,"in_reply_to":"cfa5e673_de4ae1dc","updated":"2022-12-19 21:54:22.000000000","message":"Yup; old way abused an email parser into parsing HTTP headers. Apparently some emails wouldn\u0027t have a nice, bright boundary between headers and subject/body, so there\u0027d need to be some heuristic such that if a header couldn\u0027t be parsed, it was assumed that the rest was body. Somewhere in the py3 history, header parsing got more rigid, to the point that legit swift headers (quietly) break things.\n\nHTTP *does* have a bright boundary between headers and body, so parse all headers and ensure that if there\u0027s a failure, it causes a 4xx.","commit_id":"f72f65f836942f51c2b97057f93f560745db36c7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    from eventlet.green.http import client as http_client"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class RawHeaders(object):"},{"line_number":27,"context_line":"    def __init__(self, headers):"},{"line_number":28,"context_line":"        self.items \u003d headers"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8c3214e9_bb7acf1f","line":26,"updated":"2022-12-08 18:54:27.000000000","message":"I\u0027m surprised there\u0027s no base class, the \"MessageClass\" abstraction seemed to persist across py2\u003d\u003epy3 - is that interface defined somewhere?  How do I know this is a complete implementation?","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dabd4e0ab5fa488367005f2e778c5b9c0967cd6c","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    from eventlet.green.http import client as http_client"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class RawHeaders(object):"},{"line_number":27,"context_line":"    def __init__(self, headers):"},{"line_number":28,"context_line":"        self.items \u003d headers"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"659e8526_e20f4c0f","line":26,"in_reply_to":"8c3214e9_bb7acf1f","updated":"2022-12-19 21:54:22.000000000","message":"IDK that I\u0027d call it an \"abstraction\" exactly -- it\u0027s a property of the BaseHTTPRequestHandler. See\n\n- https://github.com/python/cpython/blob/v2.7.18/Lib/BaseHTTPServer.py#L518\n- https://github.com/python/cpython/blob/v3.11.0/Lib/http/server.py#L623\n\nFrom those definitions, the inheritance chains diverge between py2/py3, but they\u0027re always rooted in email -- which seems fundamentally wrong.","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cabe92a82bb924f97e09d0790f25fb7b88d679e2","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @property"},{"line_number":31,"context_line":"    def headers(self):"},{"line_number":32,"context_line":"        headers \u003d [h + \u0027: \u0027 + v for h, v in self.items]"},{"line_number":33,"context_line":"        return headers"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get(self, header, default\u003dNone):"},{"line_number":36,"context_line":"        for h, v in self.items:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7581ab8a_3253b837","line":33,"updated":"2023-01-24 16:19:42.000000000","message":"Where do you want to use this?","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9fcbf16d9873b3099d2f666e5b3c8e59e3775afe","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @property"},{"line_number":31,"context_line":"    def headers(self):"},{"line_number":32,"context_line":"        headers \u003d [h + \u0027: \u0027 + v for h, v in self.items]"},{"line_number":33,"context_line":"        return headers"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def get(self, header, default\u003dNone):"},{"line_number":36,"context_line":"        for h, v in self.items:"}],"source_content_type":"text/x-python","patch_set":1,"id":"391d0483_6a2f3135","line":33,"in_reply_to":"7581ab8a_3253b837","updated":"2023-01-24 17:41:29.000000000","message":"It\u0027s used down in eventlet: https://github.com/eventlet/eventlet/blob/master/eventlet/wsgi.py#L716-L723\n\nThat\u0027s also where eventlet sets env[\u0027headers_raw\u0027], so I don\u0027t think we can normalize headers in __init__() or parse(), sorry. I should add some comments about it, though.","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cabe92a82bb924f97e09d0790f25fb7b88d679e2","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    def get(self, header, default\u003dNone):"},{"line_number":36,"context_line":"        for h, v in self.items:"},{"line_number":37,"context_line":"            if h.lower() \u003d\u003d header.lower():"},{"line_number":38,"context_line":"                return v"},{"line_number":39,"context_line":"        return default"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":1,"id":"5584e609_47b45605","line":38,"updated":"2023-01-24 16:19:42.000000000","message":"I think storing keys in a uniform way would avoid an explicit loop.","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cabe92a82bb924f97e09d0790f25fb7b88d679e2","unresolved":true,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @property"},{"line_number":42,"context_line":"    def type(self):"},{"line_number":43,"context_line":"        return self.get(\u0027content-type\u0027, \u0027\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"},{"line_number":46,"context_line":"    def parse(cls, rfile):"}],"source_content_type":"text/x-python","patch_set":1,"id":"09aacea9_e016f391","line":43,"updated":"2023-01-24 16:19:42.000000000","message":"And look at this, you already rely on keys being lower-cased.","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cabe92a82bb924f97e09d0790f25fb7b88d679e2","unresolved":true,"context_lines":[{"line_number":61,"context_line":"            if \u0027:\u0027 not in line:"},{"line_number":62,"context_line":"                raise ValueError(\u0027header line missing colon (\":\")\u0027)"},{"line_number":63,"context_line":"            h, v \u003d line.split(\u0027:\u0027, 1)"},{"line_number":64,"context_line":"            headers.append((h, v.strip(\u0027 \\t\\r\\n\u0027)))"},{"line_number":65,"context_line":"        return cls(headers)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"217a89c9_9db68605","line":64,"updated":"2023-01-24 16:19:42.000000000","message":"Why not lovercase the header key here?","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3ff2ebb0cf92db92c396c836d4838a1794b3b212","unresolved":true,"context_lines":[{"line_number":185,"context_line":"                \"Bad Request\","},{"line_number":186,"context_line":"                str(err)"},{"line_number":187,"context_line":"            )"},{"line_number":188,"context_line":"            return False"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        conntype \u003d self.headers.get(\u0027Connection\u0027, \"\")"},{"line_number":191,"context_line":"        if conntype.lower() \u003d\u003d \u0027close\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"947a74b5_dd310c68","line":188,"updated":"2022-12-08 18:54:27.000000000","message":"this exception seems new - is this \"just\" for un-expected parsing errors - or did we move some existing 400 handling into a ValueError?","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dabd4e0ab5fa488367005f2e778c5b9c0967cd6c","unresolved":true,"context_lines":[{"line_number":185,"context_line":"                \"Bad Request\","},{"line_number":186,"context_line":"                str(err)"},{"line_number":187,"context_line":"            )"},{"line_number":188,"context_line":"            return False"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        conntype \u003d self.headers.get(\u0027Connection\u0027, \"\")"},{"line_number":191,"context_line":"        if conntype.lower() \u003d\u003d \u0027close\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"f19ad155_81199ca1","line":188,"in_reply_to":"947a74b5_dd310c68","updated":"2022-12-19 21:54:22.000000000","message":"Yeah, just the errors you see up in parse()\n\nReally, raising HTTPException and LineTooLong are just weird/bad hacks anyway -- longer term, I\u0027d want to rewrite all these except blocks to be one, and hang status codes etc. off hte exception raised.","commit_id":"131739b00258a5e8a415d167fa7fd93b9ce4a17a"}]}
