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

Add HEAD support for storage API object endpoint #759

Merged
merged 2 commits into from
May 10, 2022

Conversation

jossemarGT
Copy link
Contributor

I think I found a easy way to fix #667.

I also took the liberty to update the documentation, spelling out the -help flag usage for new users. Fun story, when I started using this emulator, I jumped into the code to figure out if there was a way to support my use case; I went from the endpoints to the handler implementation. Lastly, I found out all that the flags definitions were sitting in config.go file, also that I could simply have used said flag all the time 😅.

Both changes are in separted commits in case we want to drop or modify any of them.

Problem: As reporeted in fsouza#667. The storage API is replying 404 for
existing objects when method HEAD is used, although GET replies with
200.

Solution: Update storage API method route and handler to behave as
"donwload" when HEAD method is used.
Problem: As a new user, one tends to simply use what comes in the
README.md file without realizing there is plenty more features at hand.

Solution: Update README.md to pointing out there is a -help flag that
lists all the available configuration overrides. I was tempted to copy
and paste its output, but it might add too much noise for the reader
so disregarded that thought.
Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

The staticcheck failure is already fixed on the main branch.

Thanks for contributing!

@@ -481,7 +481,7 @@ func (s *Server) listObjects(r *http.Request) jsonResponse {
}

func (s *Server) getObject(w http.ResponseWriter, r *http.Request) {
if alt := r.URL.Query().Get("alt"); alt == "media" {
if alt := r.URL.Query().Get("alt"); alt == "media" || r.Method == http.MethodHead {
s.downloadObject(w, r)
Copy link
Owner

Choose a reason for hiding this comment

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

Oh I didn't realize this supported HEAD 🤔

We should probably rename this method, but doesn't need to be as part of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I had a similar thought, either renaming this function with another verb or splitting its functionality. However, I picked the route with less changes 😅

@fsouza fsouza merged commit 6c0ba10 into fsouza:main May 10, 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.

Request method HEAD returns 404 even if object exists
2 participants