Merge pull request #1441 from rht/path-validate

Remove redundant path validation

core/commands/get.go

@@ -65,14 +65,8 @@ may also specify the level of compression by specifying '-l=<1-9>'.
 			return
 		}
 
-		// Validate path string
-		p, err := path.ParsePath(req.Arguments()[0])
-		if err != nil {
-			res.SetError(fmt.Errorf("failed to validate path: %v", err), cmds.ErrNormal)
-			return
-		}
+		p := path.Path(req.Arguments()[0])
 		var reader io.Reader
-
 		if archive, _, _ := req.Option("archive").Bool(); !archive && cmplvl != gzip.NoCompression {
 			// only use this when the flag is '-C' without '-a'
 			reader, err = getZip(req.Context().Context, node, p, cmplvl)
@@ -169,18 +163,18 @@ func getCompressOptions(req cmds.Request) (int, error) {
 	return gzip.NoCompression, nil
 }
 
-func get(ctx context.Context, node *core.IpfsNode, pathToResolve path.Path, compression int) (io.Reader, error) {
-	dagnode, err := core.Resolve(ctx, node, pathToResolve)
+func get(ctx context.Context, node *core.IpfsNode, p path.Path, compression int) (io.Reader, error) {
+	dagnode, err := core.Resolve(ctx, node, p)
 	if err != nil {
 		return nil, err
 	}
 
-	return utar.NewReader(pathToResolve, node.DAG, dagnode, compression)
+	return utar.NewReader(p, node.DAG, dagnode, compression)
 }
 
 // getZip is equivalent to `ipfs getdag $hash | gzip`
-func getZip(ctx context.Context, node *core.IpfsNode, pathToResolve path.Path, compression int) (io.Reader, error) {
-	dagnode, err := core.Resolve(ctx, node, pathToResolve)
+func getZip(ctx context.Context, node *core.IpfsNode, p path.Path, compression int) (io.Reader, error) {
+	dagnode, err := core.Resolve(ctx, node, p)
 	if err != nil {
 		return nil, err
 	}

core/commands/publish.go

@@ -88,15 +88,9 @@ Publish an <ipfs-path> to another public key (not implemented):
 			pstr = args[0]
 		}
 
-		p, err := path.ParsePath(pstr)
-		if err != nil {
-			res.SetError(fmt.Errorf("failed to validate path: %v", err), cmds.ErrNormal)
-			return
-		}
-
 		// TODO n.Keychain.Get(name).PrivKey
 		// TODO(cryptix): is req.Context().Context a child of n.Context()?
-		output, err := publish(req.Context().Context, n, n.PrivateKey, p)
+		output, err := publish(req.Context().Context, n, n.PrivateKey, path.Path(pstr))
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)
 			return

core/pathresolver_test.go

@@ -24,4 +24,8 @@ func TestResolveNoComponents(t *testing.T) {
 		t.Fatal("Should error with no components (/ipfs/).", err)
 	}
 
+	_, err = core.Resolve(n.Context(), n, path.Path("/../.."))
+	if err != path.ErrBadPath {
+		t.Fatal("Should error with invalid path.", err)
+	}
 }

path/path.go

@@ -61,12 +61,15 @@ func ParsePath(txt string) (Path, error) {
 	}
 
 	if parts[0] != "" {
-		return "", ErrBadPath
+		if _, err := ParseKeyToPath(parts[0]); err != nil {
+			return "", ErrBadPath
+		}
+		// The case when the path starts with hash without a protocol prefix
+		return Path("/ipfs/" + txt), nil
 	}
 
 	if parts[1] == "ipfs" {
-		_, err := ParseKeyToPath(parts[2])
-		if err != nil {
+		if _, err := ParseKeyToPath(parts[2]); err != nil {
 			return "", err
 		}
 	} else if parts[1] != "ipns" {
@@ -77,13 +80,16 @@ func ParsePath(txt string) (Path, error) {
 }
 
 func ParseKeyToPath(txt string) (Path, error) {
+	if txt == "" {
+		return "", ErrNoComponents
+	}
+
 	chk := b58.Decode(txt)
 	if len(chk) == 0 {
 		return "", errors.New("not a key")
 	}
 
-	_, err := mh.Cast(chk)
-	if err != nil {
+	if _, err := mh.Cast(chk); err != nil {
 		return "", err
 	}
 	return FromKey(key.Key(chk)), nil

path/resolver.go

@@ -65,6 +65,11 @@ func SplitAbsPath(fpath Path) (mh.Multihash, []string, error) {
 // ResolvePath fetches the node for given path. It returns the last item
 // returned by ResolvePathComponents.
 func (s *Resolver) ResolvePath(ctx context.Context, fpath Path) (*merkledag.Node, error) {
+	// validate path
+	if err := fpath.IsValid(); err != nil {
+		return nil, err
+	}
+
 	nodes, err := s.ResolvePathComponents(ctx, fpath)
 	if err != nil || nodes == nil {
 		return nil, err

test/sharness/t0090-get.sh

@@ -112,6 +112,12 @@ test_get_cmd() {
 	  test_cmp dir/b/c "$HASH2"/b/c &&
 	  rm -r "$HASH2"
 	'
+
+    test_expect_success "ipfs get ../.. should fail" '
+      echo "Error: invalid ipfs ref path" >expected &&
+      test_must_fail ipfs get ../.. 2>actual &&
+      test_cmp expected actual
+    '
 }
 
 # should work offline