)]}'
{"swift/common/wsgi.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"38700ac824f3b9340c314a99eb083328793c3e0f","unresolved":false,"context_lines":[{"line_number":495,"context_line":"                environ[\u0027headers_raw\u0027] \u003d tuple(headers_raw)"},{"line_number":496,"context_line":"                # Since we parsed some more headers, check to see if they"},{"line_number":497,"context_line":"                # change how our wsgi.input should behave"},{"line_number":498,"context_line":"                te \u003d environ.get(\u0027HTTP_TRANSFER_ENCODING\u0027, \u0027\u0027)"},{"line_number":499,"context_line":"                if te.lower() \u003d\u003d \u0027chunked\u0027:"},{"line_number":500,"context_line":"                    environ[\u0027wsgi.input\u0027].chunked_input \u003d True"},{"line_number":501,"context_line":"                else:"},{"line_number":502,"context_line":"                    length \u003d environ.get(\u0027CONTENT_LENGTH\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_65e384a5","line":499,"range":{"start_line":498,"start_character":16,"end_line":499,"end_character":43},"updated":"2019-09-26 16:13:04.000000000","message":"This should maybe get updated, too... though as I have it now, it matches what eventlet does: https://github.com/eventlet/eventlet/blob/v0.25.1/eventlet/wsgi.py#L722","commit_id":"61a49bec1ff69c60d14a5d94da3da4e849f28a31"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b994eb7b6f5161e76e15706c3d2b83b5daaa7c37","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            if header_payload:"},{"line_number":472,"context_line":"                # This shouldn\u0027t be here. We must\u0027ve bumped up against"},{"line_number":473,"context_line":"                # https://bugs.python.org/issue37093"},{"line_number":474,"context_line":"                headers_raw \u003d list(environ[\u0027headers_raw\u0027])"},{"line_number":475,"context_line":"                for line in header_payload.rstrip(\u0027\\r\\n\u0027).split(\u0027\\n\u0027):"},{"line_number":476,"context_line":"                    if \u0027:\u0027 not in line or line[:1] in \u0027 \\t\u0027:"},{"line_number":477,"context_line":"                        # Well, we\u0027re no more broken than we were before..."}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_167cefb3","line":474,"updated":"2019-10-02 18:29:26.000000000","message":"it\u0027s not obvious to me *at all* that the change below should be\n1) only if not py2\n2) only if self.headers.get_payload()\n\nare their good reasons to not make this stuff conditional?\n\nI think once we\u0027ve got a couple bugs with no response filed upstream on CPython\u0027s HTTP parser we\u0027re both opinionated and competent enough to just say \"we\u0027re maintaining this shit ourselves\" and remove a few if branches in favor of having \"one preferably obvious way to do it\"","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"281a39d51ca5aebf327633fcbb24e18c75e82133","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            if header_payload:"},{"line_number":472,"context_line":"                # This shouldn\u0027t be here. We must\u0027ve bumped up against"},{"line_number":473,"context_line":"                # https://bugs.python.org/issue37093"},{"line_number":474,"context_line":"                headers_raw \u003d list(environ[\u0027headers_raw\u0027])"},{"line_number":475,"context_line":"                for line in header_payload.rstrip(\u0027\\r\\n\u0027).split(\u0027\\n\u0027):"},{"line_number":476,"context_line":"                    if \u0027:\u0027 not in line or line[:1] in \u0027 \\t\u0027:"},{"line_number":477,"context_line":"                        # Well, we\u0027re no more broken than we were before..."}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_cc632ab1","line":474,"in_reply_to":"3fa7e38b_167cefb3","updated":"2019-10-02 19:22:38.000000000","message":"As to 1, somewhere in this sad saga we started preferring that py3-only code be marked as such, to make it clear that we really aren\u0027t going to break py2. I seem to recall it in particular in the context of https://review.opendev.org/#/c/645284/ ?\n\nFor 2, the headers should have no payload -- they should have all been parsed. The fact that they weren\u0027t means that we hit the abort-parsing bug. As long as all the headers got parsed, I\u0027m fairly comfortable saying that eventlet can take care of setting up its own wsgi.input, and we can get out of the way.","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b994eb7b6f5161e76e15706c3d2b83b5daaa7c37","unresolved":false,"context_lines":[{"line_number":501,"context_line":"                else:"},{"line_number":502,"context_line":"                    length \u003d environ.get(\u0027CONTENT_LENGTH\u0027)"},{"line_number":503,"context_line":"                    if length:"},{"line_number":504,"context_line":"                        length \u003d int(length)"},{"line_number":505,"context_line":"                    environ[\u0027wsgi.input\u0027].content_length \u003d length"},{"line_number":506,"context_line":"                if environ.get(\u0027HTTP_EXPECT\u0027, \u0027\u0027).lower() \u003d\u003d \u0027100-continue\u0027:"},{"line_number":507,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile \u003d self.wfile"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_3111f554","line":504,"updated":"2019-10-02 18:29:26.000000000","message":"It\u0027s not obvious to me why we wouldn\u0027t want to explicitly add \u0027connection: close\u0027 to the headers in the else case here?  \n\nBut I trust that setting None below is sufficient i.e. the object server will get the message.","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"281a39d51ca5aebf327633fcbb24e18c75e82133","unresolved":false,"context_lines":[{"line_number":501,"context_line":"                else:"},{"line_number":502,"context_line":"                    length \u003d environ.get(\u0027CONTENT_LENGTH\u0027)"},{"line_number":503,"context_line":"                    if length:"},{"line_number":504,"context_line":"                        length \u003d int(length)"},{"line_number":505,"context_line":"                    environ[\u0027wsgi.input\u0027].content_length \u003d length"},{"line_number":506,"context_line":"                if environ.get(\u0027HTTP_EXPECT\u0027, \u0027\u0027).lower() \u003d\u003d \u0027100-continue\u0027:"},{"line_number":507,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile \u003d self.wfile"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_11039911","line":504,"in_reply_to":"3fa7e38b_3111f554","updated":"2019-10-02 19:22:38.000000000","message":"It we don\u0027t have a length and it\u0027s not chunked, the object server shouldn\u0027t even see it. We\u0027ll bomb out with a 411 in check_object_creation, called from the obj controller.\n\nIf it\u0027s *not* an object PUT... existing code seems to allow the pipelining?? Is that right?\n\n $ curl -v -X PUT http://saio:8090/v1/AUTH_test/x -X PUT http://saio:8090/v1/AUTH_test/y \n*   Trying 192.168.8.80:8090...\n* TCP_NODELAY set\n* Connected to saio (192.168.8.80) port 8090 (#0)\n\u003e PUT /v1/AUTH_test/x HTTP/1.1\n\u003e Host: saio:8090\n\u003e User-Agent: curl/7.65.3\n\u003e Accept: application/json; q\u003d1.0, text/*; q\u003d0.9, */*; q\u003d0.8\n\u003e \n* Mark bundle as not supporting multiuse\n\u003c HTTP/1.1 201 Created\n\u003c Content-Type: text/html; charset\u003dUTF-8\n\u003c Content-Length: 0\n\u003c X-Trans-Id: tx00844bedc49e462aa64e7-005d94f53f\n\u003c X-Openstack-Request-Id: tx00844bedc49e462aa64e7-005d94f53f\n\u003c Date: Wed, 02 Oct 2019 19:06:40 GMT\n\u003c \n* Connection #0 to host saio left intact\n* Found bundle for host saio: 0x55a066872340 [serially]\n* Can not multiplex, even if we wanted to!\n* Re-using existing connection! (#0) with host saio\n* Connected to saio (192.168.8.80) port 8090 (#0)\n\u003e PUT /v1/AUTH_test/y HTTP/1.1\n\u003e Host: saio:8090\n\u003e User-Agent: curl/7.65.3\n\u003e Accept: application/json; q\u003d1.0, text/*; q\u003d0.9, */*; q\u003d0.8\n\u003e \n* Mark bundle as not supporting multiuse\n\u003c HTTP/1.1 201 Created\n\u003c Content-Type: text/html; charset\u003dUTF-8\n\u003c Content-Length: 0\n\u003c X-Trans-Id: txea88cf871d6a441e92014-005d94f540\n\u003c X-Openstack-Request-Id: txea88cf871d6a441e92014-005d94f540\n\u003c Date: Wed, 02 Oct 2019 19:06:40 GMT\n\u003c \n* Connection #0 to host saio left intact","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b994eb7b6f5161e76e15706c3d2b83b5daaa7c37","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                if environ.get(\u0027HTTP_EXPECT\u0027, \u0027\u0027).lower() \u003d\u003d \u0027100-continue\u0027:"},{"line_number":507,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile \u003d self.wfile"},{"line_number":508,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile_line \u003d \\"},{"line_number":509,"context_line":"                        b\u0027HTTP/1.1 100 Continue\\r\\n\u0027"},{"line_number":510,"context_line":"            return environ"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_b6187b84","line":509,"updated":"2019-10-02 18:29:26.000000000","message":"I used to hate it when we had to write HTTP parsing code, but after all these years I\u0027m starting to hate it more we call out to someone *elses* http parsing code.\n\nthis all looks excellent.  Kudos Tim.","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"281a39d51ca5aebf327633fcbb24e18c75e82133","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                if environ.get(\u0027HTTP_EXPECT\u0027, \u0027\u0027).lower() \u003d\u003d \u0027100-continue\u0027:"},{"line_number":507,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile \u003d self.wfile"},{"line_number":508,"context_line":"                    environ[\u0027wsgi.input\u0027].wfile_line \u003d \\"},{"line_number":509,"context_line":"                        b\u0027HTTP/1.1 100 Continue\\r\\n\u0027"},{"line_number":510,"context_line":"            return environ"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_f1e27da0","line":509,"in_reply_to":"3fa7e38b_b6187b84","updated":"2019-10-02 19:22:38.000000000","message":"After the py3 transition, I whole-heartedly agree.","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"}],"swift/proxy/server.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"e98546440d03e146c776833e14dc4f0d9c7d7426","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        if \u0027x-storage-token\u0027 in req.headers and \\"},{"line_number":458,"context_line":"                \u0027x-auth-token\u0027 not in req.headers:"},{"line_number":459,"context_line":"            req.headers[\u0027x-auth-token\u0027] \u003d req.headers[\u0027x-storage-token\u0027]"},{"line_number":460,"context_line":"        if req.headers.get(\u0027transfer-encoding\u0027) \u003d\u003d \u0027chunked\u0027 and \\"},{"line_number":461,"context_line":"                \u0027content-length\u0027 in req.headers:"},{"line_number":462,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":463,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_be00bb02","line":460,"updated":"2019-09-26 01:51:05.000000000","message":"Please pardon my ignorance, but do we allow clients to submit a header \"Transfer-Encoding: gzip, chunked\"? I think we don\u0027t, and the above is safe, but I\u0027m not 100% sure.\n\nAlso, W3C says (well, in a local copy of RFC-2616) that \"All transfer-coding values are case-insensitive.\" Is this accounted? I thought we only forced case on keys of header dictionaries, but not the values.","commit_id":"61a49bec1ff69c60d14a5d94da3da4e849f28a31"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"38700ac824f3b9340c314a99eb083328793c3e0f","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        if \u0027x-storage-token\u0027 in req.headers and \\"},{"line_number":458,"context_line":"                \u0027x-auth-token\u0027 not in req.headers:"},{"line_number":459,"context_line":"            req.headers[\u0027x-auth-token\u0027] \u003d req.headers[\u0027x-storage-token\u0027]"},{"line_number":460,"context_line":"        if req.headers.get(\u0027transfer-encoding\u0027) \u003d\u003d \u0027chunked\u0027 and \\"},{"line_number":461,"context_line":"                \u0027content-length\u0027 in req.headers:"},{"line_number":462,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":463,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_457f68df","line":460,"in_reply_to":"3fa7e38b_be00bb02","updated":"2019-09-26 16:13:04.000000000","message":"Good call on both accounts!\n\nWe don\u0027t currently allow \u0027gzip, chunked\u0027 transfers, though I\u0027ve played around with that idea before (usually re-writing the header after swapping out the wsgi.input). Still, better to be robust to it.","commit_id":"61a49bec1ff69c60d14a5d94da3da4e849f28a31"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"2f3791efbc302ca0771664cf3fc08cb8a5f22798","unresolved":false,"context_lines":[{"line_number":458,"context_line":"                \u0027x-auth-token\u0027 not in req.headers:"},{"line_number":459,"context_line":"            req.headers[\u0027x-auth-token\u0027] \u003d req.headers[\u0027x-storage-token\u0027]"},{"line_number":460,"context_line":"        te \u003d req.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":461,"context_line":"        if te.rsplit(\u0027,\u0027, 1)[-1].strip() \u003d\u003d \u0027chunked\u0027 and \\"},{"line_number":462,"context_line":"                \u0027content-length\u0027 in req.headers:"},{"line_number":463,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":464,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_72efbd1d","line":461,"updated":"2019-09-27 03:57:14.000000000","message":"I have 2 questions:\n\n1. Why did you not use req.is_chunked()? Issues with an exception that it can throw?\n\n2. Do you happen to recall why exactly is_chunked() loops through headers? It does not appear to be ever called with a normal dictionary, it should be safe for it to just do [] or get().","commit_id":"ee405685c1ad6b50bd75df18064385fa77281953"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9078c9f9ba78847dde80996d2b65415134b68a37","unresolved":false,"context_lines":[{"line_number":458,"context_line":"                \u0027x-auth-token\u0027 not in req.headers:"},{"line_number":459,"context_line":"            req.headers[\u0027x-auth-token\u0027] \u003d req.headers[\u0027x-storage-token\u0027]"},{"line_number":460,"context_line":"        te \u003d req.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":461,"context_line":"        if te.rsplit(\u0027,\u0027, 1)[-1].strip() \u003d\u003d \u0027chunked\u0027 and \\"},{"line_number":462,"context_line":"                \u0027content-length\u0027 in req.headers:"},{"line_number":463,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":464,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f225ed09","line":461,"in_reply_to":"3fa7e38b_72efbd1d","updated":"2019-09-27 04:37:26.000000000","message":"1. If I remember right, I started with that, but as I started writing tests, I realized I hate the is_chunked() API. In particular, I dislike how it raises an AttributeError (!?) on unsupported encodings. AttributeError is supposed to *mean something* (to wit: I, as programmer, did something wrong). I thought about trying to fix that mess, but this seemed more surgical for a security fix.\n\n2. I\u0027m guessing because it\u0027s not really sure about case-sensitivity -- if it\u0027s only ever called with HeaderKeyDicts or HeaderEnvironProxys, so much the better.","commit_id":"ee405685c1ad6b50bd75df18064385fa77281953"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6555de239fdce5b1884146d595a05ccb27b7f5a8","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":464,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"},{"line_number":465,"context_line":"            # ought to ignore; that offers request-smuggling vectors."},{"line_number":466,"context_line":"            del req.headers[\u0027content-length\u0027]"},{"line_number":467,"context_line":"        return req"},{"line_number":468,"context_line":""},{"line_number":469,"context_line":"    def handle_request(self, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_50a2794d","line":466,"updated":"2019-10-02 06:32:40.000000000","message":"I didn\u0027t see any tests that test this condition. Ie send a request in chunked and with a content-length and show that it\u0027s been stripped.\n\nUnless I missed it.","commit_id":"ee405685c1ad6b50bd75df18064385fa77281953"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1c06abd4f0ea443fd687503b1da947719eb66eb2","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            # RFC says if both are present, transfer-encoding wins."},{"line_number":464,"context_line":"            # Definitely *don\u0027t* forward on the header the backend"},{"line_number":465,"context_line":"            # ought to ignore; that offers request-smuggling vectors."},{"line_number":466,"context_line":"            del req.headers[\u0027content-length\u0027]"},{"line_number":467,"context_line":"        return req"},{"line_number":468,"context_line":""},{"line_number":469,"context_line":"    def handle_request(self, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_2b5bcb54","line":466,"in_reply_to":"3fa7e38b_50a2794d","updated":"2019-10-02 15:21:22.000000000","message":"Done","commit_id":"ee405685c1ad6b50bd75df18064385fa77281953"}],"test/unit/proxy/controllers/test_obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b994eb7b6f5161e76e15706c3d2b83b5daaa7c37","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"                body \u003d unchunk_body(body)"},{"line_number":1105,"context_line":"                self.assertEqual(\u0027100-continue\u0027, headers[\u0027Expect\u0027])"},{"line_number":1106,"context_line":"                self.assertEqual(\u0027chunked\u0027, headers[\u0027Transfer-Encoding\u0027])"},{"line_number":1107,"context_line":"                self.assertNotIn(\u0027Content-Length\u0027, headers)"},{"line_number":1108,"context_line":"            else:"},{"line_number":1109,"context_line":"                self.assertNotIn(\u0027Transfer-Encoding\u0027, headers)"},{"line_number":1110,"context_line":"            if body or not test_body:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_5137d1e4","line":1107,"updated":"2019-10-02 18:29:26.000000000","message":"my favorite part of this assertion is that it fails on py2 and py3 - thank you","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b994eb7b6f5161e76e15706c3d2b83b5daaa7c37","unresolved":false,"context_lines":[{"line_number":3230,"context_line":"        fd.flush()"},{"line_number":3231,"context_line":"        headers \u003d readuntil2crlfs(fd)"},{"line_number":3232,"context_line":"        exp \u003d b\u0027HTTP/1.1 100 Continue\u0027"},{"line_number":3233,"context_line":"        self.assertEqual(headers[:len(exp)], exp)"},{"line_number":3234,"context_line":"        # Since we got our 100 Continue, now we can send the body"},{"line_number":3235,"context_line":"        fd.write(b\u00272\\r\\n\u0027"},{"line_number":3236,"context_line":"                 b\u0027oh\\r\\n\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_b6607b37","line":3233,"updated":"2019-10-02 18:29:26.000000000","message":"so this new proxy server pseudo-functional test is mainly to cover the change in wsgi, and the failure mode is kinda pretty gnarly - backend times out (20s OMM) and proxy coughs up 503?\n\nproxy ERROR: ERROR with Object server 127.0.0.1:58997/sda1 re: Expect: 100-continue on /a/c/o.chunked: ResponseTimeout (20.0s) (txn: tx6fd4b604d76e4ada84610-005d94e9aa)\nproxy ERROR: ERROR with Object server 127.0.0.1:50043/sdb1 re: Expect: 100-continue on /a/c/o.chunked: ResponseTimeout (20.0s) (txn: tx6fd4b604d76e4ada84610-005d94e9aa)\nproxy ERROR: Object PUT returning 503, 0/1 required connections (txn: tx6fd4b604d76e4ada84610-005d94e9aa) (client_ip: 127.0.0.1)\n\n\nOnly under py3 of course :eye_roll:","commit_id":"bf9346d88de2aeb06da3b2cde62ffa6200936367"}]}