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

Fix blank content-encoding when none is supplied #75

Merged
merged 1 commit into from
Apr 7, 2015

Conversation

fcheung
Copy link
Contributor

@fcheung fcheung commented Mar 21, 2015

Should help with fog/fog#3450

Signature v4 requires that we insert a content-encoding header when doing
a streaming upload. If there is no pre-existing content encoding we should
set it to identity or some client may be confused by a blank content encoding
@lautis
Copy link
Member

lautis commented Mar 22, 2015

With this the Content-Encoding header is set as

Content-Encoding:  identity

Note that there is two spaces between colon and identity. I'm not sure if that could have any effect.

@fcheung
Copy link
Contributor Author

fcheung commented Mar 22, 2015

@geemus
Copy link
Member

geemus commented Mar 23, 2015

@fcheung - seems good. Have you confirmed that it helps on #3450? Figure knowing that will make it clearer if we should pull it in or not. Thanks!

@fcheung
Copy link
Contributor Author

fcheung commented Mar 23, 2015

@geemus waiting for feedback

@geemus
Copy link
Member

geemus commented Mar 23, 2015

@fcheung Thanks, that was my impression, just double checking.

@fcheung
Copy link
Contributor Author

fcheung commented Apr 4, 2015

@geemus I think we're good to go on this

@geemus
Copy link
Member

geemus commented Apr 7, 2015

@fcheung thanks, bringing it in.

geemus added a commit that referenced this pull request Apr 7, 2015
Fix blank content-encoding when none is supplied
@geemus geemus merged commit d57984d into fog:master Apr 7, 2015
@jonbuda
Copy link

jonbuda commented Apr 9, 2015

Just to follow up on this, it appears as though 'Identity' should not be used for 'Content-Encoding', according to: http://tools.ietf.org/html/rfc2616#section-3.5

Thoughts? I'm not entirely what effect it has on things though, if it is present.

@wizjo
Copy link

wizjo commented May 6, 2015

We are experiencing a problem with this fix -- all the gzip files ended up with an empty Content-Encoding file header in S3. As a result, our CDN service couldn't determine the actual Content-Encoding and is serving assets in their original size. We reverted the change in our fork of gem "fog-aws" and are using that fork, but are there any plans to fix this issue down the road?

@fcheung
Copy link
Contributor Author

fcheung commented May 6, 2015

So you suffer from the opposite problem than the person that reported the original issue? If identity is an incorrect content encoding (which it seems it is) then we need to figure out what was causing the blank content encoding previously. I suppose it could happen if the content-encoding was explicitly set to an empty string, but not sure how that would happen

@wizjo
Copy link

wizjo commented May 6, 2015

@fcheung , this particular fix:

encoding = "aws-chunked, identity"
results in a blank "Content-Encoding" file header in S3, which is causing the CDN problem described above.
screen shot 2015-05-06 at 4 20 13 pm

@fcheung
Copy link
Contributor Author

fcheung commented May 7, 2015

I've created a pull request that changes it back and improves some of the handling, but the original fog issue was by someone having this issue before we made this change, so I am not confident this won't introduce a regression

@geemus
Copy link
Member

geemus commented May 7, 2015

I've merged the change, could people who have experienced issues (or are now) try with master and see if it improves their situations? Thanks!

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.

5 participants