Skip to content

Commit

Permalink
chore: upgrade Go to 1.22, adapt to http server URL changes (#494)
Browse files Browse the repository at this point in the history
Strange behaviour causing e2e test failures when upgrading to 1.22, diagnosed
as a different form that an HTTP server decodes a request and doesn't fully
unescape its `Path` when it has special characters in it. This seems to be
related to the new ServeMux changes in 1.22 because GODEBUG=httpmuxgo121=1
reverts the behaviour and the test passes.
The fix is to explicitly unescape the path before passing it on,
go-trustless-utils will re-escape itself when it builds a new request to
pass on downstream, but in the meantime we need to know the exact unescaped
path so we can traverse it to validate the incoming data as well as pass it on
upstream to the remote.
  • Loading branch information
rvagg authored Dec 17, 2024
1 parent 6ccf6ac commit 53f405f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 14 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/filecoin-project/lassie

go 1.21
go 1.22

require (
github.com/benbjohnson/clock v1.3.5
Expand Down
19 changes: 13 additions & 6 deletions pkg/internal/itest/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -54,11 +55,14 @@ func TestTrustlessGatewayE2E(t *testing.T) {
req.NoError(err)

// install the indexer to announce to
t.Log("Installing indexer")
indexer := filepath.Join(tr.Dir, "storetheindex")
tr.Run("go", "install", "github.com/ipni/storetheindex@latest")
tr.Run("go", "install", "github.com/ipni/storetheindex@v0.8.14")
// install the ipni cli to inspect the indexer
t.Log("Installing ipni")
ipni := filepath.Join(tr.Dir, "ipni")
tr.Run("go", "install", "github.com/ipni/ipni-cli/cmd/ipni@latest")
tr.Run("go", "install", "github.com/ipni/ipni-cli/cmd/[email protected]")
t.Log("Installing frisbii")
// install frisbii to serve the content
frisbii := filepath.Join(tr.Dir, "frisbii")
tr.Run("go", "install", "github.com/ipld/frisbii/cmd/frisbii@latest")
Expand All @@ -67,7 +71,10 @@ func TestTrustlessGatewayE2E(t *testing.T) {
req.NoError(err)

// initialise and start the indexer and adjust the config
t.Log("Initialising indexer")
tr.Run(indexer, "init", "--store", "pebble", "--pubsub-topic", "/indexer/ingest/mainnet", "--no-bootstrap")

t.Log("Starting indexer")
indexerReady := test.NewStdoutWatcher(test.IndexerReadyMatch)
cmdIndexer := tr.Start(test.NewExecution(indexer, "daemon").WithWatcher(indexerReady))
select {
Expand All @@ -82,6 +89,7 @@ func TestTrustlessGatewayE2E(t *testing.T) {
carPath := trustlesspathing.Unixfs20mVarietyCARPath()

// start frisbii with the fixture CAR
t.Logf("Starting frisbii with CAR [%s] and root [%s]", carPath, root)
frisbiiReady := test.NewStdoutWatcher("Announce() complete")
cmdFrisbii := tr.Start(test.NewExecution(frisbii,
"--listen", "localhost:37471",
Expand All @@ -101,17 +109,13 @@ func TestTrustlessGatewayE2E(t *testing.T) {
req.Eventually(func() bool {
mh := root.Hash().B58String()
findOutput := tr.Run(ipni, "find", "--no-priv", "-i", "http://localhost:3000", "-mh", mh)
t.Logf("import output:\n%s\n", findOutput)

if bytes.Contains(findOutput, []byte("not found")) {
return false
}
if !bytes.Contains(findOutput, []byte("Provider:")) {
t.Logf("mh %s: unexpected error: %s", mh, findOutput)
return false
}

t.Logf("mh %s: found", mh)
return true
}, 10*time.Second, time.Second)

Expand Down Expand Up @@ -172,6 +176,7 @@ func TestTrustlessGatewayE2E(t *testing.T) {
if testCase.Path != "" {
args[len(args)-1] = args[len(args)-1] + "/" + testCase.Path
}
t.Logf("Running lassie %s", strings.Join(args, " "))
tr.Run(lassie, args...)

_, err = os.Stat(expectedCarPath)
Expand All @@ -188,6 +193,8 @@ func TestTrustlessGatewayE2E(t *testing.T) {
reqUrl, err := url.Parse("http://localhost:30000/" + testCase.AsQuery())
req.NoError(err)

t.Logf("Fetching %s", reqUrl.String())

// download and read all body from URL along with Accept:application/vnd.ipld.car header
reqReq, err := http.NewRequestWithContext(ctx, http.MethodGet, reqUrl.String(), nil)
req.NoError(err)
Expand Down
20 changes: 13 additions & 7 deletions pkg/server/http/ipfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"net/http"
"net/url"
"strconv"
"strings"

Expand All @@ -24,13 +25,18 @@ import (

func IpfsHandler(fetcher types.Fetcher, cfg HttpServerConfig) func(http.ResponseWriter, *http.Request) {
return func(res http.ResponseWriter, req *http.Request) {
statusLogger := newStatusLogger(req.Method, req.URL.Path)
unescapedPath, err := url.PathUnescape(req.URL.Path)
if err != nil {
logger.Warnf("error unescaping path: %s", err)
unescapedPath = req.URL.Path
}
statusLogger := newStatusLogger(req.Method, unescapedPath)

if !checkGet(req, res, statusLogger) {
return
}

ok, request := decodeRetrievalRequest(cfg, res, req, statusLogger)
ok, request := decodeRetrievalRequest(cfg, res, req, unescapedPath, statusLogger)
if !ok {
return
}
Expand Down Expand Up @@ -87,7 +93,7 @@ func IpfsHandler(fetcher types.Fetcher, cfg HttpServerConfig) func(http.Response
res.Header().Set("Content-Type", trustlesshttp.DefaultContentType().WithDuplicates(request.Duplicates).String())
res.Header().Set("Etag", request.Etag())
res.Header().Set("X-Content-Type-Options", "nosniff")
res.Header().Set("X-Ipfs-Path", trustlessutils.PathEscape(req.URL.Path))
res.Header().Set("X-Ipfs-Path", trustlessutils.PathEscape(unescapedPath))
res.Header().Set("X-Trace-Id", requestId)
statusLogger.logStatus(200, "OK")
close(bytesWritten)
Expand Down Expand Up @@ -152,8 +158,8 @@ func checkGet(req *http.Request, res http.ResponseWriter, statusLogger *statusLo
return false
}

func decodeRequest(res http.ResponseWriter, req *http.Request, statusLogger *statusLogger) (bool, trustlessutils.Request) {
rootCid, path, err := trustlesshttp.ParseUrlPath(req.URL.Path)
func decodeRequest(res http.ResponseWriter, req *http.Request, unescapedPath string, statusLogger *statusLogger) (bool, trustlessutils.Request) {
rootCid, path, err := trustlesshttp.ParseUrlPath(unescapedPath)
if err != nil {
if errors.Is(err, trustlesshttp.ErrPathNotFound) {
errorResponse(res, statusLogger, http.StatusNotFound, err)
Expand Down Expand Up @@ -205,8 +211,8 @@ func decodeRequest(res http.ResponseWriter, req *http.Request, statusLogger *sta
}
}

func decodeRetrievalRequest(cfg HttpServerConfig, res http.ResponseWriter, req *http.Request, statusLogger *statusLogger) (bool, types.RetrievalRequest) {
ok, request := decodeRequest(res, req, statusLogger)
func decodeRetrievalRequest(cfg HttpServerConfig, res http.ResponseWriter, req *http.Request, unescapedPath string, statusLogger *statusLogger) (bool, types.RetrievalRequest) {
ok, request := decodeRequest(res, req, unescapedPath, statusLogger)
if !ok {
return false, types.RetrievalRequest{}
}
Expand Down

0 comments on commit 53f405f

Please sign in to comment.