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

Can't put contents to s3 bucket outside of US standard region with file object #82

Closed
jamesls opened this issue Jul 19, 2013 · 3 comments · Fixed by #143
Closed

Can't put contents to s3 bucket outside of US standard region with file object #82

jamesls opened this issue Jul 19, 2013 · 3 comments · Fixed by #143
Labels
bug This issue is a confirmed bug.

Comments

@jamesls
Copy link
Member

jamesls commented Jul 19, 2013

When you use a file like object as the body param to s3.PutObject for a bucket outside of US standard, you'll get a 307 followed by a 400 bad request and eventually a socket timeout. To repro:

  1. Create a bucket outside of us standard.
  2. Send a PutObject request with a file like object for the body: op.call(endpoint, body=open('/some/file', 'rb'), ...)

You'll see a 307 then a 400. I think this is because requests is configured to follow redirects, but there's no location header and hence the bad request.

I think the fact that this is a file like object exposes the bug because we send two packets, one for the request+headers then one for the body. When the body is a string (like what we use in the integration tests), then httplib will do a msg += message_body and send it as a single chunk which succeeds.

This also might be related to the virtual host handler we have. I noticed this in the log messages:

botocore.handlers: DEBUG: Checking for DNS compatible bucket for: https://s3-us-west-2.amazonaws.com/botocoretest1374195914-317
botocore.handlers: DEBUG: URI updated to: https://botocoretest1374195914-317.s3.amazonaws.com

Would be good to get an integration test written for this also.

@garnaat
Copy link
Member

garnaat commented Jul 19, 2013

I'm assuming that the bucket you were trying to write to was a newly created bucket?

In that case, you can receive a temporary redirect (307) when you try to access the bucket because the DNS records that have to be created to resolve the bucket using virtual-hosted addressing have not yet propagated throughout the network.

This 307 redirect should, however, contain a Location header. Are you sure there was no location header present?

@jamesls
Copy link
Member Author

jamesls commented Jul 19, 2013

Did more digging into this. You do get a location header with the region specific endpoint (bucket.s3region.amazonaws.com), but the problem is that the fileobj has been exhausted so .read() returns nothing. When you send the request to the new location it waits for content-length bytes, but eventually s3 closes the socket on their end.

Does requests provide anything to rewind a stream? Is this something we could do before requests retries the request?

@jamesls
Copy link
Member Author

jamesls commented Sep 30, 2013

I spent more time researching this and have a few observations:

  • The ideal way to handle this would be through an Expect: 100-continue header. From the research I've done it seems like httplib does not properly support this. It will handle a 100 continue response but it already sends the body. I also double checked that requests/urllib3 does not support 100 continue.
  • The requests library has a hook when a request is received, this includes redirect responses. We could add a hook that .seek(0)'s the body on redirects, otherwise it raises an error if it's not seekable. The only downside here is that we're potentially sending part of the body twice.

The reason I'd like to get this implemented is for better memory utilization in this branch of the CLI: https://github.com/jamesls/aws-cli/compare/memory-utilization

This branch is ready to go, except that by switching from bytearrays to file objects, we run into this issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a confirmed bug.
Projects
None yet
2 participants