fix: perform gateway error conversion for graph backend

blockstore.go

@@ -3,14 +3,11 @@ package main
 import (
 	"context"
 	"errors"
-	"fmt"
-	"strings"
 	"time"
 
-	"github.com/filecoin-saturn/caboose"
+	"github.com/ipfs/bifrost-gateway/lib"
 	blockstore "github.com/ipfs/boxo/blockstore"
 	exchange "github.com/ipfs/boxo/exchange"
-	"github.com/ipfs/boxo/gateway"
 	blocks "github.com/ipfs/go-block-format"
 	"github.com/ipfs/go-cid"
 	"go.uber.org/zap/zapcore"
@@ -38,7 +35,7 @@ func (e *exchangeBsWrapper) GetBlock(ctx context.Context, c cid.Cid) (blocks.Blo
 
 	blk, err := e.bstore.Get(ctx, c)
 	if err != nil {
-		return nil, gatewayError(err)
+		return nil, lib.GatewayError(err)
 	}
 	return blk, nil
 }
@@ -67,33 +64,4 @@ func (e *exchangeBsWrapper) Close() error {
 	return nil
 }
 
-// gatewayError translates underlying blockstore error into one that gateway code will return as HTTP 502 or 504
-// it also makes sure Retry-After hint from remote blockstore will be passed to HTTP client, if present.
-func gatewayError(err error) error {
-	if errors.Is(err, &gateway.ErrorStatusCode{}) ||
-		errors.Is(err, &gateway.ErrorRetryAfter{}) {
-		// already correct error
-		return err
-	}
-
-	// All timeouts should produce 504 Gateway Timeout
-	if errors.Is(err, context.DeadlineExceeded) ||
-		errors.Is(err, caboose.ErrSaturnTimeout) ||
-		// Unfortunately this is not an exported type so we have to check for the content.
-		strings.Contains(err.Error(), "Client.Timeout exceeded") {
-		return fmt.Errorf("%w: %s", gateway.ErrGatewayTimeout, err.Error())
-	}
-
-	// (Saturn) errors that support the RetryAfter interface need to be converted
-	// to the correct gateway error, such that the HTTP header is set.
-	for v := err; v != nil; v = errors.Unwrap(v) {
-		if r, ok := v.(interface{ RetryAfter() time.Duration }); ok {
-			return gateway.NewErrorRetryAfter(err, r.RetryAfter())
-		}
-	}
-
-	// everything else returns 502 Bad Gateway
-	return fmt.Errorf("%w: %s", gateway.ErrBadGateway, err.Error())
-}
-
 var _ exchange.Interface = (*exchangeBsWrapper)(nil)

graph_gateway_test.go

@@ -1063,6 +1063,102 @@ func TestGetCAR(t *testing.T) {
 	}
 }
 
+func TestPassthroughErrors(t *testing.T) {
+	t.Run("PathTraversalError", func(t *testing.T) {
+		pathTraversalTest := func(t *testing.T, traversal func(ctx context.Context, p gateway.ImmutablePath, backend *lib.GraphGateway) error) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			var requestNum int
+			s := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+				requestNum++
+				switch requestNum {
+				case 1:
+					// Expect the full request, but return one that terminates in the middle of the path
+					expectedUri := "/ipfs/bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi/hamtDir/exampleA"
+					if request.URL.Path != expectedUri {
+						panic(fmt.Errorf("expected URI %s, got %s", expectedUri, request.RequestURI))
+					}
+
+					if err := sendBlocks(ctx, dirWithMultiblockHAMTandFiles, writer, []string{
+						"bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi", // root dir
+					}); err != nil {
+						panic(err)
+					}
+				case 2:
+					// Expect the full request, but return one that terminates in the middle of the file
+					// Note: this is an implementation detail, it could be in the future that we request less data (e.g. partial path)
+					expectedUri := "/ipfs/bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi/hamtDir/exampleA"
+					if request.URL.Path != expectedUri {
+						panic(fmt.Errorf("expected URI %s, got %s", expectedUri, request.RequestURI))
+					}
+
+					if err := sendBlocks(ctx, dirWithMultiblockHAMTandFiles, writer, []string{
+						"bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi", // root dir
+						"bafybeignui4g7l6cvqyy4t6vnbl2fjtego4ejmpcia77jhhwnksmm4bejm", // hamt root
+					}); err != nil {
+						panic(err)
+					}
+				default:
+					t.Fatal("unsupported request number")
+				}
+			}))
+			defer s.Close()
+
+			bs := newProxyBlockStore([]string{s.URL}, newCachedDNS(dnsCacheRefreshInterval))
+			p, err := gateway.NewImmutablePath(path.New("/ipfs/bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi/hamtDir/exampleA"))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			bogusErr := gateway.NewErrorStatusCode(fmt.Errorf("this is a test error"), 418)
+
+			clientRequestNum := 0
+			backend, err := lib.NewGraphGatewayBackend(&retryFetcher{
+				inner: &fetcherWrapper{fn: func(ctx context.Context, path string, cb lib.DataCallback) error {
+					clientRequestNum++
+					if clientRequestNum > 2 {
+						return bogusErr
+					}
+					return bs.(lib.CarFetcher).Fetch(ctx, path, cb)
+				}},
+				allowedRetries: 3, retriesRemaining: 3})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = traversal(ctx, p, backend)
+			parsedErr := &gateway.ErrorStatusCode{}
+			if errors.As(err, &parsedErr) {
+				if parsedErr.StatusCode == bogusErr.StatusCode {
+					return
+				}
+			}
+			t.Fatal("error did not pass through")
+		}
+		t.Run("Block", func(t *testing.T) {
+			pathTraversalTest(t, func(ctx context.Context, p gateway.ImmutablePath, backend *lib.GraphGateway) error {
+				_, _, err := backend.GetBlock(ctx, p)
+				return err
+			})
+		})
+		t.Run("File", func(t *testing.T) {
+			pathTraversalTest(t, func(ctx context.Context, p gateway.ImmutablePath, backend *lib.GraphGateway) error {
+				_, _, err := backend.Get(ctx, p)
+				return err
+			})
+		})
+	})
+}
+
+type fetcherWrapper struct {
+	fn func(ctx context.Context, path string, cb lib.DataCallback) error
+}
+
+func (w *fetcherWrapper) Fetch(ctx context.Context, path string, cb lib.DataCallback) error {
+	return w.fn(ctx, path, cb)
+}
+
 type retryFetcher struct {
 	inner            lib.CarFetcher
 	allowedRetries   int

lib/gateway_traversal.go

@@ -89,7 +89,7 @@ func carToLinearBlockGetter(ctx context.Context, reader io.Reader, metrics *Grap
 			if !ok || errors.Is(blkRead.err, io.EOF) {
 				return nil, io.ErrUnexpectedEOF
 			}
-			return nil, blkRead.err
+			return nil, GatewayError(blkRead.err)
 		}
 		if blkRead.block != nil {
 			metrics.carBlocksFetchedMetric.Inc()

lib/gateway_utils.go

@@ -1,10 +1,15 @@
 package lib
 
 import (
+	"context"
+	"errors"
+	"fmt"
 	"net/url"
 	"strconv"
 	"strings"
+	"time"
 
+	"github.com/filecoin-saturn/caboose"
 	"github.com/ipfs/boxo/gateway"
 )
 
@@ -37,3 +42,32 @@ func carParamsToString(params gateway.CarParams) string {
 	}
 	return paramsBuilder.String()
 }
+
+// GatewayError translates underlying blockstore error into one that gateway code will return as HTTP 502 or 504
+// it also makes sure Retry-After hint from remote blockstore will be passed to HTTP client, if present.
+func GatewayError(err error) error {
+	if errors.Is(err, &gateway.ErrorStatusCode{}) ||
+		errors.Is(err, &gateway.ErrorRetryAfter{}) {
+		// already correct error
+		return err
+	}
+
+	// All timeouts should produce 504 Gateway Timeout
+	if errors.Is(err, context.DeadlineExceeded) ||
+		errors.Is(err, caboose.ErrSaturnTimeout) ||
+		// Unfortunately this is not an exported type so we have to check for the content.
+		strings.Contains(err.Error(), "Client.Timeout exceeded") {
+		return fmt.Errorf("%w: %s", gateway.ErrGatewayTimeout, err.Error())
+	}
+
+	// (Saturn) errors that support the RetryAfter interface need to be converted
+	// to the correct gateway error, such that the HTTP header is set.
+	for v := err; v != nil; v = errors.Unwrap(v) {
+		if r, ok := v.(interface{ RetryAfter() time.Duration }); ok {
+			return gateway.NewErrorRetryAfter(err, r.RetryAfter())
+		}
+	}
+
+	// everything else returns 502 Bad Gateway
+	return fmt.Errorf("%w: %s", gateway.ErrBadGateway, err.Error())
+}

lib/gateway_utils_test.go

@@ -1,7 +1,11 @@
 package lib
 
 import (
+	"errors"
+	"fmt"
+	"github.com/stretchr/testify/assert"
 	"testing"
+	"time"
 
 	ifacepath "github.com/ipfs/boxo/coreiface/path"
 	"github.com/ipfs/boxo/gateway"
@@ -55,3 +59,36 @@ func TestContentPathToCarUrl(t *testing.T) {
 		})
 	}
 }
+
+type testErr struct {
+	message    string
+	retryAfter time.Duration
+}
+
+func (e *testErr) Error() string {
+	return e.message
+}
+
+func (e *testErr) RetryAfter() time.Duration {
+	return e.retryAfter
+}
+
+func TestGatewayErrorRetryAfter(t *testing.T) {
+	originalErr := &testErr{message: "test", retryAfter: time.Minute}
+	var (
+		convertedErr error
+		gatewayErr   *gateway.ErrorRetryAfter
+	)
+
+	// Test unwrapped
+	convertedErr = GatewayError(originalErr)
+	ok := errors.As(convertedErr, &gatewayErr)
+	assert.True(t, ok)
+	assert.EqualValues(t, originalErr.retryAfter, gatewayErr.RetryAfter)
+
+	// Test wrapped.
+	convertedErr = GatewayError(fmt.Errorf("wrapped error: %w", originalErr))
+	ok = errors.As(convertedErr, &gatewayErr)
+	assert.True(t, ok)
+	assert.EqualValues(t, originalErr.retryAfter, gatewayErr.RetryAfter)
+}