S3 list_object_versions - ensure the prefix can contain a plus (#5097)

.github/workflows/test_outdated_versions.yml

@@ -16,6 +16,7 @@ jobs:
         python-version: [ "3.10" ]
         responses-version: ["0.11.0", "0.12.0", "0.13.0", "0.15.0", "0.17.0", "0.19.0" ]
         mock-version: [ "3.0.5", "4.0.0", "4.0.3" ]
+        werkzeug-version: ["2.0.1", "2.1.1"]
 
     steps:
     - name: Checkout repository
@@ -35,7 +36,8 @@ jobs:
         pip install -r requirements-dev.txt
         pip install responses==${{ matrix.responses-version }}
         pip install mock==${{ matrix.mock-version }}
+        pip install werkzeug==${{ matrix.werkzeug-version }}
 
     - name: Run tests
       run: |
-        pytest -sv tests/test_core ./tests/test_apigateway/test_apigateway_integration.py
+        pytest -sv tests/test_core ./tests/test_apigateway/test_apigateway_integration.py ./tests/test_s3/test_server.py

moto/s3/responses.py

@@ -332,7 +332,16 @@ class ResponseObject(_TemplateEnvironmentMixin, ActionAuthenticatorMixin):
     @staticmethod
     def _get_querystring(full_url):
         parsed_url = urlparse(full_url)
-        querystring = parse_qs(parsed_url.query, keep_blank_values=True)
+        # full_url can be one of two formats, depending on the version of werkzeug used:
+        # http://foobaz.localhost:5000/?prefix=bar%2Bbaz
+        # http://foobaz.localhost:5000/?prefix=bar+baz
+        # Werkzeug helpfully encodes the plus-sign for us, from >= 2.1.0
+        # However, the `parse_qs` method will (correctly) replace '+' with a space
+        #
+        # Workaround - manually reverse the encoding.
+        # Keep the + encoded, ensuring that parse_qsl doesn't replace it, and parse_qsl will unquote it afterwards
+        qs = (parsed_url.query or "").replace("+", "%2B")
+        querystring = parse_qs(qs, keep_blank_values=True)
         return querystring
 
     def _bucket_response_head(self, bucket_name, querystring):

tests/test_s3/test_s3.py

@@ -3154,21 +3154,22 @@ if settings.TEST_SERVER_MODE:
 
 
 @mock_s3
-def test_get_object_versions_with_prefix():
+@pytest.mark.parametrize("prefix", ["file", "file+else", "file&another"])
+def test_get_object_versions_with_prefix(prefix):
     bucket_name = "testbucket-3113"
     s3_resource = boto3.resource("s3", region_name=DEFAULT_REGION_NAME)
     s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
     s3_client.create_bucket(Bucket=bucket_name)
     bucket_versioning = s3_resource.BucketVersioning(bucket_name)
     bucket_versioning.enable()
-    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key="file.txt")
-    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key="file.txt")
-    s3_client.put_object(Bucket=bucket_name, Body=b"alttest", Key="altfile.txt")
-    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key="file.txt")
+    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key=f"{prefix}.txt")
+    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key=f"{prefix}.txt")
+    s3_client.put_object(Bucket=bucket_name, Body=b"alttest", Key=f"alt{prefix}.txt")
+    s3_client.put_object(Bucket=bucket_name, Body=b"test", Key=f"{prefix}.txt")
 
-    versions = s3_client.list_object_versions(Bucket=bucket_name, Prefix="file")
+    versions = s3_client.list_object_versions(Bucket=bucket_name, Prefix=prefix)
     versions["Versions"].should.have.length_of(3)
-    versions["Prefix"].should.equal("file")
+    versions["Prefix"].should.equal(prefix)
 
 
 @mock_s3

tests/test_s3/test_server.py

@@ -45,13 +45,22 @@ def test_s3_server_bucket_create():
     res.status_code.should.equal(200)
     res.data.should.contain(b"ListBucketResult")
 
-    res = test_client.put("/bar", "http://foobaz.localhost:5000/", data="test value")
-    res.status_code.should.equal(200)
-    assert "ETag" in dict(res.headers)
+    for key_name in ("bar_baz", "bar+baz"):
+        res = test_client.put(
+            f"/{key_name}", "http://foobaz.localhost:5000/", data="test value"
+        )
+        res.status_code.should.equal(200)
+        assert "ETag" in dict(res.headers)
 
-    res = test_client.get("/bar", "http://foobaz.localhost:5000/")
-    res.status_code.should.equal(200)
-    res.data.should.equal(b"test value")
+        res = test_client.get(
+            "/", "http://foobaz.localhost:5000/", query_string={"prefix": key_name}
+        )
+        res.status_code.should.equal(200)
+        res.data.should.contain(b"Contents")
+
+        res = test_client.get(f"/{key_name}", "http://foobaz.localhost:5000/")
+        res.status_code.should.equal(200)
+        res.data.should.equal(b"test value")
 
 
 def test_s3_server_ignore_subdomain_for_bucketnames():