Merge pull request #80 from ipfs/fix/gateway-workaround

fix(dagreader): remove a buggy workaround for a gateway issue

Author: Stebalien

io/dagreader.go

@@ -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
@@ -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 {
@@ -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)
@@ -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
@@ -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
 		}
@@ -484,6 +480,7 @@ func (dr *dagReader) Seek(offset int64, whence int) (int64, error) {
 // in the `SeekStart` case.
 func (dr *dagReader) resetPosition() {
 	dr.currentNodeData = nil
+	dr.offset = 0
 
 	dr.dagWalker = ipld.NewWalker(dr.ctx, ipld.NewNavigableIPLDNode(dr.rootNode, dr.serv))
 	// TODO: This could be avoided (along with storing the `dr.rootNode` and

io/dagreader_test.go

@@ -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())