From 42289d4f21a9ec5408098a42f97ca9910ac9a9a6 Mon Sep 17 00:00:00 2001 From: Travis Person Date: Wed, 20 May 2015 19:04:36 -0700 Subject: [PATCH 1/4] Daemon panics if no path is given If no path after `/ipfs/` or `/ipns/` is given, then the daemon will panic with a slice bounds out of range error. This checks to see if we have anything after `ipfs` or `ipns`. --- core/pathresolver.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/pathresolver.go b/core/pathresolver.go index 367b82001..08bbac0e3 100644 --- a/core/pathresolver.go +++ b/core/pathresolver.go @@ -29,6 +29,10 @@ func Resolve(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, er } seg := p.Segments() + if len(seg) < 2 { + return nil, errors.New("No path given") + } + extensions := seg[2:] resolvable, err := path.FromSegments("/", seg[0], seg[1]) if err != nil { From dcc4da0b37109d7073590731794b4891f6bd4b69 Mon Sep 17 00:00:00 2001 From: Travis Person Date: Wed, 20 May 2015 20:38:57 -0700 Subject: [PATCH 2/4] Replaced old logic to check for valid path Added the original logic to check for a invalid path and a simple test. --- core/pathresolver.go | 6 ++++-- core/pathresolver_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 core/pathresolver_test.go diff --git a/core/pathresolver.go b/core/pathresolver.go index 08bbac0e3..1ca605250 100644 --- a/core/pathresolver.go +++ b/core/pathresolver.go @@ -3,6 +3,7 @@ package core import ( "errors" "strings" + "fmt" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" @@ -29,8 +30,9 @@ func Resolve(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, er } seg := p.Segments() - if len(seg) < 2 { - return nil, errors.New("No path given") + + if len(seg) < 2 || seg[1] == "" { // just "/" without further segments + return nil, fmt.Errorf("invalid path: %s", string(p)) } extensions := seg[2:] diff --git a/core/pathresolver_test.go b/core/pathresolver_test.go new file mode 100644 index 000000000..b458a98e8 --- /dev/null +++ b/core/pathresolver_test.go @@ -0,0 +1,41 @@ +package core + +import ( + "testing" + + context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" + config "github.com/ipfs/go-ipfs/repo/config" + "github.com/ipfs/go-ipfs/util/testutil" + "github.com/ipfs/go-ipfs/repo" + path "github.com/ipfs/go-ipfs/path" +) + +func TestResolveInvalidPath(t *testing.T) { + ctx := context.TODO() + id := testIdentity + + r := &repo.Mock{ + C: config.Config{ + Identity: id, + Datastore: config.Datastore{ + Type: "memory", + }, + Addresses: config.Addresses{ + Swarm: []string{"/ip4/0.0.0.0/tcp/4001"}, + API: "/ip4/127.0.0.1/tcp/8000", + }, + }, + D: testutil.ThreadSafeCloserMapDatastore(), + } + + n, err := NewIPFSNode(ctx, Standard(r, false)) + if n == nil || err != nil { + t.Error("Should have constructed.", err) + } + + _, err = Resolve(ctx, n, path.Path("/ipfs/")) + if err == nil { + t.Error("Should get invalid path") + } + +} From d96246c6223021f55049a8714bda70466f47bd24 Mon Sep 17 00:00:00 2001 From: Travis Person Date: Wed, 20 May 2015 21:56:20 -0700 Subject: [PATCH 3/4] Fixed tests to actually test for the error we are seeking Cleaned the tests, and actually test for the error. --- core/pathresolver_test.go | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/core/pathresolver_test.go b/core/pathresolver_test.go index b458a98e8..89262e527 100644 --- a/core/pathresolver_test.go +++ b/core/pathresolver_test.go @@ -3,39 +3,19 @@ package core import ( "testing" - context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" - config "github.com/ipfs/go-ipfs/repo/config" - "github.com/ipfs/go-ipfs/util/testutil" - "github.com/ipfs/go-ipfs/repo" path "github.com/ipfs/go-ipfs/path" + "strings" ) func TestResolveInvalidPath(t *testing.T) { - ctx := context.TODO() - id := testIdentity - - r := &repo.Mock{ - C: config.Config{ - Identity: id, - Datastore: config.Datastore{ - Type: "memory", - }, - Addresses: config.Addresses{ - Swarm: []string{"/ip4/0.0.0.0/tcp/4001"}, - API: "/ip4/127.0.0.1/tcp/8000", - }, - }, - D: testutil.ThreadSafeCloserMapDatastore(), - } - - n, err := NewIPFSNode(ctx, Standard(r, false)) + n, err := NewMockNode() if n == nil || err != nil { - t.Error("Should have constructed.", err) + t.Fatal("Should have constructed.", err) } - _, err = Resolve(ctx, n, path.Path("/ipfs/")) - if err == nil { - t.Error("Should get invalid path") + _, err = Resolve(n.Context(), n, path.Path("/ipfs/")) + if !strings.HasPrefix(err.Error(), "invalid path") { + t.Fatal("Should get invalid path.", err) } } From 2c71c54823e4dbd85482625863afc0538d2abc49 Mon Sep 17 00:00:00 2001 From: Travis Person Date: Fri, 22 May 2015 09:18:49 -0700 Subject: [PATCH 4/4] Named error for `no components` Update the previous `invalid path` error to match the error returned from `SplitAbsPath`. --- core/pathresolver.go | 3 +-- core/pathresolver_test.go | 14 +++++++++----- path/resolver.go | 7 ++++++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/core/pathresolver.go b/core/pathresolver.go index 1ca605250..610eeb50b 100644 --- a/core/pathresolver.go +++ b/core/pathresolver.go @@ -3,7 +3,6 @@ package core import ( "errors" "strings" - "fmt" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" @@ -32,7 +31,7 @@ func Resolve(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, er seg := p.Segments() if len(seg) < 2 || seg[1] == "" { // just "/" without further segments - return nil, fmt.Errorf("invalid path: %s", string(p)) + return nil, path.ErrNoComponents } extensions := seg[2:] diff --git a/core/pathresolver_test.go b/core/pathresolver_test.go index 89262e527..2b694fc4b 100644 --- a/core/pathresolver_test.go +++ b/core/pathresolver_test.go @@ -4,18 +4,22 @@ import ( "testing" path "github.com/ipfs/go-ipfs/path" - "strings" ) -func TestResolveInvalidPath(t *testing.T) { +func TestResolveNoComponents(t *testing.T) { n, err := NewMockNode() if n == nil || err != nil { - t.Fatal("Should have constructed.", err) + t.Fatal("Should have constructed a mock node", err) + } + + _, err = Resolve(n.Context(), n, path.Path("/ipns/")) + if err != path.ErrNoComponents { + t.Fatal("Should error with no components (/ipns/).", err) } _, err = Resolve(n.Context(), n, path.Path("/ipfs/")) - if !strings.HasPrefix(err.Error(), "invalid path") { - t.Fatal("Should get invalid path.", err) + if err != path.ErrNoComponents { + t.Fatal("Should error with no components (/ipfs/).", err) } } diff --git a/path/resolver.go b/path/resolver.go index a08df8741..b4d6239dd 100644 --- a/path/resolver.go +++ b/path/resolver.go @@ -4,6 +4,7 @@ package path import ( "fmt" "time" + "errors" mh "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multihash" "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" @@ -14,6 +15,10 @@ import ( var log = u.Logger("path") +// Paths after a protocol must contain at least one component +var ErrNoComponents = errors.New( + "path must contain at least one component") + // ErrNoLink is returned when a link is not found in a path type ErrNoLink struct { name string @@ -43,7 +48,7 @@ func SplitAbsPath(fpath Path) (mh.Multihash, []string, error) { // if nothing, bail. if len(parts) == 0 { - return nil, nil, fmt.Errorf("ipfs path must contain at least one component") + return nil, nil, ErrNoComponents } // first element in the path is a b58 hash (for now)