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

Parse URL When Ensuring CORS for Direct Uploads #46

Merged

Conversation

tubbo
Copy link
Contributor

@tubbo tubbo commented Sep 10, 2019

The request.url returns the full URL, with path included. This isn't
valid for a CORS header, which needs just the scheme, host, and port if
it's non-standard. Update the DirectUpload.ensure_cors! method to
parse out those pieces of the URL and re-assemble it for the CORS
header and ID.

No changelog

Fixes #19

The `request.url` returns the full URL, with path included. This isn't
valid for a CORS header, which needs just the scheme, host, and port if
it's non-standard. Update the `DirectUpload.ensure_cors!` method to
parse out those pieces of the URL and re-assemble it for the CORS
header and ID.

No changelog

Fixes #19
@bencrouse
Copy link
Contributor

Why does the port being "non-standard" matter?

@tubbo
Copy link
Contributor Author

tubbo commented Sep 11, 2019

Why does the port being "non-standard" matter?

Because in a browser, :80 and :443 are assumed when you use http:// and https://, respectively. If I didn't filter out the port, the URL would be something like http://store.test:80 and that would not create the right CORS configuration for the bucket.

@bencrouse bencrouse merged commit 4c3c292 into v3.4-stable Sep 12, 2019
@bencrouse bencrouse deleted the parse-url-when-ensuring-cors-for-direct-uploads branch September 12, 2019 13:31
mttdffy added a commit that referenced this pull request Sep 12, 2019
mttdffy added a commit that referenced this pull request Sep 13, 2019
@tubbo tubbo added this to the v3.4.16 milestone Sep 16, 2019
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

2 participants