Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Using double quotes around etag in If-None-Match changes content-type in response #5345

Closed
1 task done
p-himik opened this issue Jan 24, 2022 · 6 comments · Fixed by getmoto/moto#5715 or #8630
Closed
1 task done
Assignees
Labels
aws:s3 Amazon Simple Storage Service status: resolved/fixed Resolved via a fix or an implementation

Comments

@p-himik
Copy link

p-himik commented Jan 24, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Using double quotes around etag in If-None-Match changes content-type in response

Expected Behavior

Double quotes around etag should not affect anything. In fact, they should be the only thing that works since double quotes around etag in If-None-Match are required.

How are you starting LocalStack?

With a docker run command

Steps To Reproduce

How are you starting localstack (e.g., bin/localstack command, arguments, or docker-compose.yml)

docker run localstack/localstack

Client commands (e.g., AWS SDK code snippet, or sequence of "awslocal" commands)

awslocal s3 mb s3://etag-test
# Generating a 1 second silent mp3, but any mp3 will do
ffmpeg -f lavfi -i anullsrc=r=44100:cl=mono -t 1 -b:a 32k -acodec libmp3lame silence.mp3
awslocal s3 cp silence.mp3 s3://etag-test
curl -v http://localhost:4566/etag-test/silence.mp3 --output /dev/null # [1]
# Using etag from [1] above
curl -H 'If-None-Match: 5782c73c28075f7db7c89348ad2057a7' -v http://localhost:4566/etag-test/silence.mp3 --output /dev/null  # [2]
curl -H 'If-None-Match: "5782c73c28075f7db7c89348ad2057a7"' -v http://localhost:4566/etag-test/silence.mp3 --output /dev/null  # [3]

Here's a diff of headers from [2] and [3] above:

 > Host: localhost:4566
 > User-Agent: curl/7.74.0
 > Accept: */*
-> If-None-Match: 5782c73c28075f7db7c89348ad2057a7
+> If-None-Match: "5782c73c28075f7db7c89348ad2057a7"
 > 
 * Mark bundle as not supporting multiuse
 < HTTP/1.1 304 
-< content-type: audio/mpeg
+< Content-Type: text/html; charset=utf-8
 < Content-Length: 0
-< content-md5: V4LHPCgHX323yJNIrSBXpw==
-< ETag: "5782c73c28075f7db7c89348ad2057a7"
-< last-modified: Mon, 24 Jan 2022 17:07:55 GMT
-< x-amzn-requestid: YTSC0AMTYZ8HXL8Q67VW0IVYDWNFD9LSFCGQTOBTWEVLEKA5WD2D
+< x-amzn-requestid: IKVLP5J78MYPYRPROKS0LE32DGOGP70RF81F0KYV1LZP81ZIMWR3
 < Access-Control-Allow-Origin: *
-< x-amz-request-id: 8D2C27B576108EB8
-< x-amz-id-2: MzRISOwyjmnup8D2C27B576108EB87/JypPGXLh0OVFGcJaaO3KW/hRAqKOpIEEp
+< Last-Modified: Mon, 24 Jan 2022 17:08:20 GMT
+< x-amz-request-id: 17718C1584A8A051
+< x-amz-id-2: MzRISOwyjmnup17718C1584A8A0517/JypPGXLh0OVFGcJaaO3KW/hRAqKOpIEEp
 < accept-ranges: bytes
 < content-language: en-US
 < Access-Control-Allow-Methods: HEAD,GET,PUT,POST,DELETE,OPTIONS,PATCH

Notice how some other headers have also changed or even disappeared.

Environment

- OS: Ubuntu 21.10
- LocalStack: latest (ef3e5d10562c)

Anything else?

No response

@p-himik p-himik added type: bug Bug report status: triage needed Requires evaluation by maintainers labels Jan 24, 2022
@p-himik
Copy link
Author

p-himik commented Jan 24, 2022

This seems relevant:

def fix_range_content_type(bucket_name, path, headers, response):
# Fix content type for Range requests - https://github.com/localstack/localstack/issues/1259
if "Range" not in headers:
return
if response.status_code >= 400:
return
s3_client = aws_stack.connect_to_service("s3")
path = urlparse.urlparse(urlparse.unquote(path)).path
key_name = extract_key_name(headers, path)
result = s3_client.head_object(Bucket=bucket_name, Key=key_name)
content_type = result["ContentType"]
if response.headers.get("Content-Type") == "text/html; charset=utf-8":
response.headers["Content-Type"] = content_type

The fix of the Range issue suggests that there's a deeper root cause that set the header to text/html; charset=utf-8 in the first place.

@p-himik
Copy link
Author

p-himik commented Jan 24, 2022

An additional observation - apparently, it's invalid to specify Content-Length: 0 when the actual data has non-zero size:
https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2

A server MAY send a Content-Length header field in a 304 (Not
Modified) response to a conditional GET request (Section 4.1 of
[RFC7232]); a server MUST NOT send Content-Length in such a response
unless its field-value equals the decimal number of octets that would
have been sent in the payload body of a 200 (OK) response to the same
request.

@whummer whummer added the aws:s3 Amazon Simple Storage Service label Jan 25, 2022
@thrau thrau removed their assignment Sep 3, 2022
@macnev2013 macnev2013 self-assigned this Nov 28, 2022
@macnev2013 macnev2013 removed the status: triage needed Requires evaluation by maintainers label Nov 29, 2022
@bentsku
Copy link
Contributor

bentsku commented Apr 7, 2023

Hi @p-himik and thanks for the report! Could you confirm you're not encountering the issue anymore with our latest version? You can give it a try by either pulling latest or 2.0.1. Thanks!

@bentsku bentsku added the status: response required Waiting for a response from the reporter label Apr 7, 2023
@p-himik
Copy link
Author

p-himik commented Apr 7, 2023

Sorry, it'd be quite an effort to try it out as I've switched from Localstack to MinIO in my projects some time ago.

@bentsku
Copy link
Contributor

bentsku commented Apr 7, 2023

I understand, thanks for the quick response! I'll quickly give it a try and if the reported issue seems resolved, I'll close the issue. Thanks again for the response!

@bentsku bentsku added status: triage needed Requires evaluation by maintainers and removed status: response required Waiting for a response from the reporter labels Apr 7, 2023
@bentsku
Copy link
Contributor

bentsku commented Jul 3, 2023

To come back to this issue, it seems now we don't return a ContentType at all anymore, surely due to the exception parsing with the 304.

$ curl -v http://localhost:4566/etag-test/7n9ule.jpg --output /dev/null 
* Connected to localhost (127.0.0.1) port 4566 (#0)
> GET /etag-test/7n9ule.jpg HTTP/1.1
> Host: localhost:4566
> User-Agent: curl/7.88.1
> Accept: */*
> 
< HTTP/1.1 200 
< Content-Type: image/jpeg
< accept-ranges: bytes
< Last-Modified: Mon, 03 Jul 2023 08:18:15 GMT
< Content-Length: 62442
< ETag: "06a5d934b998d8923102c804caf5bf17"
< x-amz-request-id: c5698e57-52cb-4e63-b783-efa268f40930
< x-amz-id-2: s9lzHYrFp76ZVxRcpX9+5cjAnEH2ROuNkd2BHfIa6UkFVdtjf5mKR3/eTPFvsiP/XV/VLi31234=
< Connection: close
< date: Mon, 03 Jul 2023 08:18:53 GMT
< server: hypercorn-h11
< 

$ curl -H 'If-None-Match: "06a5d934b998d8923102c804caf5bf17"' -v http://localhost:4566/etag-test/7n9ule.jpg --output /dev/null
* Connected to localhost (127.0.0.1) port 4566 (#0)
> GET /etag-test/7n9ule.jpg HTTP/1.1
> Host: localhost:4566
> User-Agent: curl/7.88.1
> Accept: */*
> If-None-Match: "06a5d934b998d8923102c804caf5bf17"
> 
< HTTP/1.1 304 
< x-amz-request-id: fbc14dac-8ecc-4b44-9d10-b06dd9849ace
< x-amz-id-2: s9lzHYrFp76ZVxRcpX9+5cjAnEH2ROuNkd2BHfIa6UkFVdtjf5mKR3/eTPFvsiP/XV/VLi31234=
< Connection: close
< date: Mon, 03 Jul 2023 08:20:53 GMT
< server: hypercorn-h11
< 

Also, we still accept no double quotes around the ETag. We will need to create an AWS validated test to see what headers are returned, and how we can improve parity here.

@bentsku bentsku added status: confirmed Bug report was confirmed and removed status: triage needed Requires evaluation by maintainers labels Jul 3, 2023
@macnev2013 macnev2013 added status: resolved/fixed Resolved via a fix or an implementation and removed type: bug Bug report status: confirmed Bug report was confirmed labels Jul 5, 2023
@bentsku bentsku linked a pull request Jul 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:s3 Amazon Simple Storage Service status: resolved/fixed Resolved via a fix or an implementation
Projects
None yet
5 participants