)]}'
{".zuul.yaml":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":true,"context_lines":[{"line_number":282,"context_line":"      Setup a Swift/Keystone environment and run Swift\u0027s func tests."},{"line_number":283,"context_line":"    # TODO: Remove the nodeset pinning to focal once below bug is fixed"},{"line_number":284,"context_line":"    # https://bugs.launchpad.net/swift/+bug/1996627"},{"line_number":285,"context_line":"    nodeset: openstack-single-node-focal"},{"line_number":286,"context_line":"    required-projects:"},{"line_number":287,"context_line":"      - opendev.org/openstack/requirements"},{"line_number":288,"context_line":"      - opendev.org/openstack/swift"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"d1f69a00_ca7db219","side":"PARENT","line":285,"updated":"2022-12-01 16:45:52.000000000","message":"so *this* is a good thing, but it\u0027s not really blocking anyting I\u0027m aware of:\n\n    Now is better than never.\n    Although never is often better than *right* now.","commit_id":"280f460c80d2170f5e0c04e03605e3eebae2329c"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"94fa42824d95d8f79431cb97e37b9076cdf9eb30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"23ce93ec_b6421c25","updated":"2022-11-04 00:52:22.000000000","message":"Oh yeah, we\u0027ve also got some non-FIPS jobs that are failing in the experimental pipeline without this.\n\nProbably worth editing the commit message, though -- this goes beyond fixing tests and into preserving existing behaviors in spite of a hostile upstream.","commit_id":"e0992e7db005e12976b5d17eb771e331398bff16"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4f0edf2cef510bb6f9354511776b86080dddb39b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f638d0b1_9ec493ee","updated":"2022-11-03 00:25:13.000000000","message":"check experimental","commit_id":"e0992e7db005e12976b5d17eb771e331398bff16"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ce67f90f_649db0aa","updated":"2022-11-18 12:50:53.000000000","message":"In principle it seems we have to do something like this, so I\u0027m on board with that. But -1 because of the possibility of a None command, and I think the testing could get more targeted.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dfc4b6977b77fcd922a273c5b472d70333342f70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e7b282ec_24fb400e","updated":"2022-11-04 04:23:23.000000000","message":"check experimental","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28c35aeaef3f657cf04402ba56bd66f1158b5b21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"224f4775_bfddcbf1","updated":"2022-11-07 19:44:28.000000000","message":"recheck\n\nProposed https://review.opendev.org/c/openstack/swift/+/863670 to address the flakey test","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"45f38510ed2af146598f66d930869410804d0373","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3a64ac01_2c57267a","updated":"2022-11-08 00:42:24.000000000","message":"recheck\n\nRolling upgrade job is now non-voting: https://review.opendev.org/c/openstack/swift/+/863929","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16205282961663574004055c5d23aaddbcd59ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c4b67534_be1c9755","updated":"2022-11-07 20:31:36.000000000","message":"whatever, i guess if everything is broken this is fine.\n\nIf I was more principled I\u0027d say this is obviously a regression and push to get a revert upstream.  But even as lazy as I am, I wouldn\u0027t write THIS patch until after it became obvious 1) people need swift to work on this broken version of python and 2) python maintainers aren\u0027t going to fix their broken python releases - then MAYBE I\u0027d think the right thing to do is paper over their mess.\n\nDo you have an upstream bug report for this regression?  Do you have any reason to believe swift is alone in getting hozed by this recent regression or that everyone else plans to capitulate to stdlib shipping a broken http parser forever?  What python versions are effected - do we have to have gating tests on those versions?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"76b9c80cbfab079b73b5b3a27f2848fedb5fd435","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5257a857_073b7765","in_reply_to":"02600eb8_469df44b","updated":"2022-11-07 22:10:56.000000000","message":"\u003e I suppose I should at least write the bug, though...\n\nhttps://github.com/python/cpython/issues/99220","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"41415b568b9c658224201845d7b4305fb26414ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"02600eb8_469df44b","in_reply_to":"c4b67534_be1c9755","updated":"2022-11-07 21:14:35.000000000","message":"\u003e 1) people need swift to work on this broken version of python\n\nAs already linked, upstream\u0027s already released fixes to py37-py311. The only question now is how long it\u0027s going to take for distros to pick up the patch.\n\n\u003e 2) python maintainers aren\u0027t going to fix their broken python releases\n\nMy experience with submitting cpython bugs has me skeptical that this would ever get addressed. (My bellwethers have been https://bugs.python.org/issue33973 / https://github.com/python/cpython/pull/7932 and https://bugs.python.org/issue37093 / https://github.com/python/cpython/pull/13788 -- I was so hopeful years ago that phrases like \"request-smuggling vector\" could actually elicit some response.) I suppose I should at least write the bug, though...\n\n\u003e stdlib shipping a broken http parser forever\n\nThat\u0027s the thing -- even if we push stdlib to fix their issue better, in a way that doesn\u0027t break us by default -- they\u0027ve got releases out with the fix now. They have now *forever* shipped a broken parser that may get picked up by distros. Are we so sure we *don\u0027t* want to support them?\n\nI kinda dread having to say something like \"Oh yeah, swift supports py39 -- well, except 3.9.14 and 3.9.15 -- if you\u0027re on *them* make sure you upgrade to 3.9.16+\" and that\u0027s *before* we get to things like whatever crazy cherry-picking of patches RH (for example) might apply...","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d6e97eafdbb03f2f35ef899581e7ffb5b8240143","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5a776606_e0be19da","updated":"2022-11-21 06:30:18.000000000","message":"I see three options, none of which are great:\n\n* we do something brittle like this and continue using stdlib under the hood for HTTP parsing\n* we bring in a new dependency like h11 (either ourselves or by pushing eventlet to adopt it) and try to discover the new ways that parsing is broken (while probably needing to address https://bugs.launchpad.net/swift/+bug/1496636 much sooner than we have thus far had the will to do)\n* we write our own HTTP parser, which is likely to be chock full of its own bugs\n\nDevil-you-know seems like it may well be least-bad :-/","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"250f4bd3b815963888a30136ee2c797c111f78c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ba6de3d0_054f7da8","updated":"2022-12-02 09:29:47.000000000","message":"I understood from earlier discussion (maybe verbal) that the cpython \"feature\" has shipped, back-ported, and will forever be in some versions of python. If that understanding is correct then we either stop supporting a bunch of python versions (all future until the cpython changes), or we need to take some action.\n\nAssuming we choose to take some action, then there\u0027s obviously differing opinions as to what code we should write and maintain. Is anyone able to work on an alternative patchset?","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"51be8952_6ca5e100","updated":"2022-12-01 16:45:52.000000000","message":"Pete, thanks for looking at this!  Do you have any pull with red hat or cpython maintainers that might get eyes on https://github.com/python/cpython/issues/99220\n\nThe one thing we haven\u0027t done is tried to find a more reasonable fix for https://github.com/python/cpython/issues/87389 (a bug in the *server*) that accidently tricked them into thinking they can break the *protocol* handler.\n\nInstead of stripping the leading /\u0027s before they ever get to the environ they should have just gone into to the thing that wasn\u0027t properly sanitizing it\u0027s inputs before sending them to the filesystem.  Fix THAT and you don\u0027t need to break the parser.  It might be easy:\n\nhttps://github.com/python/cpython/blob/main/Lib/http/server.py#L834","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0af378a073613a2305294efbfc5a7c44a444ea9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"0ad5015f_63e19a8d","updated":"2022-11-30 17:24:15.000000000","message":"This seems like a necessary evil - we\u0027ve been forced into this kludge by the lib bug. Tim\u0027s concern about breaking out methods is reasonable, so I don\u0027t want to stand in the way for the sake of test elegance. It doesn\u0027t seem any of us came up with a smarter way to count number of \u0027/\u0027 at the start of string.","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f1cfe4916db2fb63780d609335e73cc003b0dbe7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"50a471a0_08d42910","in_reply_to":"51be8952_6ca5e100","updated":"2022-12-01 18:32:31.000000000","message":"It does sound better to me to fix the file system issue closer to the file system in the stack.\n\nI\u0027ll see what I can do about talking to our Python people. But they seem to be only doing packaging.","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"}],"swift/common/wsgi.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":true,"context_lines":[{"line_number":457,"context_line":"                parts[1] \u003d path + q + query"},{"line_number":458,"context_line":"                self.raw_requestline \u003d b\u0027 \u0027.join(parts)"},{"line_number":459,"context_line":"            # else, mangled protocol, most likely; let base class deal with it"},{"line_number":460,"context_line":"        retval \u003d wsgi.HttpProtocol.parse_request(self)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # Upstream CPython started discarding information in response to a CVE"},{"line_number":463,"context_line":"        # but we shouldn\u0027t be vulnerable to it. See"}],"source_content_type":"text/x-python","patch_set":5,"id":"8cea70f0_532d94a1","line":460,"updated":"2022-11-18 12:50:53.000000000","message":"why not return here if retval is False?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":false,"context_lines":[{"line_number":457,"context_line":"                parts[1] \u003d path + q + query"},{"line_number":458,"context_line":"                self.raw_requestline \u003d b\u0027 \u0027.join(parts)"},{"line_number":459,"context_line":"            # else, mangled protocol, most likely; let base class deal with it"},{"line_number":460,"context_line":"        retval \u003d wsgi.HttpProtocol.parse_request(self)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # Upstream CPython started discarding information in response to a CVE"},{"line_number":463,"context_line":"        # but we shouldn\u0027t be vulnerable to it. See"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f1dc2c7_87a823cc","line":460,"in_reply_to":"8cea70f0_532d94a1","updated":"2022-12-01 16:45:52.000000000","message":"Done","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":true,"context_lines":[{"line_number":464,"context_line":"        # https://github.com/python/cpython/issues/87389 and"},{"line_number":465,"context_line":"        # python-security.readthedocs.io/vuln/http-server-redirection.html"},{"line_number":466,"context_line":"        if self.requestline.startswith(\u0027%s //\u0027 % self.command):"},{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"}],"source_content_type":"text/x-python","patch_set":5,"id":"73602c38_9fb1d7af","line":467,"updated":"2022-11-18 12:50:53.000000000","message":"I think self.command can be None\n\nso this crazy test blows up:\n\n  diff --git a/test/unit/common/test_wsgi.py \n  b/test/unit/common/test_wsgi.py\n  index 59e6aff7b..1ab2c2cbc 100644\n  --- a/test/unit/common/test_wsgi.py\n  +++ b/test/unit/common/test_wsgi.py\n  @@ -1122,6 +1122,11 @@ class TestSwiftHttpProtocol(unittest.TestCase):\n           ], proto_obj.send_error.mock_calls)\n           self.assertEqual((\u0027a\u0027, \u0027123\u0027), proto_obj.client_address)\n   \n  +    def test_bad_request_line(self):\n  +        proto_obj \u003d self._proto_obj()\n  +        proto_obj.raw_requestline \u003d b\u0027None //\u0027\n  +        self.assertEqual(False, proto_obj.parse_request())\n  +\n       def test_request_line_cleanup(self):\n           def do_test(line_from_socket, expected_line\u003dNone):\n               if expected_line is None:","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"250f4bd3b815963888a30136ee2c797c111f78c6","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        # https://github.com/python/cpython/issues/87389 and"},{"line_number":465,"context_line":"        # python-security.readthedocs.io/vuln/http-server-redirection.html"},{"line_number":466,"context_line":"        if self.requestline.startswith(\u0027%s //\u0027 % self.command):"},{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"}],"source_content_type":"text/x-python","patch_set":5,"id":"18be1c8e_b6cb1206","line":467,"in_reply_to":"73602c38_9fb1d7af","updated":"2022-12-02 09:29:47.000000000","message":"Done","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16205282961663574004055c5d23aaddbcd59ab","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"2ff11fc9_799e8ad0","line":470,"updated":"2022-11-07 20:31:36.000000000","message":"this is dumb way to count up how many `/`\n\n... but everything else i can come up with is dumber.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"021bb804a2ae3ee65b2bdcc4b33e127e949cc29e","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"d8e04065_b3476bfa","line":470,"in_reply_to":"0e6d4af1_0062dec4","updated":"2022-12-01 05:08:32.000000000","message":"I considered using regex -- didn\u0027t for a few reasons:\n\n- extra import\n- a regex would essentially be doing the same thing but in a more general-purpose manner, so it\u0027s not clear to me that it would be a performance win -- though maybe the fact that so much more of the heavy lifting is in C would help?\n- it\u0027s much harder to convince myself that the implementation is correct -- https://xkcd.com/1171/\n\nIn fact, I see a bug already -- we\u0027d need to make sure there\u0027s actually a match, or change the regex to use * instead of +. (And I need to revisit https://review.opendev.org/c/openstack/swift/+/856327 to support absolute-form request paths...)","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"dbdf1942c9150c48d852b1003d8d1f8d69e25f58","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b5dc10f3_d3c5051c","line":470,"in_reply_to":"2ff11fc9_799e8ad0","updated":"2022-11-17 18:21:03.000000000","message":"An interesting challenge. Matching \" [/]+\" and len() on result may work, but it\u0027s cumbersome too.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"da5193466bc8b4dea90c8dd81ca2858a8a138622","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"0e6d4af1_0062dec4","line":470,"in_reply_to":"541775f4_76d0f7f3","updated":"2022-12-01 00:04:17.000000000","message":"Or not count at all, and use a RE simple match that only looks at the start of the string and the RE precompiled so it doesn\u0027t take very long, something like: https://paste.opendev.org/show/817823/\n\nTried to keep it as quick as possible by pre-compiling, very specific search from the start etc.. though don\u0027t really know how it compares speed wise.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"69e7a5e4a2e5307c7aaf63fcfa4ee2d7117fc5ad","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"2313b7bb_18eaea3a","line":470,"in_reply_to":"b0376fde_ce5e65a9","updated":"2022-12-01 22:59:53.000000000","message":"\u003e In fact, I see a bug already -- we\u0027d need to make sure there\u0027s actually a match, or change the regex to use * instead of +. (And I need to revisit \n\nWell it only runs the match if there is a \u0027//\u0027 (post the startswith check) so we know there will be a  + match, and am only matching from the start of the string, so it wont go searching down the line.\nBut yeah good point.\n\nHmm, it is an interesting thought from clay. I guess it does mean we\u0027d have to maintain that function our selves.. but maybe it\u0027s a small price to pay.\n\nOn the otherhad we now already have 2 +2\u0027s on this patch as it is.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"541775f4_76d0f7f3","line":470,"in_reply_to":"b5dc10f3_d3c5051c","updated":"2022-11-18 12:50:53.000000000","message":"IDK if its any better/faster:\n\n  part_line \u003d self.requestline[len(self.command) + 1:]\n  prefix \u003d \u0027/\u0027 * (len(part_line) - len(part_line.lstrip(\u0027/\u0027)))","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"},{"line_number":468,"context_line":"                if c !\u003d \u0027/\u0027:"},{"line_number":469,"context_line":"                    break"},{"line_number":470,"context_line":"            prefix \u003d \u0027/\u0027 * i"},{"line_number":471,"context_line":"            if not self.path.startswith(prefix):"},{"line_number":472,"context_line":"                # This should still trigger a 404 down in proxy/server.py"},{"line_number":473,"context_line":"                self.path \u003d prefix + self.path.lstrip(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b0376fde_ce5e65a9","line":470,"in_reply_to":"d8e04065_b3476bfa","updated":"2022-12-01 16:45:52.000000000","message":"This is not the way. I think we\u0027d be way smarter/happier long term to just inline the entire non-broken parse_request method from before it grew a stupid regex in the middle:\n\nhttps://github.com/python/cpython/pull/24848/files#diff-10e6db6c775e351ba2b55c4ad5e177191afe25d4942d1d2162d6e10505788a81R340\n\n... we can even inline our py3 bytes_to_wsgi translation, SAVE a stack trace and call the whole investment an optimization.\n\n“Find the dependencies — and eliminate them.” When you’re working on a really, really good team with great programmers, everybody else’s code, frankly, is bug-infested garbage, and nobody else knows how to ship on time. \n\nhttps://www.joelonsoftware.com/2001/10/14/in-defense-of-not-invented-here-syndrome/","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":true,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        # Upstream CPython started discarding information in response to a CVE"},{"line_number":465,"context_line":"        # but we shouldn\u0027t be vulnerable to it. See"},{"line_number":466,"context_line":"        # https://github.com/python/cpython/issues/87389 and"},{"line_number":467,"context_line":"        # python-security.readthedocs.io/vuln/http-server-redirection.html"},{"line_number":468,"context_line":"        if self.requestline.startswith(self.command + \u0027 //\u0027):"},{"line_number":469,"context_line":"            for i, c in enumerate(self.requestline[len(self.command) + 1:]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"8d6c70a1_b1078f27","line":466,"updated":"2022-12-01 16:45:52.000000000","message":"this is the wrong issue to reference.  I get it - they had a CVE to deal with there was a new contributer and they assumed no-one was doing anything remotely serious with stdlib httpserver stack, but eventlet.wsgi and wsgiref do, so the \"bug\" that\u0027s causing us to carry this inline runtime hack is:\n\nhttps://github.com/python/cpython/issues/99220\n\n... and maybe no one cares?\n\nIf the path was more clear to me how eventlet (and other greenlet/stack-swapping) platforms were going to evolve into or cooperate with modern asyncio eventloops - I\u0027d say we should invest in *fixing* eventlet.wsgi to not depend on broken http parsers.  There\u0027s good options out there:\n\n    * https://github.com/python-hyper/h11/\n    * https://github.com/benoitc/gunicorn/\n\n... but since the path is not clear to me, and eventlet might be just as barely \"maintained\" as the stdlib http parser - maybe instead of \"fixing\" eventlet.wsgi we could just open a bug to try and get more eyes on the cpyhton bug?","commit_id":"0a875d91355ff19b5cb34e55d8c1c32d755f553e"}],"test/unit/common/test_wsgi.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":true,"context_lines":[{"line_number":1134,"context_line":"                proto_obj.requestline \u003d ("},{"line_number":1135,"context_line":"                    line_from_socket if six.PY2"},{"line_number":1136,"context_line":"                    else line_from_socket.decode(\u0027latin1\u0027))"},{"line_number":1137,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1140,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"90278225_c75813a5","line":1137,"updated":"2022-11-18 12:50:53.000000000","message":"the test scaffolding seems to be quite fragile: I wonder if the goal of this test might be better realised by breaking the cleanup code out to a help method and testing that directly?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":true,"context_lines":[{"line_number":1134,"context_line":"                proto_obj.requestline \u003d ("},{"line_number":1135,"context_line":"                    line_from_socket if six.PY2"},{"line_number":1136,"context_line":"                    else line_from_socket.decode(\u0027latin1\u0027))"},{"line_number":1137,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1140,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7ea45a81_9b654503","line":1137,"in_reply_to":"83e91248_b5d42909","updated":"2022-12-01 16:45:52.000000000","message":"I can\u0027t follow this thread - it sounds like we\u0027re talking about two different things.  The test does look dumb to me on the surface and I don\u0027t think we can implicitly blame that on the unit under test.  The more that we\u0027re going to have to maintain the behaviors we want from our http parser w/o 3rd party changes breaking us the more we\u0027ll have to test (and refactor to be more testable).","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d6e97eafdbb03f2f35ef899581e7ffb5b8240143","unresolved":true,"context_lines":[{"line_number":1134,"context_line":"                proto_obj.requestline \u003d ("},{"line_number":1135,"context_line":"                    line_from_socket if six.PY2"},{"line_number":1136,"context_line":"                    else line_from_socket.decode(\u0027latin1\u0027))"},{"line_number":1137,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1140,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"83e91248_b5d42909","line":1137,"in_reply_to":"90278225_c75813a5","updated":"2022-11-21 06:30:18.000000000","message":"I think that\u0027s mostly because *our current HTTP parsing* is quite fragile -- as evidenced by this breakage from something that\u0027s two steps removed from us! It\u0027s duct-tape, paperclips, and shoe string the whole way down :-(\n\nAs to the tactics: I think the only way to have the helper-method approach improve things is to have the `if self.requestline.startswith(...):` inside the helper and always call the helper... but IDK that I really want to add the extra function call at such a low level of every request. I suppose there are already plenty of comparable function calls, but surely at some point it starts to have a noticeable impact, no?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16205282961663574004055c5d23aaddbcd59ab","unresolved":true,"context_lines":[{"line_number":1171,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1172,"context_line":"                # See https://github.com/python/cpython/issues/87389"},{"line_number":1173,"context_line":"                proto_obj.path \u003d \u0027/\u0027 + proto_obj.requestline.split("},{"line_number":1174,"context_line":"                    \u0027 \u0027, 2)[1].lstrip(\u0027/\u0027)"},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1177,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"200c25f7_b23801ed","line":1174,"updated":"2022-11-07 20:31:36.000000000","message":"we\u0027re litterally installing the bad behavior we\u0027re claiming exists in our depdendency so that we can assert we fix it.\n\nthis test doesn\u0027t seem reasonable to me.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"41415b568b9c658224201845d7b4305fb26414ef","unresolved":true,"context_lines":[{"line_number":1171,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1172,"context_line":"                # See https://github.com/python/cpython/issues/87389"},{"line_number":1173,"context_line":"                proto_obj.path \u003d \u0027/\u0027 + proto_obj.requestline.split("},{"line_number":1174,"context_line":"                    \u0027 \u0027, 2)[1].lstrip(\u0027/\u0027)"},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1177,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"a91a1a8c_ff35ecbb","line":1174,"in_reply_to":"200c25f7_b23801ed","updated":"2022-11-07 21:14:35.000000000","message":"If we *did* convince upstream to fix the break, the test would no longer verify the fix! We might rip it all out, revert the whole patch, because our CI got an upgraded python -- despite there potentially being users out there still affected. I feel like I can\u0027t win here :-(","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b6ea151485da96c6a1bdf8ea90d92706ddfb5710","unresolved":true,"context_lines":[{"line_number":1171,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1172,"context_line":"                # See https://github.com/python/cpython/issues/87389"},{"line_number":1173,"context_line":"                proto_obj.path \u003d \u0027/\u0027 + proto_obj.requestline.split("},{"line_number":1174,"context_line":"                    \u0027 \u0027, 2)[1].lstrip(\u0027/\u0027)"},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1177,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"a604e161_d7879efc","line":1174,"in_reply_to":"a2b00b8d_d0d4308f","updated":"2022-12-01 16:45:52.000000000","message":"I think Al sounds like he has a pretty clear picture of some imporeoved behavior based testing that we could apply here - seems like sound technical investment to me.  If not *right now* does the oppertunity cost calculus change after the new year?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"dbdf1942c9150c48d852b1003d8d1f8d69e25f58","unresolved":true,"context_lines":[{"line_number":1171,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1172,"context_line":"                # See https://github.com/python/cpython/issues/87389"},{"line_number":1173,"context_line":"                proto_obj.path \u003d \u0027/\u0027 + proto_obj.requestline.split("},{"line_number":1174,"context_line":"                    \u0027 \u0027, 2)[1].lstrip(\u0027/\u0027)"},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1177,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2dc643_15ce4b17","line":1174,"in_reply_to":"a91a1a8c_ff35ecbb","updated":"2022-11-17 18:21:03.000000000","message":"So, what\u0027s the answer? Maybe it\u0027s \"not test this at all\" and if something breaks we\u0027ll know?","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a63a7d9d33538b7c226767c74f210cb73e0d955b","unresolved":true,"context_lines":[{"line_number":1171,"context_line":"                proto_obj.command \u003d proto_obj.requestline.split(\u0027 \u0027, 1)[0]"},{"line_number":1172,"context_line":"                # See https://github.com/python/cpython/issues/87389"},{"line_number":1173,"context_line":"                proto_obj.path \u003d \u0027/\u0027 + proto_obj.requestline.split("},{"line_number":1174,"context_line":"                    \u0027 \u0027, 2)[1].lstrip(\u0027/\u0027)"},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"            with mock.patch(\u0027swift.common.wsgi.wsgi.HttpProtocol\u0027) \\"},{"line_number":1177,"context_line":"                    as mock_super:"}],"source_content_type":"text/x-python","patch_set":5,"id":"a2b00b8d_d0d4308f","line":1174,"in_reply_to":"ba2dc643_15ce4b17","updated":"2022-11-18 12:50:53.000000000","message":"as above, maybe create a separate method to do the \u0027/\u0027-mangling and test it directly? then a test that asserts the various helper methods are called.","commit_id":"7b296275b12f6d63e1aa5c78fe65eb355eee0080"}]}
