fix(gateway)!: no duplicate payload during subdomain redirects

gateway/errors.go

@@ -123,7 +123,7 @@ func (e *ErrorResponse) Unwrap() error {
 	return e.Err
 }
 
-func (i *handler) webError(w http.ResponseWriter, r *http.Request, err error, defaultCode int) {
+func webError(w http.ResponseWriter, r *http.Request, c *Config, err error, defaultCode int) {
 	code := defaultCode
 
 	// Pass Retry-After hint to the client
@@ -161,7 +161,7 @@ func (i *handler) webError(w http.ResponseWriter, r *http.Request, err error, de
 		w.WriteHeader(code)
 		_ = assets.ErrorTemplate.Execute(w, assets.ErrorTemplateData{
 			GlobalData: assets.GlobalData{
-				Menu: i.config.Menu,
+				Menu: c.Menu,
 			},
 			StatusCode: code,
 			StatusText: http.StatusText(code),

gateway/errors_test.go

@@ -40,15 +40,13 @@ func TestWebError(t *testing.T) {
 	t.Parallel()
 
 	// Create a handler to be able to test `webError`.
-	api, _ := newMockAPI(t)
-	config := Config{Headers: map[string][]string{}}
-	handler := NewHandler(config, api).(*handler)
+	config := &Config{Headers: map[string][]string{}}
 
 	t.Run("429 Too Many Requests", func(t *testing.T) {
 		err := fmt.Errorf("wrapped for testing: %w", NewErrorRetryAfter(ErrTooManyRequests, 0))
 		w := httptest.NewRecorder()
 		r := httptest.NewRequest(http.MethodGet, "/blah", nil)
-		handler.webError(w, r, err, http.StatusInternalServerError)
+		webError(w, r, config, err, http.StatusInternalServerError)
 		assert.Equal(t, http.StatusTooManyRequests, w.Result().StatusCode)
 		assert.Zero(t, len(w.Result().Header.Values("Retry-After")))
 	})
@@ -57,7 +55,7 @@ func TestWebError(t *testing.T) {
 		err := NewErrorRetryAfter(ErrTooManyRequests, 25*time.Second)
 		w := httptest.NewRecorder()
 		r := httptest.NewRequest(http.MethodGet, "/blah", nil)
-		handler.webError(w, r, err, http.StatusInternalServerError)
+		webError(w, r, config, err, http.StatusInternalServerError)
 		assert.Equal(t, http.StatusTooManyRequests, w.Result().StatusCode)
 		assert.Equal(t, "25", w.Result().Header.Get("Retry-After"))
 	})
@@ -66,7 +64,7 @@ func TestWebError(t *testing.T) {
 		err := NewErrorRetryAfter(ErrServiceUnavailable, 50*time.Second)
 		w := httptest.NewRecorder()
 		r := httptest.NewRequest(http.MethodGet, "/blah", nil)
-		handler.webError(w, r, err, http.StatusInternalServerError)
+		webError(w, r, config, err, http.StatusInternalServerError)
 		assert.Equal(t, http.StatusServiceUnavailable, w.Result().StatusCode)
 		assert.Equal(t, "50", w.Result().Header.Get("Retry-After"))
 	})

gateway/gateway_test.go

@@ -310,11 +310,11 @@ func TestUriQueryRedirect(t *testing.T) {
 		{"/ipfs/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
 		{"/ipfs/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
 		{"/ipfs/?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},
-		{"/ipfs?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/?uri=ipfs://" + cid},
+		{"/ipfs?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},
 		{"/ipfs/?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 		{"/ipns/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
 		{"/ipns/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
-		{"/ipns?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/?uri=ipns://" + cid},
+		{"/ipns?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 		{"/ipns/?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 		{"/ipns/?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},
 		{"/ipfs/?uri=unsupported://" + cid, http.StatusBadRequest, ""},

gateway/handler.go

@@ -88,26 +88,6 @@ func NewHandler(c Config, api IPFSBackend) http.Handler {
 	return newHandlerWithMetrics(c, api)
 }
 
-// StatusResponseWriter enables us to override HTTP Status Code passed to
-// WriteHeader function inside of http.ServeContent.  Decision is based on
-// presence of HTTP Headers such as Location.
-type statusResponseWriter struct {
-	http.ResponseWriter
-}
-
-func (sw *statusResponseWriter) WriteHeader(code int) {
-	// Check if we need to adjust Status Code to account for scheduled redirect
-	// This enables us to return payload along with HTTP 301
-	// for subdomain redirect in web browsers while also returning body for cli
-	// tools which do not follow redirects by default (curl, wget).
-	redirect := sw.ResponseWriter.Header().Get("Location")
-	if redirect != "" && code == http.StatusOK {
-		code = http.StatusMovedPermanently
-		log.Debugw("subdomain redirect", "location", redirect, "status", code)
-	}
-	sw.ResponseWriter.WriteHeader(code)
-}
-
 // ServeContent replies to the request using the content in the provided ReadSeeker
 // and returns the status code written and any error encountered during a write.
 // It wraps http.ServeContent which takes care of If-None-Match+Etag,
@@ -201,7 +181,11 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if requestHandled := i.handleProtocolHandlerRedirect(w, r, logger); requestHandled {
+	if redirectURL, err := getProtocolHandlerRedirect(r); err != nil {
+		i.webError(w, r, err, http.StatusBadRequest)
+		return
+	} else if redirectURL != "" {
+		http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
 		return
 	}
 
@@ -779,29 +763,25 @@ func handleUnsupportedHeaders(r *http.Request) (err *ErrorResponse) {
 // via navigator.registerProtocolHandler Web API
 // https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler
 // TLDR: redirect /ipfs/?uri=ipfs%3A%2F%2Fcid%3Fquery%3Dval to /ipfs/cid?query=val
-func (i *handler) handleProtocolHandlerRedirect(w http.ResponseWriter, r *http.Request, logger *zap.SugaredLogger) (requestHandled bool) {
+func getProtocolHandlerRedirect(r *http.Request) (string, error) {
 	if uriParam := r.URL.Query().Get("uri"); uriParam != "" {
 		u, err := url.Parse(uriParam)
 		if err != nil {
-			i.webError(w, r, fmt.Errorf("failed to parse uri query parameter: %w", err), http.StatusBadRequest)
-			return true
+			return "", fmt.Errorf("failed to parse uri query parameter: %w", err)
 		}
 		if u.Scheme != "ipfs" && u.Scheme != "ipns" {
-			i.webError(w, r, fmt.Errorf("uri query parameter scheme must be ipfs or ipns: %w", err), http.StatusBadRequest)
-			return true
+			return "", fmt.Errorf("uri query parameter scheme must be ipfs or ipns: %w", err)
 		}
 		path := u.Path
 		if u.RawQuery != "" { // preserve query if present
 			path = path + "?" + u.RawQuery
 		}
 
 		redirectURL := gopath.Join("/", u.Scheme, u.Host, path)
-		logger.Debugw("uri param, redirect", "to", redirectURL, "status", http.StatusMovedPermanently)
-		http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
-		return true
+		return redirectURL, nil
 	}
 
-	return false
+	return "", nil
 }
 
 // Disallow Service Worker registration on namespace roots
@@ -828,13 +808,6 @@ func handleIpnsB58mhToCidRedirection(w http.ResponseWriter, r *http.Request) boo
 		return false
 	}
 
-	if w.Header().Get("Location") != "" {
-		// Ignore this if there is already a redirection in place. This happens
-		// if there is a subdomain redirection. In that case, the path is already
-		// converted to CIDv1.
-		return false
-	}
-
 	pathParts := strings.Split(r.URL.Path, "/")
 	if len(pathParts) < 3 {
 		return false
@@ -934,3 +907,7 @@ func (i *handler) getTemplateGlobalData(r *http.Request, contentPath ipath.Path)
 		DNSLink:    dnsLink,
 	}
 }
+
+func (i *handler) webError(w http.ResponseWriter, r *http.Request, err error, defaultCode int) {
+	webError(w, r, &i.config, err, defaultCode)
+}

gateway/handler_codec.go

@@ -154,12 +154,6 @@ func (i *handler) renderCodec(ctx context.Context, w http.ResponseWriter, r *htt
 }
 
 func (i *handler) serveCodecHTML(ctx context.Context, w http.ResponseWriter, r *http.Request, blockCid cid.Cid, blockData io.ReadSeekCloser, resolvedPath ipath.Resolved, contentPath ipath.Path) bool {
-	// If a redirect is setup (e.g. subdomains), do it and do not render the HTML.
-	if w.Header().Get("Location") != "" {
-		w.WriteHeader(http.StatusMovedPermanently)
-		return true
-	}
-
 	// WithHostname may have constructed an IPFS (or IPNS) path using the Host header.
 	// In this case, we need the original path for constructing the redirect.
 	requestURI, err := url.ParseRequestURI(r.RequestURI)
@@ -240,9 +234,6 @@ func parseNode(blockCid cid.Cid, blockData io.ReadSeekCloser) *assets.ParsedNode
 
 // serveCodecRaw returns the raw block without any conversion
 func (i *handler) serveCodecRaw(ctx context.Context, w http.ResponseWriter, r *http.Request, blockData io.ReadSeekCloser, contentPath ipath.Path, name string, modtime, begin time.Time) bool {
-	// Special fix around redirects.
-	w = &statusResponseWriter{w}
-
 	// ServeContent will take care of
 	// If-None-Match+Etag, Content-Length and range requests
 	_, dataSent, _ := ServeContent(w, r, name, modtime, blockData)
@@ -257,9 +248,6 @@ func (i *handler) serveCodecRaw(ctx context.Context, w http.ResponseWriter, r *h
 
 // serveCodecConverted returns payload converted to codec specified in toCodec
 func (i *handler) serveCodecConverted(ctx context.Context, w http.ResponseWriter, r *http.Request, blockCid cid.Cid, blockData io.ReadSeekCloser, contentPath ipath.Path, toCodec mc.Code, modtime, begin time.Time) bool {
-	// Special fix around redirects.
-	w = &statusResponseWriter{w}
-
 	codec := blockCid.Prefix().Codec
 	decoder, err := multicodec.LookupDecoder(codec)
 	if err != nil {

gateway/handler_unixfs_dir.go

@@ -108,16 +108,6 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *
 		return false
 	}
 
-	// See statusResponseWriter.WriteHeader
-	// and https://github.com/ipfs/kubo/issues/7164
-	// Note: this needs to occur before listingTemplate.Execute otherwise we get
-	// superfluous response.WriteHeader call from prometheus/client_golang
-	if w.Header().Get("Location") != "" {
-		logger.Debugw("location moved permanently", "status", http.StatusMovedPermanently)
-		w.WriteHeader(http.StatusMovedPermanently)
-		return true
-	}
-
 	// A HTML directory index will be presented, be sure to set the correct
 	// type instead of relying on autodetection (which may fail).
 	w.Header().Set("Content-Type", "text/html")

gateway/handler_unixfs_file.go

@@ -91,9 +91,6 @@ func (i *handler) serveFile(ctx context.Context, w http.ResponseWriter, r *http.
 	// (unifies behavior across gateways and web browsers)
 	w.Header().Set("Content-Type", ctype)
 
-	// special fixup around redirects
-	w = &statusResponseWriter{w}
-
 	// ServeContent will take care of
 	// If-None-Match+Etag, Content-Length and range requests
 	_, dataSent, _ := ServeContent(w, r, name, modtime, content)

gateway/hostname.go

@@ -25,6 +25,15 @@ func WithHostname(c Config, api IPFSBackend, next http.Handler) http.HandlerFunc
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		defer panicHandler(w)
 
+		// First check for protocol handler redirects.
+		if redirectURL, err := getProtocolHandlerRedirect(r); err != nil {
+			webError(w, r, &c, err, http.StatusBadRequest)
+			return
+		} else if redirectURL != "" {
+			http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
+			return
+		}
+
 		// Unfortunately, many (well, ipfs.io) gateways use
 		// DNSLink so if we blindly rewrite with DNSLink, we'll
 		// break /ipfs links.
@@ -56,19 +65,12 @@ func WithHostname(c Config, api IPFSBackend, next http.Handler) http.HandlerFunc
 					useInlinedDNSLink := gw.InlineDNSLink
 					newURL, err := toSubdomainURL(host, r.URL.Path, r, useInlinedDNSLink, api)
 					if err != nil {
-						http.Error(w, err.Error(), http.StatusBadRequest)
+						webError(w, r, &c, err, http.StatusBadRequest)
 						return
 					}
 					if newURL != "" {
-						// Set "Location" header with redirect destination.
-						// It is ignored by curl in default mode, but will
-						// be respected by user agents that follow
-						// redirects by default, namely web browsers
-						w.Header().Set("Location", newURL)
-
-						// Note: we continue regular gateway processing:
-						// HTTP Status Code http.StatusMovedPermanently
-						// will be set later, in statusResponseWriter
+						http.Redirect(w, r, newURL, http.StatusMovedPermanently)
+						return
 					}
 				}
 
@@ -117,14 +119,14 @@ func WithHostname(c Config, api IPFSBackend, next http.Handler) http.HandlerFunc
 				// Do we need to redirect root CID to a canonical DNS representation?
 				dnsCID, err := toDNSLabel(rootID, rootCID)
 				if err != nil {
-					http.Error(w, err.Error(), http.StatusBadRequest)
+					webError(w, r, &c, err, http.StatusBadRequest)
 					return
 				}
 				if !strings.HasPrefix(r.Host, dnsCID) {
 					dnsPrefix := "/" + ns + "/" + dnsCID
 					newURL, err := toSubdomainURL(gwHostname, dnsPrefix+r.URL.Path, r, useInlinedDNSLink, api)
 					if err != nil {
-						http.Error(w, err.Error(), http.StatusBadRequest)
+						webError(w, r, &c, err, http.StatusBadRequest)
 						return
 					}
 					if newURL != "" {
@@ -140,7 +142,7 @@ func WithHostname(c Config, api IPFSBackend, next http.Handler) http.HandlerFunc
 					if rootCID.Type() != cid.Libp2pKey {
 						newURL, err := toSubdomainURL(gwHostname, pathPrefix+r.URL.Path, r, useInlinedDNSLink, api)
 						if err != nil {
-							http.Error(w, err.Error(), http.StatusBadRequest)
+							webError(w, r, &c, err, http.StatusBadRequest)
 							return
 						}
 						if newURL != "" {
@@ -438,9 +440,18 @@ func toSubdomainURL(hostname, path string, r *http.Request, inlineDNSLink bool,
 				// update path prefix to use real FQDN with DNSLink
 				rootID = dnsLabel
 			}
+		} else if ns == "ipfs" {
+			// If rootID is not a CID, but it's within the IPFS namespace, let it
+			// be handled by the regular handler.
+			return "", nil
 		}
 	}
 
+	if rootID == "" {
+		// If the rootID is empty, then we cannot produce a redirect URL.
+		return "", nil
+	}
+
 	return safeRedirectURL(fmt.Sprintf(
 		"%s//%s.%s.%s/%s%s",
 		scheme,