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

Commit 47e4181

Browse files
committed
fix(dagreader): remove a buggy workaround for a gateway issue
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.
1 parent 0faf573 commit 47e4181

File tree

2 files changed

+73
-12
lines changed

2 files changed

+73
-12
lines changed

io/dagreader.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ var (
1616
ErrIsDir = errors.New("this dag node is a directory")
1717
ErrCantReadSymlinks = errors.New("cannot currently read symlinks")
1818
ErrUnkownNodeType = errors.New("unknown node type")
19+
ErrSeekNotSupported = errors.New("file does not support seeking")
1920
)
2021

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

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

363+
// Shortcut seeking to the beginning, we're already there.
364+
if offset == 0 {
365+
return 0, nil
366+
}
367+
362368
// Use the internal reader's context to fetch the child node promises
363369
// (see `ipld.NavigableIPLDNode.FetchChild` for details).
364370
dr.dagWalker.SetContext(dr.ctx)
@@ -388,7 +394,7 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
388394
// If there aren't enough size hints don't seek
389395
// (see the `io.EOF` handling error comment below).
390396
if fsNode.NumChildren() != len(node.Links()) {
391-
return io.EOF
397+
return ErrSeekNotSupported
392398
}
393399

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

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

io/dagreader_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,71 @@ func TestSeekAndRead(t *testing.T) {
7272
}
7373
}
7474

75+
func TestSeekWithoutBlocksizes(t *testing.T) {
76+
dserv := testu.GetDAGServ()
77+
ctx, closer := context.WithCancel(context.Background())
78+
defer closer()
79+
80+
inbuf := make([]byte, 1024)
81+
82+
for i := 0; i < 256; i++ {
83+
inbuf[i*4] = byte(i)
84+
}
85+
86+
inbuf[1023] = 1 // force the reader to be 1024 bytes
87+
node := testu.GetNode(t, dserv, inbuf, testu.UseProtoBufLeaves)
88+
89+
// remove the blocksizes
90+
pbnode := node.Copy().(*mdag.ProtoNode)
91+
fsnode, err := unixfs.FSNodeFromBytes(pbnode.Data())
92+
if err != nil {
93+
t.Fatal(err)
94+
}
95+
fsnode.RemoveAllBlockSizes()
96+
newData, err := fsnode.GetBytes()
97+
if err != nil {
98+
t.Fatal(err)
99+
}
100+
pbnode.SetData(newData)
101+
err = dserv.Add(ctx, pbnode)
102+
if err != nil {
103+
t.Fatal(err)
104+
}
105+
node = pbnode
106+
107+
reader, err := NewDagReader(ctx, node, dserv)
108+
if err != nil {
109+
t.Fatal(err)
110+
}
111+
112+
_, err = reader.Seek(-4, io.SeekEnd)
113+
if err == nil {
114+
t.Fatal("seeking shouldn't work without blocksizes")
115+
}
116+
117+
_, err = reader.Seek(4, io.SeekStart)
118+
if err == nil {
119+
t.Fatal("seeking shouldn't work without blocksizes")
120+
}
121+
122+
_, err = reader.Seek(4, io.SeekCurrent)
123+
if err == nil {
124+
t.Fatal("seeking shouldn't work without blocksizes")
125+
}
126+
127+
// Seeking to the current position or the end should still work.
128+
129+
_, err = reader.Seek(0, io.SeekCurrent)
130+
if err != nil {
131+
t.Fatal(err)
132+
}
133+
134+
_, err = reader.Seek(0, io.SeekStart)
135+
if err != nil {
136+
t.Fatal(err)
137+
}
138+
}
139+
75140
func TestRelativeSeek(t *testing.T) {
76141
dserv := testu.GetDAGServ()
77142
ctx, closer := context.WithCancel(context.Background())

0 commit comments

Comments
 (0)