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

Make resumable uploads work from browser with signed url #1022

Merged
merged 3 commits into from Dec 28, 2022

Conversation

jasonford
Copy link
Contributor

These headers are necessary to make browser originated signed url requests work.

I don't know go, and these edits likely should be handled at a different level/more comprehensively. Happy to modify and repush if you've got feedback. If not, this PR could be a good sketch for better updates to support browser based signed url requests.

@jasonford jasonford changed the title Make client resumable uploads work form client with signed url Make resumable uploads work from browser with signed url Dec 27, 2022
@@ -810,6 +810,7 @@ func (s *Server) downloadObject(w http.ResponseWriter, r *http.Request) {
w.Header().Set("X-Goog-Generation", strconv.FormatInt(obj.Generation, 10))
w.Header().Set("X-Goog-Hash", fmt.Sprintf("crc32c=%s,md5=%s", obj.Crc32c, obj.Md5Hash))
w.Header().Set("Last-Modified", obj.Updated.Format(http.TimeFormat))
w.Header().Set("Access-Control-Allow-Origin", "*")
Copy link
Owner

Choose a reason for hiding this comment

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

Looking at the docs for GCS API, it seems that this should be configured in the bucket and honored by fake-gcs-server? https://cloud.google.com/storage/docs/configuring-cors#rest-apis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointer (and all your excellent work with this project)!

On quick look it seems the necessary work is:

  1. add a handler for PATCH to /storage/v1/b/BUCKET_NAME?fields=cors that stores the configuration somewhere (do you have a pointer to existing code that shows the preferred way to store this type of config?)
  2. load that config at this step and set the header.

Seems pretty straightforward, but being new to go and not having too much time to dive deeply, I'd appreciate a pointer to code with the preferred config set/get approach.

Copy link
Owner

Choose a reason for hiding this comment

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

Ugh, thinking a bit more about this, this one is tricky for the same versioning is tricky with the

The filesystem backend currently doesn't support storing any bucket attributes, so that would be a requirement. I don't want to make this PR too large for you.

I opened #1025, we can merge this PR as is to get you unblocked.

I've merged some other changes though, so please rebase your PR so we can get it ready to ship.

@jasonford jasonford force-pushed the make-client-resumable-uploads-work branch from ebd37b3 to 1a563df Compare December 28, 2022 23:15
@fsouza fsouza merged commit e6f1413 into fsouza:main Dec 28, 2022
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