Repository: localstack/localstack
Pull request: Honor signed header requests
This adds support for signed requests that include response header overrides.

Fixes #1203

Problem statement:
response-content-disposition is not honored on download
When generating a pre-signed URL for download and specifying response-content-disposition to set a target filename, the actual download does not contain the Content-Disposition header therefore the download file does not have the requested filename.

I have confirmed that this works corrected in S3 itself.

I'd be happy to explore a fix if anyone can point me in the right direction/location.

Hints from issue discussion:

Commit message: Honor signed header requests

Fixes #1203

Patch to review:
diff --git a/localstack/services/s3/s3_listener.py b/localstack/services/s3/s3_listener.py
index 671ffda790534..376e5e6276915 100644
--- a/localstack/services/s3/s3_listener.py
+++ b/localstack/services/s3/s3_listener.py
@@ -649,6 +649,22 @@ def return_response(self, method, path, data, headers, response):
             except Exception:
                 pass
 
+            # Honor response header overrides
+            # https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html
+            if method == 'GET':
+                query_map = urlparse.parse_qs(parsed.query, keep_blank_values=True)
+                allowed_overrides = {
+                    'response-content-type': 'Content-Type',
+                    'response-content-language': 'Content-Language',
+                    'response-expires': 'Expires',
+                    'response-cache-control': 'Cache-Control',
+                    'response-content-disposition': 'Content-Disposition',
+                    'response-content-encoding': 'Content-Encoding',
+                }
+                for param_name, header_name in allowed_overrides.items():
+                    if param_name in query_map:
+                        response.headers[header_name] = query_map[param_name][0]
+
             # We need to un-pretty-print the XML, otherwise we run into this issue with Spark:
             # https://github.com/jserver/mock-s3/pull/9/files
             # https://github.com/localstack/localstack/issues/183
diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py
index e0e2b76c13dc0..e76d542185f21 100644
--- a/tests/integration/test_s3.py
+++ b/tests/integration/test_s3.py
@@ -320,6 +320,40 @@ def test_s3_get_response_headers(self):
         # clean up
         self._delete_bucket(bucket_name, [object_key])
 
+    def test_s3_get_response_header_overrides(self):
+        # Signed requests may include certain header overrides in the querystring
+        # https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html
+
+        bucket_name = 'test-bucket-%s' % short_uid()
+        self.s3_client.create_bucket(Bucket=bucket_name)
+
+        # put object
+        object_key = 'key-by-hostname'
+        self.s3_client.put_object(Bucket=bucket_name, Key=object_key, Body='something')
+
+        # get object and assert headers
+        url = self.s3_client.generate_presigned_url(
+            'get_object', Params={
+                'Bucket': bucket_name,
+                'Key': object_key,
+                'ResponseCacheControl': 'max-age=74',
+                'ResponseContentDisposition': 'attachment; filename="foo.jpg"',
+                'ResponseContentEncoding': 'identity',
+                'ResponseContentLanguage': 'de-DE',
+                'ResponseContentType': 'image/jpeg',
+                'ResponseExpires': 'Wed, 21 Oct 2015 07:28:00 GMT'}
+        )
+        response = requests.get(url, verify=False)
+
+        self.assertEqual(response.headers['cache-control'], 'max-age=74')
+        self.assertEqual(response.headers['content-disposition'], 'attachment; filename="foo.jpg"')
+        self.assertEqual(response.headers['content-encoding'], 'identity')
+        self.assertEqual(response.headers['content-language'], 'de-DE')
+        self.assertEqual(response.headers['content-type'], 'image/jpeg')
+        self.assertEqual(response.headers['expires'], '2015-10-21T07:28:00Z')
+        # clean up
+        self._delete_bucket(bucket_name, [object_key])
+
     def test_s3_copy_md5(self):
         bucket_name = 'test-bucket-%s' % short_uid()
         self.s3_client.create_bucket(Bucket=bucket_name)


Code review comments:
diff_hunk: @@ -649,6 +649,22 @@ def return_response(self, method, path, data, headers, response):
             except Exception:
                 pass
 
+            # Honor response header overrides
+            # https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html
+            if method == 'GET':
+                query_map = urlparse.parse_qs(parsed.query, keep_blank_values=True)
+                allowed_overrides = {
path: localstack/services/s3/s3_listener.py
text: @user1:
nitpick: could we move these to a top-level constant `ALLOWED_HEADER_OVERRIDES`? Not critical, but it would help us reduce the amount of "spaghetti code" we already have in here. :)

@author:
No problem at all, I'll make the change.