diff --git a/CHANGELOG.md b/CHANGELOG.md index aae74ac26..f13c44044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The following emojis are used to highlight certain changes: ### Changed +- 🛠 `files`: `File` interface no longer embeds `io.Seeker`. Implementations that support seeking (e.g. `ReaderFile` wrapping a seekable reader, `Symlink`, UnixFS files) still have a `Seek` method, but callers must type-assert to `io.Seeker` instead of assuming all files are seekable. This fixes interface-sniffing bugs where non-seekable readers (HTTP multipart streams, `WebFile`) falsely advertised seek support and caused runtime errors in downstream consumers like `go-car`. [#1128](https://github.com/ipfs/boxo/pull/1128) - `chunker`: `FromString` now rejects malformed `size-` strings with extra parameters (e.g. `size-123-extra` was previously silently accepted). - upgrade to `go-libp2p` [v0.48.0](https://github.com/libp2p/go-libp2p/releases/tag/v0.48.0) diff --git a/files/doc.go b/files/doc.go index 69c7dc550..6d6f2a9ba 100644 --- a/files/doc.go +++ b/files/doc.go @@ -6,7 +6,7 @@ // The package defines a hierarchy of interfaces: // // - [Node]: Base interface for all file-like objects (mode, modification time, size) -// - [File]: A regular file with [io.Reader] and [io.Seeker] +// - [File]: A regular file with [io.Reader] (some implementations also support [io.Seeker]) // - [Directory]: Contains entries traversable via [DirIterator] // - [FileInfo]: Extends [Node] with local filesystem information // diff --git a/files/file.go b/files/file.go index e2ece2862..f9feda660 100644 --- a/files/file.go +++ b/files/file.go @@ -32,12 +32,16 @@ type Node interface { Size() (int64, error) } -// Node represents a regular Unix file +// File represents a regular Unix file. +// +// Implementations that support random access may also implement [io.Seeker]. +// Callers that need seeking should type-assert rather than assume all Files +// are seekable, since some implementations (e.g. HTTP multipart streams) +// only support sequential reads. type File interface { Node io.Reader - io.Seeker } // DirEntry exposes information about a directory entry diff --git a/files/webfile.go b/files/webfile.go index 6cc9d5782..b64672f08 100644 --- a/files/webfile.go +++ b/files/webfile.go @@ -100,11 +100,6 @@ func (wf *WebFile) Close() error { return wf.body.Close() } -// TODO: implement -func (wf *WebFile) Seek(offset int64, whence int) (int64, error) { - return 0, ErrNotSupported -} - func (wf *WebFile) Size() (int64, error) { if err := wf.start(); err != nil { return 0, err diff --git a/gateway/backend_blocks.go b/gateway/backend_blocks.go index 45b11c327..59590978f 100644 --- a/gateway/backend_blocks.go +++ b/gateway/backend_blocks.go @@ -175,7 +175,11 @@ func (bb *BlocksBackend) Get(ctx context.Context, path path.ImmutablePath, range } if rootCodec == uint64(mc.Raw) { - if err := seekToRangeStart(f, ra, fileSize); err != nil { + s, ok := f.(io.Seeker) + if !ok { + return ContentPathMetadata{}, nil, fmt.Errorf("file does not support seeking: %w", ErrInternalServerError) + } + if err := seekToRangeStart(s, ra, fileSize); err != nil { return ContentPathMetadata{}, nil, err } } @@ -216,8 +220,12 @@ func (bb *BlocksBackend) Get(ctx context.Context, path path.ImmutablePath, range return ContentPathMetadata{}, nil, err } - if err := seekToRangeStart(file, ra, fileSize); err != nil { - return ContentPathMetadata{}, nil, err + if seeker, ok := file.(io.Seeker); ok { + if err := seekToRangeStart(seeker, ra, fileSize); err != nil { + return ContentPathMetadata{}, nil, err + } + } else if ra != nil && ra.From != 0 { + return ContentPathMetadata{}, nil, fmt.Errorf("file does not support seeking: %w", ErrInternalServerError) } if s, ok := f.(*files.Symlink); ok { @@ -279,7 +287,12 @@ func loadUnixFSFileWithLazyBlocks(ctx context.Context, path path.ImmutablePath, } // Seek to the start of the requested range - if err := seekToRangeStart(file, ra, fileSize); err != nil { + seeker, ok := file.(io.Seeker) + if !ok { + log.Debugw("file does not support seeking, skipping range optimization", "path", path) + return nil + } + if err := seekToRangeStart(seeker, ra, fileSize); err != nil { log.Debugw("failed to seek to range start", "path", path, "error", err) diff --git a/gateway/backend_car.go b/gateway/backend_car.go index aaa20258a..f89616c64 100644 --- a/gateway/backend_car.go +++ b/gateway/backend_car.go @@ -470,7 +470,11 @@ func loadTerminalEntity(ctx context.Context, c cid.Cid, blk blocks.Block, lsys * return nil, fmt.Errorf("invalid car backend range: negative start bigger than the file size") } } - if _, err := f.Seek(from, io.SeekStart); err != nil { + s, ok := f.(io.Seeker) + if !ok { + return nil, fmt.Errorf("file does not support seeking") + } + if _, err := s.Seek(from, io.SeekStart); err != nil { return nil, err } } diff --git a/gateway/backend_car_files.go b/gateway/backend_car_files.go index 8bae0ebcf..14522aa67 100644 --- a/gateway/backend_car_files.go +++ b/gateway/backend_car_files.go @@ -102,9 +102,9 @@ func (b *backpressuredFile) Read(p []byte) (n int, err error) { continue } - f, ok := nd.(files.File) + f, ok := nd.(io.ReadSeeker) if !ok { - return 0, fmt.Errorf("not a file, should be unreachable") + return 0, fmt.Errorf("not a seekable file, should be unreachable") } b.f = f diff --git a/gateway/backend_car_test.go b/gateway/backend_car_test.go index 029b96c71..29d8a7368 100644 --- a/gateway/backend_car_test.go +++ b/gateway/backend_car_test.go @@ -604,7 +604,8 @@ func TestCarBackendGetFileRangeRequest(t *testing.T) { uio, err := unixfile.NewUnixfsFile(ctx, dsrv, fileRootNd) require.NoError(t, err) f := uio.(files.File) - _, err = f.Seek(int64(startIndex), io.SeekStart) + fSeeker := f.(io.Seeker) + _, err = fSeeker.Seek(int64(startIndex), io.SeekStart) require.NoError(t, err) expectedFileData, err := io.ReadAll(io.LimitReader(f, int64(endIndex)-int64(startIndex)+1)) require.NoError(t, err) diff --git a/gateway/handler_block.go b/gateway/handler_block.go index 27507272e..d5825d84e 100644 --- a/gateway/handler_block.go +++ b/gateway/handler_block.go @@ -2,6 +2,8 @@ package gateway import ( "context" + "fmt" + "io" "net/http" "time" @@ -44,7 +46,12 @@ func (i *handler) serveRawBlock(ctx context.Context, w http.ResponseWriter, r *h return false } - if !i.seekToStartOfFirstRange(w, r, data, sz) { + s, ok := data.(io.Seeker) + if !ok { + i.webError(w, r, fmt.Errorf("block data does not support seeking"), http.StatusInternalServerError) + return false + } + if !i.seekToStartOfFirstRange(w, r, s, sz) { return false } diff --git a/gateway/handler_codec.go b/gateway/handler_codec.go index 414c89900..6569bb6d2 100644 --- a/gateway/handler_codec.go +++ b/gateway/handler_codec.go @@ -78,7 +78,12 @@ func (i *handler) serveCodec(ctx context.Context, w http.ResponseWriter, r *http return false } - return i.renderCodec(ctx, w, r, rq, blockSize, data) + rsc, ok := data.(io.ReadSeekCloser) + if !ok { + i.webError(w, r, fmt.Errorf("block data does not support seeking"), http.StatusInternalServerError) + return false + } + return i.renderCodec(ctx, w, r, rq, blockSize, rsc) } func (i *handler) renderCodec(ctx context.Context, w http.ResponseWriter, r *http.Request, rq *requestData, blockSize int64, blockData io.ReadSeekCloser) bool {