fix(subdomain-gw): curl on localhost (Option A: HTTP301+payload)#6982
Merged
Stebalien merged 1 commit intofeat/gateway-subdomainsfrom Mar 13, 2020
Merged
fix(subdomain-gw): curl on localhost (Option A: HTTP301+payload)#6982Stebalien merged 1 commit intofeat/gateway-subdomainsfrom
Stebalien merged 1 commit intofeat/gateway-subdomainsfrom
Conversation
Stebalien
reviewed
Mar 11, 2020
This was referenced Mar 11, 2020
Closed
lidel
added a commit
that referenced
this pull request
Mar 12, 2020
This changes the way we return CID payload in body of HTTP 301 response. We no longer get superfluous response.WriteHeader call, as it is executed only once, in http.ServeContent() Gist: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: #6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
lidel
commented
Mar 12, 2020
Comment on lines
+53
to
+58
| func (sw *statusResponseWriter) WriteHeader(code int) { | ||
| // Check if we need to adjust Status Code to account for scheduled redirect | ||
| // This enables us to return HTTP 301 | ||
| // for subdomain redirect in web browsers while also returning body for cli | ||
| // tools which does not follow redirects by default (curl, wget). | ||
| redirect := sw.w.Header().Get("Location") | ||
| if redirect != "" { | ||
| code = http.StatusMovedPermanently | ||
| } | ||
| sw.w.WriteHeader(code) | ||
| } |
Member
Author
There was a problem hiding this comment.
I came up with this surgical wrapper because status code is hardcoded to 200 inside http.ServeContent().
Is this idiomatic/acceptable in go? Not sure if we can make this change more elegant while keeping is small without duplicating http.ServeContent.
Member
There was a problem hiding this comment.
Wrapping is normal. Swapping the code like this is weird but fine.
Stebalien
approved these changes
Mar 12, 2020
Comment on lines
+53
to
+58
| func (sw *statusResponseWriter) WriteHeader(code int) { | ||
| // Check if we need to adjust Status Code to account for scheduled redirect | ||
| // This enables us to return HTTP 301 | ||
| // for subdomain redirect in web browsers while also returning body for cli | ||
| // tools which does not follow redirects by default (curl, wget). | ||
| redirect := sw.w.Header().Get("Location") | ||
| if redirect != "" { | ||
| code = http.StatusMovedPermanently | ||
| } | ||
| sw.w.WriteHeader(code) | ||
| } |
Member
There was a problem hiding this comment.
Wrapping is normal. Swapping the code like this is weird but fine.
When request is sent to http://localhost:8080/ipfs/$cid response has HTTP 301 status code and "Location" header with redirect destination at $cid.ipfs.localhost:8080 Redirect is followed by browsersi, but not by commandline tools. Status 301 is ignored by curl in default mode: it will print response and won't follow redirect, user needs to add -L for that. To fix curl, we return correct payload in body of HTTP 301 response, but set Clear-Site-Data header to ensure Origin sandbox can't be abused. This requires a surgical workaround: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: #6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
e705f02 to
9fe44b7
Compare
Stebalien
approved these changes
Mar 13, 2020
2 tasks
Stebalien
pushed a commit
that referenced
this pull request
Mar 18, 2020
When request is sent to http://localhost:8080/ipfs/$cid response has HTTP 301 status code and "Location" header with redirect destination at $cid.ipfs.localhost:8080 Redirect is followed by browsersi, but not by commandline tools. Status 301 is ignored by curl in default mode: it will print response and won't follow redirect, user needs to add -L for that. To fix curl, we return correct payload in body of HTTP 301 response, but set Clear-Site-Data header to ensure Origin sandbox can't be abused. This requires a surgical workaround: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: #6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
Stebalien
pushed a commit
that referenced
this pull request
Mar 18, 2020
When request is sent to http://localhost:8080/ipfs/$cid response has HTTP 301 status code and "Location" header with redirect destination at $cid.ipfs.localhost:8080 Redirect is followed by browsersi, but not by commandline tools. Status 301 is ignored by curl in default mode: it will print response and won't follow redirect, user needs to add -L for that. To fix curl, we return correct payload in body of HTTP 301 response, but set Clear-Site-Data header to ensure Origin sandbox can't be abused. This requires a surgical workaround: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: #6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
dgrisham
pushed a commit
to dgrisham/go-ipfs
that referenced
this pull request
Apr 3, 2020
When request is sent to http://localhost:8080/ipfs/$cid response has HTTP 301 status code and "Location" header with redirect destination at $cid.ipfs.localhost:8080 Redirect is followed by browsersi, but not by commandline tools. Status 301 is ignored by curl in default mode: it will print response and won't follow redirect, user needs to add -L for that. To fix curl, we return correct payload in body of HTTP 301 response, but set Clear-Site-Data header to ensure Origin sandbox can't be abused. This requires a surgical workaround: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: ipfs#6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
hacdias
pushed a commit
to ipfs/boxo
that referenced
this pull request
Jan 27, 2023
When request is sent to http://localhost:8080/ipfs/$cid response has HTTP 301 status code and "Location" header with redirect destination at $cid.ipfs.localhost:8080 Redirect is followed by browsersi, but not by commandline tools. Status 301 is ignored by curl in default mode: it will print response and won't follow redirect, user needs to add -L for that. To fix curl, we return correct payload in body of HTTP 301 response, but set Clear-Site-Data header to ensure Origin sandbox can't be abused. This requires a surgical workaround: If Location header is present in ResponseWriter's Header map, we ensure http.ServeContent() returns HTTP 301 Context: ipfs/kubo#6982 License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org> This commit was moved from ipfs/kubo@f9567a0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Return payload with HTTP 301 response:
Locationheader, so there should not be any bandwidth wastedClear-Site-Dataheader with such responseDescription
When request is sent to
http://localhost:8080/ipfs/$cidHTTP 301 status code with "Location" header redirect client to subdomain destination at$cid.ipfs.localhost:8080Redirects are not followed by
curlin default mode, but will be respected by user agents that follow redirects by default, namely web browsers.To fix curl, we return correct payload in body of HTTP 301 response, but set
Clear-Site-Dataheader to ensure Origin sandbox can't be abused.Caveat
This comes with side effect of prometeus client making redundantWriteHeadercall, which produces warning in logs:I attempted to fix prometeus and avoid redundant call in https://github.com/prometheus/client_golang/pull/724 but am not sure if that approach will be approved upstream.Update:
superfluous response.WriteHeader callfixed in e705f02