Skip to content

vary should be set to 'Accept' in the response header #59

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

Closed
wants to merge 2 commits into from

Conversation

omrisiri
Copy link

@omrisiri omrisiri commented Oct 7, 2018

When enabling webp we need to return vary: 'Accept' header rather than take it from the request headers since it doesn't exist there and will throw an exception

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@timkelty
Copy link

timkelty commented Jan 9, 2019

I'm trying to get AUTO_WEBP to work…https://github.com/thumbor/thumbor/wiki/Configuration#auto_webp

Is there an update on this issue or a reason it isn't merged in? @omrisiri @gsingh04

@timkelty timkelty mentioned this pull request Jan 11, 2019
@timkelty
Copy link

@omrisiri Were you able to get this working (AUTO_WEBP)?

I merged tested this PR, and confirmed it does work when you access the API directly (when setting AUTO_WEBP), but doesn't work through the cloudfront url.

Example:

Request png w/ accept header, directly from api: get back webp

❯ curl -I --header "accept: image/webp,image/*,*/*;q=0.8" "https://i308nq9jlf.execute-api.us-east-1.amazonaws.com/image/950x950/test.png"
HTTP/2 200
content-type: image/webp
content-length: 111482

Request png w/ accept header, from cloudfront url: get back png:

❯ curl -I --header "accept: image/webp,image/*,*/*;q=0.8" "https://ds3bnhnnjgxql.cloudfront.net/950x950/test.png"
HTTP/2 200
content-type: image/png
content-length: 241881

@omrisiri
Copy link
Author

In order for this to work with cloudfront you will need to whitelist the Accept header in the behaviors tab in the distribution settings

@timkelty
Copy link

if thumbor_response.status_code != 200:
return response_formater(status_code=thumbor_response.status_code)
if vary:
vary = thumbor_response.headers['vary']
vary = 'Accept'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@omrisiri We don't need to change anything here. If we forward the headers from cloudfront (https://aws.amazon.com/premiumsupport/knowledge-center/configure-cloudfront-to-forward-headers/ choose accept instead of host), this will work as it is. And I think we will have to forward the headers anyway since the thumbor needs to know the header value.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jainraj
Copy link

jainraj commented Jan 25, 2019

Request the moderators to close the PR and add in documentation that we will need to forward ACCEPT header in CloudFront for AUTO_WEBP to work as expected

@timkelty
Copy link

timkelty commented May 1, 2019

I can verify @jainraj's solution works. Shouldn't we add the Accepts whitelist header to the cloudfront dist as part of the default configuration?

@timkelty
Copy link

timkelty commented May 8, 2019

Just came across an issue using @jainraj's solution (adding the Accept header to whitelist). It throws an error when you try to set the format via the format() filter. I end up with an error like:

[ERROR]	2019-05-08T17:32:50.341Z	43028d25-8143-4d69-90f2-550e86bbc35a	lambda_handler trace: Traceback (most recent call last):
File "/var/task/image_handler/lambda_function.py", line 338, in lambda_handler
result = call_thumbor(event)
File "/var/task/image_handler/lambda_function.py", line 317, in call_thumbor
return process_thumbor_responde(thumbor_response, vary)
File "/var/task/image_handler/lambda_function.py", line 287, in process_thumbor_responde
vary = thumbor_response.headers['vary']
File "/var/task/requests/structures.py", line 52, in __getitem__
return self._store[key.lower()][1]
KeyError: 'vary'

@shsenior
Copy link
Contributor

Resolved with v4.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants