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

update to use AWS Signature Version 4 #174

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mattwarrenrnp
Copy link

this is a rewrite of the form helper to implement the current AWS Signature version 4 method of signing the policy.

@lenart
Copy link

lenart commented Jan 14, 2015

I'd love to see this merged into s3_direct_upload gem. New buckets only work with v4 policy.

@hakunin
Copy link

hakunin commented Mar 19, 2015

Seems like this would only work with us-east-1? What about other regions?

@lenart
Copy link

lenart commented Mar 19, 2015

@hakunin what do you mean?

You can configure your region in the configurator.

@hakunin
Copy link

hakunin commented Mar 19, 2015

I am not an expert at s3, but this part seems hardcoded just for one region, shouldn't it match all the new ones or something?

https://github.com/waynehoover/s3_direct_upload/pull/174/files#diff-98fcf18bb8f0556479337cf71e97dc1fR35

@lenart
Copy link

lenart commented Mar 19, 2015

It's how Amazon decided to name their servers. Have a look at their site (scroll down to Amazon Simple Storage Service) and see how the region name and endpoints are related.

The code you pointed out just follows this (weird) rule.

@hakunin
Copy link

hakunin commented Mar 19, 2015

Thanks for clarifying that @lenart, I just checked out Matt's fork and can verify that this works. Would be nice if this was merged after so many months, or if a fork could be made to carry on the awesome direct_s3 gem's functionality.

@mcfiredrill
Copy link
Collaborator

I think alot of people are using this in forks. I'd like to see a solution for this problem merged.

This shouldn't break any existing apps right?

@@ -16,7 +16,7 @@ def initialize(options)
aws_access_key_id: S3DirectUpload.config.access_key_id,
aws_secret_access_key: S3DirectUpload.config.secret_access_key,
bucket: options[:bucket] || S3DirectUpload.config.bucket,
region: S3DirectUpload.config.region || "s3",
region: S3DirectUpload.config.region || "us-east-1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should us-east-1 be the default?

@Preen
Copy link

Preen commented Apr 6, 2016

I can't see that this has been implemented yet?

apricote pushed a commit to narando/s3_file_field that referenced this pull request Aug 24, 2016
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.

None yet

5 participants