Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Commit

Permalink
fix(dagreader): remove a buggy workaround for a gateway issue
Browse files Browse the repository at this point in the history
We had a hack that "pretended" seeking worked when it didn't as go's HTTP
library uses seeks to determine the file size. However,

1. I'm changing the gateway to actually rely on seeking working as specified.
2. We don't even need this hack. The gateway performs two types of seeks (unless
   a range query is passed):
    1. It seeks to the beginning. We can always shortcut this.
    2. It seeks to the end. The gateway now has a special "lazy" seeker to avoid
      seeking until we actually try to _read_. Therefore, we don't need a hack
      for that either.
  • Loading branch information
Stebalien committed Jan 4, 2020
1 parent 0faf573 commit 47e4181
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 12 deletions.
20 changes: 8 additions & 12 deletions io/dagreader.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var (
ErrIsDir = errors.New("this dag node is a directory")
ErrCantReadSymlinks = errors.New("cannot currently read symlinks")
ErrUnkownNodeType = errors.New("unknown node type")
ErrSeekNotSupported = errors.New("file does not support seeking")
)

// TODO: Rename the `DagReader` interface, this doesn't read *any* DAG, just
Expand Down Expand Up @@ -345,7 +346,7 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
switch whence {
case io.SeekStart:
if offset < 0 {
return -1, errors.New("invalid offset")
return dr.offset, errors.New("invalid offset")
}

if offset == dr.offset {
Expand All @@ -359,6 +360,11 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
// Seek from the beginning of the DAG.
dr.resetPosition()

// Shortcut seeking to the beginning, we're already there.
if offset == 0 {
return 0, nil
}

// Use the internal reader's context to fetch the child node promises
// (see `ipld.NavigableIPLDNode.FetchChild` for details).
dr.dagWalker.SetContext(dr.ctx)
Expand Down Expand Up @@ -388,7 +394,7 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
// If there aren't enough size hints don't seek
// (see the `io.EOF` handling error comment below).
if fsNode.NumChildren() != len(node.Links()) {
return io.EOF
return ErrSeekNotSupported
}

// Internal nodes have no data, so just iterate through the
Expand Down Expand Up @@ -445,16 +451,6 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
}
})

if err == io.EOF {
// TODO: Taken from https://github.com/ipfs/go-ipfs/pull/4320,
// check if still valid.
// Return negative number if we can't figure out the file size. Using io.EOF
// for this seems to be good(-enough) solution as it's only returned by
// precalcNextBuf when we step out of file range.
// This is needed for gateway to function properly
return -1, nil
}

if err != nil {
return 0, err
}
Expand Down
65 changes: 65 additions & 0 deletions io/dagreader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,71 @@ func TestSeekAndRead(t *testing.T) {
}
}

func TestSeekWithoutBlocksizes(t *testing.T) {
dserv := testu.GetDAGServ()
ctx, closer := context.WithCancel(context.Background())
defer closer()

inbuf := make([]byte, 1024)

for i := 0; i < 256; i++ {
inbuf[i*4] = byte(i)
}

inbuf[1023] = 1 // force the reader to be 1024 bytes
node := testu.GetNode(t, dserv, inbuf, testu.UseProtoBufLeaves)

// remove the blocksizes
pbnode := node.Copy().(*mdag.ProtoNode)
fsnode, err := unixfs.FSNodeFromBytes(pbnode.Data())
if err != nil {
t.Fatal(err)
}
fsnode.RemoveAllBlockSizes()
newData, err := fsnode.GetBytes()
if err != nil {
t.Fatal(err)
}
pbnode.SetData(newData)
err = dserv.Add(ctx, pbnode)
if err != nil {
t.Fatal(err)
}
node = pbnode

reader, err := NewDagReader(ctx, node, dserv)
if err != nil {
t.Fatal(err)
}

_, err = reader.Seek(-4, io.SeekEnd)
if err == nil {
t.Fatal("seeking shouldn't work without blocksizes")
}

_, err = reader.Seek(4, io.SeekStart)
if err == nil {
t.Fatal("seeking shouldn't work without blocksizes")
}

_, err = reader.Seek(4, io.SeekCurrent)
if err == nil {
t.Fatal("seeking shouldn't work without blocksizes")
}

// Seeking to the current position or the end should still work.

_, err = reader.Seek(0, io.SeekCurrent)
if err != nil {
t.Fatal(err)
}

_, err = reader.Seek(0, io.SeekStart)
if err != nil {
t.Fatal(err)
}
}

func TestRelativeSeek(t *testing.T) {
dserv := testu.GetDAGServ()
ctx, closer := context.WithCancel(context.Background())
Expand Down

0 comments on commit 47e4181

Please sign in to comment.