From 10669e8b8c1d44ac7510fa17947956a408a85384 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 8 May 2015 16:07:01 -0700 Subject: [PATCH 1/2] path/resolver: Fix recursive path resolution I'm not entirely clear on Go's scoping (there's some text I can't quite parse here [1]), but it seems like the := version (because this is the first time we use 'err') was masking the function-level 'nd' just for this if block. That means that after we get out of the if block and return to the start of the for-loop for the next pass, nd.Links would still be pointing at the original object's links. This commit drops the :=, which fixes the earlier: $ ipfs ls QmXX7YRpU7nNBKfw75VG7Y1c3GwpSAGHRev67XVPgZFv9R/static/css Error: no link named "css" under QmXX7YRpU7nNBKfw75VG7Y1c3GwpSAGHRev67XVPgZFv9R so we get the intended: $ ipfs ls QmXX7YRpU7nNBKfw75VG7Y1c3GwpSAGHRev67XVPgZFv9R/static/css Qme4r3eA4h1revFBgCEv1HF1U7sLL4vvAyzRLWJhCFhwg2 7051 style.css It also means we're probably missing (or are unreliably using) a multi-level-path-resolving test. [1]: https://golang.org/ref/spec#Declarations_and_scope --- path/resolver.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/path/resolver.go b/path/resolver.go index b24f45f21..a08df8741 100644 --- a/path/resolver.go +++ b/path/resolver.go @@ -122,7 +122,8 @@ func (s *Resolver) ResolveLinks(ctx context.Context, ndd *merkledag.Node, names // fetch object for link and assign to nd ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() - nd, err := s.DAG.Get(ctx, next) + var err error + nd, err = s.DAG.Get(ctx, next) if err != nil { return append(result, nd), err } From 19823c6704c89f305613316e873460f9347ed19a Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 8 May 2015 21:43:43 -0700 Subject: [PATCH 2/2] path/resolver_test: Test recursive Link resolution Setup a three-level graph: a -(child)-> b -(grandchild)-> c and then try and resolve: /ipfs//child/grandchild Before 10669e8b (path/resolver: Fix recursive path resolution, 2015-05-08) this failed with: resolver_test.go:71: no link named "grandchild" under QmSomeRandomHash The boilerplate for this test is from pin/pin_test.go, and I make no claims that it's the best way to setup the test graph ;). --- path/resolver_test.go | 83 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 path/resolver_test.go diff --git a/path/resolver_test.go b/path/resolver_test.go new file mode 100644 index 000000000..3772f1b9b --- /dev/null +++ b/path/resolver_test.go @@ -0,0 +1,83 @@ +package path_test + +import ( + "fmt" + "testing" + + datastore "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore" + sync "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-datastore/sync" + context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" + + blockstore "github.com/ipfs/go-ipfs/blocks/blockstore" + blockservice "github.com/ipfs/go-ipfs/blockservice" + offline "github.com/ipfs/go-ipfs/exchange/offline" + merkledag "github.com/ipfs/go-ipfs/merkledag" + path "github.com/ipfs/go-ipfs/path" + util "github.com/ipfs/go-ipfs/util" +) + +func randNode() (*merkledag.Node, util.Key) { + node := new(merkledag.Node) + node.Data = make([]byte, 32) + util.NewTimeSeededRand().Read(node.Data) + k, _ := node.Key() + return node, k +} + +func TestRecurivePathResolution(t *testing.T) { + ctx := context.Background() + dstore := sync.MutexWrap(datastore.NewMapDatastore()) + bstore := blockstore.NewBlockstore(dstore) + bserv, err := blockservice.New(bstore, offline.Exchange(bstore)) + if err != nil { + t.Fatal(err) + } + + dagService := merkledag.NewDAGService(bserv) + + a, _ := randNode() + b, _ := randNode() + c, cKey := randNode() + + err = b.AddNodeLink("grandchild", c) + if err != nil { + t.Fatal(err) + } + + err = a.AddNodeLink("child", b) + if err != nil { + t.Fatal(err) + } + + err = dagService.AddRecursive(a) + if err != nil { + t.Fatal(err) + } + + aKey, err := a.Key() + if err != nil { + t.Fatal(err) + } + + segments := []string{"", "ipfs", aKey.String(), "child", "grandchild"} + p, err := path.FromSegments(segments...) + if err != nil { + t.Fatal(err) + } + + resolver := &path.Resolver{DAG: dagService} + node, err := resolver.ResolvePath(ctx, p) + if err != nil { + t.Fatal(err) + } + + key, err := node.Key() + if err != nil { + t.Fatal(err) + } + if key.String() != cKey.String() { + t.Fatal(fmt.Errorf( + "recursive path resolution failed for %s: %s != %s", + p.String(), key.String(), cKey.String())) + } +}