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

Strip 'Expect' header when presigning URLs #444

Merged
merged 1 commit into from
Jan 25, 2018

Conversation

mossprescott
Copy link
Contributor

Fixes #363.

This is more or less the simplest thing that can be done to make
pre-signed PUT urls work, and it exposes the header transformation
at the lowest level, so that it's possible to build some other
behavior on top of that. However, this is neither as targeted nor
as flexible as the solutions proposed in the issue.

This is the approach from the workaround I posted on that issue, but simpler because I didn't thread the transformation of headers all the way up through the layers. I'm submitting this for feedback; happy to rework this branch to make this more robust, flexible, or whatever.

I've tested it by running the following:

main = do
  now <- getCurrentTime
  let expireSeconds = 300
  env <- newEnv Discover
  urlBytes <- runResourceT $ runAWS env $ presignURL now expireSeconds $
    putObject testBucket testKey ""
  putStrLn $ unpack $ decodeUtf8 urlBytes
curl -X PUT "https://s3.amazonaws.com/..." -d "foo"

Fixes brendanhay#363.

This is more or less the _simplest_ thing that can be done to make
pre-signed PUT urls work, and it exposes the header transformation
at the lowest level, so that it's possible to build some other
behavior on top of that. However, this is neither as targeted nor
as flexible as the solutions proposed in the bug.
@MaxGabriel
Copy link
Contributor

I reached out to @mossprescott after I saw his Github comment, since we need client S3 uploads at work. I can confirm that both his code from #363 and this PR fixes the issue 👍

@mossprescott
Copy link
Contributor Author

The GHC 7.10 build is broken here, but also on master/develop, so I'm not sure what if anything I can do about that.

@brendanhay
Copy link
Owner

Thanks for taking the time to make a PR, it looks like a good balance of backwards compatibility, flexibility, and simplicity! :shipit:

@brendanhay brendanhay merged commit 4d7b186 into brendanhay:develop Jan 25, 2018
@MaxGabriel
Copy link
Contributor

@brendanhay Any chance you could do a release with this fix? I understand if not—I imagine it's a huge pain to do releases when this many packages are released in lock step, and I can keep using Github to pull in Amazonka.

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.

3 participants