Merge branch 'main' into missing-passthrough

.github/workflows/gateway-conformance.yml

@@ -22,7 +22,7 @@ jobs:
     steps:
       # 1. Download the gateway-conformance fixtures
       - name: Download gateway-conformance fixtures
-        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7
         with:
           output: fixtures
           merged: true
@@ -47,7 +47,7 @@ jobs:
 
       # 4. Run the gateway-conformance tests
       - name: Run gateway-conformance tests
-        uses: ipfs/gateway-conformance/.github/actions/test@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/test@v0.7
         with:
           gateway-url: http://127.0.0.1:8040
           subdomain-url: http://example.net:8040
@@ -84,7 +84,7 @@ jobs:
     steps:
       # 1. Download the gateway-conformance fixtures
       - name: Download gateway-conformance fixtures
-        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7
         with:
           output: fixtures
           merged: true
@@ -114,7 +114,7 @@ jobs:
 
       # 4. Run the gateway-conformance tests
       - name: Run gateway-conformance tests
-        uses: ipfs/gateway-conformance/.github/actions/test@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/test@v0.7
         with:
           gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote block gateway
           subdomain-url: http://example.net:8040
@@ -152,7 +152,7 @@ jobs:
     steps:
       # 1. Download the gateway-conformance fixtures
       - name: Download gateway-conformance fixtures
-        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.7
         with:
           output: fixtures
           merged: true
@@ -182,7 +182,7 @@ jobs:
 
       # 4. Run the gateway-conformance tests
       - name: Run gateway-conformance tests
-        uses: ipfs/gateway-conformance/.github/actions/test@v0.6
+        uses: ipfs/gateway-conformance/.github/actions/test@v0.7
         with:
           gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote car gateway
           subdomain-url: http://example.net:8040

CHANGELOG.md

@@ -22,6 +22,8 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+- `gateway` Fix redirect URLs for subdirectories with characters that need escaping. [#779](https://github.com/ipfs/boxo/pull/779)
+
 ### Security
 
 

bitswap/README.md

@@ -23,7 +23,7 @@ contain wantlists or blocks.
 
 A node sends a wantlist to tell peers which blocks it wants. When a node receives
 a wantlist it should check which blocks it has from the wantlist, and consider
-sending the matching blocks to the requestor.
+sending the matching blocks to the requester.
 
 When a node receives blocks that it asked for, the node should send out a
 notification called a 'Cancel' to tell its peers that the node no longer
@@ -74,7 +74,7 @@ block, err := exchange.GetBlock(ctx, c)
 
 Parameter Notes:
 
-1. `ctx` is the context for this request, which can be cancelled to cancel the request
+1. `ctx` is the context for this request, which can be canceled to cancel the request
 2. `c` is the content ID of the block you're requesting
 
 ### Get Several Blocks Asynchronously
@@ -89,7 +89,7 @@ blockChannel, err := exchange.GetBlocks(ctx, cids)
 
 Parameter Notes:
 
-1. `ctx` is the context for this request, which can be cancelled to cancel the request
+1. `ctx` is the context for this request, which can be canceled to cancel the request
 2. `cids` is a slice of content IDs for the blocks you're requesting
 
 ### Get Related Blocks Faster With Sessions

bitswap/client/internal/messagequeue/messagequeue.go

@@ -93,7 +93,7 @@ type MessageQueue struct {
 	cancels   *cid.Set
 	priority  int32
 
-	// Dont touch any of these variables outside of run loop
+	// Don't touch any of these variables outside of run loop
 	sender         bsnet.MessageSender
 	rebroadcastNow chan struct{}
 	// For performance reasons we just clear out the fields of the message
@@ -160,7 +160,7 @@ func (r *recallWantlist) MarkSent(e bswl.Entry) bool {
 
 // SentAt records the time at which a want was sent
 func (r *recallWantlist) SentAt(c cid.Cid, at time.Time) {
-	// The want may have been cancelled in the interim
+	// The want may have been canceled in the interim
 	if _, ok := r.sent.Contains(c); ok {
 		if _, ok := r.sentAt[c]; !ok {
 			r.sentAt[c] = at
@@ -222,7 +222,7 @@ type DontHaveTimeoutManager interface {
 	// Shutdown the manager (Shutdown is final, manager cannot be restarted)
 	Shutdown()
 	// AddPending adds the wants as pending a response. If the are not
-	// cancelled before the timeout, the OnDontHaveTimeout method will be called.
+	// canceled before the timeout, the OnDontHaveTimeout method will be called.
 	AddPending([]cid.Cid)
 	// CancelPending removes the wants
 	CancelPending([]cid.Cid)
@@ -597,7 +597,7 @@ func (mq *MessageQueue) sendMessage() {
 	}
 }
 
-// If want-block times out, simulate a DONT_HAVE reponse.
+// If want-block times out, simulate a DONT_HAVE response.
 // This is necessary when making requests to peers running an older version of
 // Bitswap that doesn't support the DONT_HAVE response, and is also useful to
 // mitigate getting blocked by a peer that takes a long time to respond.
@@ -847,13 +847,13 @@ FINISH:
 		defer mq.wllock.Unlock()
 
 		for _, e := range peerEntries[:sentPeerEntries] {
-			if e.Cid.Defined() { // Check if want was cancelled in the interim
+			if e.Cid.Defined() { // Check if want was canceled in the interim
 				mq.peerWants.SentAt(e.Cid, now)
 			}
 		}
 
 		for _, e := range bcstEntries[:sentBcstEntries] {
-			if e.Cid.Defined() { // Check if want was cancelled in the interim
+			if e.Cid.Defined() { // Check if want was canceled in the interim
 				mq.bcstWants.SentAt(e.Cid, now)
 			}
 		}

bitswap/server/internal/decision/engine.go

@@ -916,7 +916,7 @@ func (e *Engine) handleOverflow(ctx context.Context, p peer.ID, overflow, wants
 	return wants
 }
 
-// Split the want-havek entries from the cancel and deny entries.
+// Split the want, cancel, and deny entries.
 func (e *Engine) splitWantsCancelsDenials(p peer.ID, m bsmsg.BitSwapMessage) ([]bsmsg.Entry, []bsmsg.Entry, []bsmsg.Entry, error) {
 	entries := m.Wantlist() // creates copy; safe to modify
 	if len(entries) == 0 {

gateway/gateway_test.go

@@ -700,13 +700,13 @@ func TestRedirects(t *testing.T) {
 			// - Browsers will send original URI in URL-escaped form
 			// - We expect query parameters to be persisted
 			// - We drop fragments, as those should not be sent by a browser
-			{"/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%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/" + 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=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/" + cid},
 			{"/ipns/?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 			{"/ipns/?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},

gateway/handler.go

@@ -906,9 +906,10 @@ func handleProtocolHandlerRedirect(w http.ResponseWriter, r *http.Request, c *Co
 			webError(w, r, c, fmt.Errorf("uri query parameter scheme must be ipfs or ipns: %w", err), http.StatusBadRequest)
 			return true
 		}
-		path := u.Path
+
+		path := u.EscapedPath()
 		if u.RawQuery != "" { // preserve query if present
-			path = path + "?" + u.RawQuery
+			path += "?" + url.PathEscape(u.RawQuery)
 		}
 
 		redirectURL := gopath.Join("/", u.Scheme, u.Host, path)
@@ -989,7 +990,7 @@ func (i *handler) handleSuperfluousNamespace(w http.ResponseWriter, r *http.Requ
 	}
 
 	// Attempt to fix the superflous namespace
-	intendedPath, err := path.NewPath(strings.TrimPrefix(r.URL.Path, "/ipfs"))
+	intendedPath, err := path.NewPath(strings.TrimPrefix(r.URL.EscapedPath(), "/ipfs"))
 	if err != nil {
 		i.webError(w, r, fmt.Errorf("invalid ipfs path: %w", err), http.StatusBadRequest)
 		return true

gateway/handler_codec.go

@@ -170,10 +170,15 @@ func (i *handler) serveCodecHTML(ctx context.Context, w http.ResponseWriter, r *
 		suffix := "/"
 		// preserve query parameters
 		if r.URL.RawQuery != "" {
-			suffix = suffix + "?" + r.URL.RawQuery
+			suffix = suffix + "?" + url.PathEscape(r.URL.RawQuery)
+		}
+		// Re-escape path instead of reusing RawPath to avod mix of lawer
+		// and upper hex that may come from RawPath.
+		if strings.ContainsRune(requestURI.RawPath, '%') {
+			requestURI.RawPath = ""
 		}
 		// /ipfs/cid/foo?bar must be redirected to /ipfs/cid/foo/?bar
-		redirectURL := requestURI.Path + suffix
+		redirectURL := requestURI.EscapedPath() + suffix
 		http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
 		return true
 	}

gateway/handler_unixfs_dir.go

@@ -47,11 +47,17 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *
 			suffix := "/"
 			// preserve query parameters
 			if r.URL.RawQuery != "" {
-				suffix = suffix + "?" + r.URL.RawQuery
+				suffix = suffix + "?" + url.PathEscape(r.URL.RawQuery)
+			}
+			// Re-escape path instead of reusing RawPath to avod mix of lawer
+			// and upper hex that may come from RawPath.
+			if strings.ContainsRune(requestURI.RawPath, '%') {
+				requestURI.RawPath = ""
 			}
 			// /ipfs/cid/foo?bar must be redirected to /ipfs/cid/foo/?bar
-			redirectURL := originalURLPath + suffix
+			redirectURL := requestURI.EscapedPath() + suffix
 			rq.logger.Debugw("directory location moved permanently", "status", http.StatusMovedPermanently)
+
 			http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
 			return true
 		}

namesys/interface.go

@@ -207,7 +207,7 @@ func PublishWithEOL(eol time.Time) PublishOption {
 	}
 }
 
-// PublishWithEOL sets [PublishOptions.TTL].
+// PublishWithTTL sets [PublishOptions.TTL].
 func PublishWithTTL(ttl time.Duration) PublishOption {
 	return func(o *PublishOptions) {
 		o.TTL = ttl

pinning/pinner/dspinner/pin_test.go

@@ -367,7 +367,7 @@ func TestAddLoadPin(t *testing.T) {
 		t.Fatal(err)
 	}
 	if pinData.Mode != mode {
-		t.Error("worng pin mode")
+		t.Error("wrong pin mode")
 	}
 	if pinData.Cid != ak {
 		t.Error("wrong pin cid")
@@ -428,17 +428,17 @@ func TestPinAddOverwriteName(t *testing.T) {
 }
 
 func TestIsPinnedLookup(t *testing.T) {
-	// Test that lookups work in pins which share
-	// the same branches.  For that construct this tree:
+	// Test that lookups work in pins which share the same branches. For that
+	// construct this tree:
 	//
 	// A5->A4->A3->A2->A1->A0
 	//         /           /
 	// B-------           /
 	//  \                /
 	//   C---------------
 	//
-	// This ensures that IsPinned works for all objects both when they
-	// are pinned and once they have been unpinned.
+	// This ensures that IsPinned works for all objects both when they are
+	// pinned and once they have been unpinned.
 	aBranchLen := 6
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -449,8 +449,8 @@ func TestIsPinnedLookup(t *testing.T) {
 
 	dserv := mdag.NewDAGService(bserv)
 
-	// Create new pinner.  New will not load anything since there are
-	// no pins saved in the datastore yet.
+	// Create new pinner. New will not load anything since there are no pins
+	// saved in the datastore yet.
 	p, err := New(ctx, dstore, dserv)
 	if err != nil {
 		t.Fatal(err)
@@ -563,7 +563,7 @@ func TestPinRecursiveFail(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// NOTE: This isnt a time based test, we expect the pin to fail
+	// NOTE: This isn't a time based test, we expect the pin to fail
 	mctx, cancel := context.WithTimeout(ctx, time.Millisecond)
 	defer cancel()
 
@@ -582,7 +582,7 @@ func TestPinRecursiveFail(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// this one is time based... but shouldnt cause any issues
+	// this one is time based... but shouldn't cause any issues
 	mctx, cancel = context.WithTimeout(ctx, time.Second)
 	defer cancel()
 	err = p.Pin(mctx, a, true, "")
@@ -732,7 +732,7 @@ func TestLoadDirty(t *testing.T) {
 		t.Fatal("index should be deleted")
 	}
 
-	// Create new pinner on same datastore that was never flushed.  This should
+	// Create new pinner on same datastore that was never flushed. This should
 	// detect the dirty flag and repair the indexes.
 	p, err = New(ctx, dstore, dserv)
 	if err != nil {
@@ -950,7 +950,7 @@ func makeStore() (ds.Datastore, ipld.DAGService) {
 	return dstore, dserv
 }
 
-// BenchmarkLoadRebuild loads a pinner that has some number of saved pins, and
+// BenchmarkLoad loads a pinner that has some number of saved pins, and
 // compares the load time when rebuilding indexes to loading without rebuilding
 // indexes.
 func BenchmarkLoad(b *testing.B) {
@@ -995,8 +995,8 @@ func BenchmarkLoad(b *testing.B) {
 	})
 }
 
-// BenchmarkNthPins shows the time it takes to create/save 1 pin when a number
-// of other pins already exist.  Each run in the series shows performance for
+// BenchmarkNthPin shows the time it takes to create/save 1 pin when a number
+// of other pins already exist. Each run in the series shows performance for
 // creating a pin in a larger number of existing pins.
 func BenchmarkNthPin(b *testing.B) {
 	dstore, dserv := makeStore()
@@ -1044,8 +1044,8 @@ func benchmarkNthPin(b *testing.B, count int, pinner ipfspin.Pinner, dserv ipld.
 	}
 }
 
-// BenchmarkNPins demonstrates creating individual pins.  Each run in the
-// series shows performance for a larger number of individual pins.
+// BenchmarkNPins demonstrates creating individual pins. Each run in the series
+// shows performance for a larger number of individual pins.
 func BenchmarkNPins(b *testing.B) {
 	for count := 128; count < 16386; count <<= 1 {
 		b.Run(fmt.Sprint("PinDS-", count), func(b *testing.B) {
@@ -1270,7 +1270,7 @@ func TestCidIndex(t *testing.T) {
 	defer results.Close()
 
 	// Iterate all pins and check if the corresponding recursive or direct
-	// index is missing.  If the index is missing then create the index.
+	// index is missing. If the index is missing then create the index.
 	seen = false
 	for r := range results.Next() {
 		if seen {

routing/http/filters/filters.go

@@ -49,7 +49,7 @@ func AddFiltersToURL(baseURL string, protocolFilter, addrFilter []string) string
 	return parsedURL.String()
 }
 
-// applyFiltersToIter applies the filters to the given iterator and returns a new iterator.
+// ApplyFiltersToIter applies the filters to the given iterator and returns a new iterator.
 //
 // The function iterates over the input iterator, applying the specified filters to each record.
 // It supports both positive and negative filters for both addresses and protocols.

routing/http/internal/goroutines.go

@@ -7,8 +7,10 @@ import (
 	"github.com/samber/lo"
 )
 
-// DoBatch processes a slice of items with concurrency no higher than maxConcurrency by splitting it into batches no larger than maxBatchSize.
-// If an error is returned for any batch, the process is short-circuited and the error is immediately returned.
+// DoBatch processes a slice of items with concurrency no higher than
+// maxConcurrency by splitting it into batches no larger than maxBatchSize. If
+// an error is returned for any batch, the process is short-circuited and the
+// error is immediately returned.
 func DoBatch[A any](ctx context.Context, maxBatchSize, maxConcurrency int, items []A, f func(context.Context, []A) error) error {
 	if len(items) == 0 {
 		return nil
@@ -62,8 +64,8 @@ func DoBatch[A any](ctx context.Context, maxBatchSize, maxConcurrency int, items
 			// we finished without any errors, congratulations
 			return nil
 		}
-		// short circuit on the first error we get
-		// canceling the worker ctx and thus all workers,
+		// short circuit on the first error we get canceling the worker ctx and
+		// thus all workers,
 		return err
 	case <-ctx.Done():
 		return ctx.Err()