)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Florent Vennetier \u003cflorent.vennetier@ovhcloud.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-08-09 17:39:47 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"s3api: return InvalidURI (400) instead of 500 and traceback"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We have seen some broken handcrafted client application sending a"},{"line_number":10,"context_line":"complete URL instead of the object path, and things did not go well:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1d636940_c273f51a","line":7,"updated":"2022-08-09 19:12:42.000000000","message":"almost any solution is better than a traceback!","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We have seen some broken handcrafted client application sending a"},{"line_number":10,"context_line":"complete URL instead of the object path, and things did not go well:"},{"line_number":11,"context_line":"traceback and error 500. Now just tell them they are doing it wrong."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Id3b29107a5b3331c7078fb5e63f08e286a7371f9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fc92185_32121b5a","line":11,"updated":"2022-08-09 19:12:42.000000000","message":"haha.  Clients do the strangest things!  amirite!?","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":10,"context_line":"complete URL instead of the object path, and things did not go well:"},{"line_number":11,"context_line":"traceback and error 500. Now just tell them they are doing it wrong."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Id3b29107a5b3331c7078fb5e63f08e286a7371f9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"dfa3b0f7_147b3f55","line":13,"updated":"2022-08-09 19:12:42.000000000","message":"since this client/operator facing improvement - it sounds like it could benifit from an lp bug (for better tracking of upgrades/releases)\n\nAlso if there\u0027s a functional repo an lp bug is normally a good place to comment and discuss and they\u0027re a little easier to find for some kinds of swift users","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1b5acd4af6827915c9a84f1a96a945d57b3ad5f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62e38ead_f5a0b7d4","updated":"2022-09-07 20:38:13.000000000","message":"Digging into it more, I think we want to support the absolute-form URI -- Clay\u0027s got a test to exercise it now (https://review.opendev.org/c/openstack/swift/+/856329), and it passes against AWS. And from the RFC (https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2):\n\n   To allow for transition to the absolute-form for all requests in some\n   future version of HTTP, a server MUST accept the absolute-form in\n   requests, even though HTTP/1.1 clients will only send them in\n   requests to proxies.\n\nI pushed up https://review.opendev.org/c/openstack/swift/+/856327 to do the translation super-early -- think you\u0027d have a chance to try it out/review it?","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5214b463_979397d1","updated":"2022-08-09 19:12:42.000000000","message":"There is a few unit tests aimed at verifying the behavior of the proxy with \"on the wire\" clients:\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/proxy/test_server.py#L2703\n\nI couldn\u0027t find one that attempts to send eventlet.wsgi and the swift API an absolute URI.  I wouldn\u0027t be surprised at all if it doesn\u0027t work - but my reading of what I think is the current RFC says we should try to make it work?\n\nI\u0027m not sure how s3 responds either - if they return 400 then this might be perfect!  But if they DO allow clients to send absolute-uir (which might explain why there was a client doing it) - maybe we should start thinking about how we\u0027re going to support it!?  (It may be reasonable to return 4xx in the interm as long as swift requests don\u0027t have any behavior change)\n\nI\u0027m concnered the problem migth be bigger than described and the proposed fix is just plugging one of many holes?  I like *not throwing stack* in principle tho ;)","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7cc26dfc93802c23fd451c9242c0cd12e0287927","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ac8dab7a_d7c6be7e","updated":"2022-08-10 15:55:56.000000000","message":"yeah!\n\nwell, maybe... so the swift API it seems returns 404 - https://review.opendev.org/c/openstack/swift/+/852772/1 - better than a traceback! \n I\u0027m not sure the implications WRT correctness, but my lp bug search makes it look like no one has complained (yet?!)\n\nThe only remaining question I have is: what does s3 do when a client sends the absolute uri in the request line?  If people use s3 behind a forward proxy (like inside a corp network with HTTP_PROXY\u003dhttp://corp-proxy.example.com) it may have to support absolute uris (or maybe the proxy has to transform the request and strip them).\n\n","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":26274,"name":"Florent Vennetier","email":"florent.vennetier@ovhcloud.com","username":"fvennetier"},"change_message_id":"2710ec936d6ecef2cfddec78509674ddffc7c761","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5599d186_3ec63fe5","in_reply_to":"5214b463_979397d1","updated":"2022-08-10 10:04:13.000000000","message":"I was not aware that absolute URIs were accepted here.\nMaybe we should do something like this instead of denying the request?\n\n```\ndiff --git a/swift/common/utils.py b/swift/common/utils.py\nindex ab6615a4c..7dcf66a0b 100644\n--- a/swift/common/utils.py\n+++ b/swift/common/utils.py\n@@ -1634,7 +1634,8 @@ def link_fd_to_path(fd, target_path, dirs_created\u003d0, retries\u003d2, fsync\u003dTrue):\n             dirpath \u003d os.path.dirname(dirpath)\n \n \n-def split_path(path, minsegs\u003d1, maxsegs\u003dNone, rest_with_last\u003dFalse):\n+def split_path(path, minsegs\u003d1, maxsegs\u003dNone, rest_with_last\u003dFalse,\n+               allow_absolute_uri\u003dFalse):\n     \"\"\"\n     Validate and split the given HTTP request path.\n \n@@ -1655,6 +1656,10 @@ def split_path(path, minsegs\u003d1, maxsegs\u003dNone, rest_with_last\u003dFalse):\n               segments will return as None)\n     :raises ValueError: if given an invalid path\n     \"\"\"\n+    if allow_absolute_uri:\n+        # We are only interested in the path part.\n+        # Safe if there is no scheme.\n+        path \u003d urlparse(path).path\n     if not maxsegs:\n         maxsegs \u003d minsegs\n     if minsegs \u003e maxsegs:\n```","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d3de939a77ffffebed48306997b2dac074d1dc32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f6d8fb77_bcb96d69","in_reply_to":"ac8dab7a_d7c6be7e","updated":"2022-08-15 00:17:11.000000000","message":"I\u0027ve seen some clients/SDKs do this before: there\u0027s https://github.com/aws/aws-sdk-go/issues/944 and I seem to recall seeing some old versions of rclone that would send absolute-uris (though maybe it was just using the golang SDK).\n\nI still haven\u0027t tested it against AWS, but it seems likely that they\u0027d accept it. Probably a good candidate for something in the test/s3api/ tree, particularly now that those are run in the gate as an additional set of func tests.","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"}],"swift/common/middleware/s3api/s3request.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d3de939a77ffffebed48306997b2dac074d1dc32","unresolved":true,"context_lines":[{"line_number":403,"context_line":"        It won\u0027t require bucket name in canonical_uri for v4."},{"line_number":404,"context_line":"        \"\"\""},{"line_number":405,"context_line":"        return swob.wsgi_to_bytes(swob.wsgi_quote("},{"line_number":406,"context_line":"            self.environ.get(\u0027PATH_INFO\u0027, self.path), safe\u003d\u0027-_.~/\u0027))"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    def _canonical_request(self):"},{"line_number":409,"context_line":"        # prepare \u0027canonical_request\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"62ece1f6_2909c1ef","line":406,"updated":"2022-08-15 00:17:11.000000000","message":"Do we need to worry about this use of PATH_INFO, too? I guess at least it won\u0027t 500...","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":633,"context_line":"            raise InvalidURI(self.path)"},{"line_number":634,"context_line":""},{"line_number":635,"context_line":"        if self.bucket_in_host:"},{"line_number":636,"context_line":"            obj \u003d self.environ[\u0027PATH_INFO\u0027][1:] or None"},{"line_number":637,"context_line":"            return self.bucket_in_host, obj"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"412f1c8a_0a398506","line":636,"updated":"2022-08-09 19:12:42.000000000","message":"I guess we assumed eventlet.wsgi was normalizing all this for us","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d3de939a77ffffebed48306997b2dac074d1dc32","unresolved":true,"context_lines":[{"line_number":633,"context_line":"            raise InvalidURI(self.path)"},{"line_number":634,"context_line":""},{"line_number":635,"context_line":"        if self.bucket_in_host:"},{"line_number":636,"context_line":"            obj \u003d self.environ[\u0027PATH_INFO\u0027][1:] or None"},{"line_number":637,"context_line":"            return self.bucket_in_host, obj"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"23062c6f_6a9e569b","line":636,"in_reply_to":"412f1c8a_0a398506","updated":"2022-08-15 00:17:11.000000000","message":"I think eventlet\u0027s doing the right thing -- after all, what if you wanted to write an http proxy with eventlet as the server?","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":637,"context_line":"            return self.bucket_in_host, obj"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"        try:"},{"line_number":640,"context_line":"            bucket, obj \u003d self.split_path(0, 2, True)"},{"line_number":641,"context_line":"        except ValueError:"},{"line_number":642,"context_line":"            raise InvalidURI(self.path)"},{"line_number":643,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"253ce2d4_dcdcbaef","line":640,"updated":"2022-08-09 19:12:42.000000000","message":"I\u0027m a little disappointed that swift.common.utils.split_path can\u0027t handle absolute-uri\u0027s - it seems to assume path starts with a `/` (after segs \u003d path.split(\u0027/\u0027) any non empty segs[0] raises ValueError)","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"}],"test/unit/common/middleware/s3api/test_s3api.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a5d5f04f79954fe9e6be23354771ad764e25a8ed","unresolved":true,"context_lines":[{"line_number":673,"context_line":"            headers\u003d{\u0027Authorization\u0027: \u0027AWS test:tester:hmac\u0027,"},{"line_number":674,"context_line":"                     \u0027Date\u0027: self.get_date_header()})"},{"line_number":675,"context_line":"        req.environ[\u0027PATH_INFO\u0027] \u003d \\"},{"line_number":676,"context_line":"            \u0027https://s3api.cloud/bucket?delimiter\u003d\u0026prefix\u003d\u0027"},{"line_number":677,"context_line":"        status, headers, body \u003d self.call_s3api(req)"},{"line_number":678,"context_line":"        self.assertEqual(self._get_error_code(body), \u0027InvalidURI\u0027)"},{"line_number":679,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"58e9d5fc_232b9539","line":676,"updated":"2022-08-09 19:12:42.000000000","message":"oic, interesting - I think some kinds of http servers let you do this actually?\n\nhttps://www.rfc-editor.org/rfc/rfc9112.html#name-request-target\n\nI don\u0027t know what s3 does, or even what swift does exactly...","commit_id":"5be3a3155c4da2c5a74dead79f90974450275f7a"}]}
