base32: make GetEncoderFromPath more robust

Primarily, get rid of extractCidString and cidVer. Neither of these functions
did sane things when a path when a path didn't actually include a CID. For
example, "boo" would yield a base32 encoder.

Also:

* Avoid "optional" errors.
* Make it a pure function of the input path.
* Extract the multibase from *any* type of path of the form
  /namespace/cid-like-thing/... This is a DWIM function.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>

core/commands/cmdenv/cidbase.go

@@ -1,8 +1,10 @@
 package cmdenv
 
 import (
+	"fmt"
 	"strings"
 
+	cid "gx/ipfs/QmR8BauakNcBa3RbE4nbQu76PDiJgoQgz8AJdhJuiU4TAw/go-cid"
 	cmds "gx/ipfs/QmWGm4AbZEbnmdgVTza52MSNpEmBdFVqzmAysRbjrRyGbH/go-ipfs-cmds"
 	cidenc "gx/ipfs/QmdPQx9fvN5ExVwMhRmh7YpCQJzJrFhd1AjVBwJmRMFJeX/go-cidutil/cidenc"
 	cmdkit "gx/ipfs/Qmde5VP1qUkyQXKCfmEUA7bP64V2HAptbJ7phuPp7jXWwg/go-ipfs-cmdkit"
@@ -59,32 +61,44 @@ func CidBaseDefined(req *cmds.Request) bool {
 // CidEncoderFromPath creates a new encoder that is influenced from
 // the encoded Cid in a Path.  For CidV0 the multibase from the base
 // encoder is used and automatic upgrades are disabled.  For CidV1 the
-// multibase from the CID is used and upgrades are eneabled.  On error
-// the base encoder is returned.  If you don't care about the error
-// condition, it is safe to ignore the error returned.
-func CidEncoderFromPath(enc cidenc.Encoder, p string) (cidenc.Encoder, error) {
-	v := extractCidString(p)
-	if cidVer(v) == 0 {
-		return cidenc.Encoder{Base: enc.Base, Upgrade: false}, nil
+// multibase from the CID is used and upgrades are enabled.
+//
+// This logic is intentionally fuzzy and will match anything of the form
+// `CidLike` or `/namespace/CidLike/...`.
+//
+// For example:
+//
+// * Qm...
+// * /ipfs/Qm...
+// * /ipns/bafybeiahnxfi7fpmr5wtxs2imx4abnyn7fdxeiox7xxjem6zuiioqkh6zi/...
+// * /bzz/bafybeiahnxfi7fpmr5wtxs2imx4abnyn7fdxeiox7xxjem6zuiioqkh6zi/...
+func CidEncoderFromPath(p string) (cidenc.Encoder, error) {
+	components := strings.SplitN(p, "/", 4)
+
+	var maybeCid string
+	if len(components) == 1 {
+		maybeCid = components[0]
+	} else if len(components) < 3 {
+		return cidenc.Encoder{}, fmt.Errorf("no cid in path: %s", p)
+	} else {
+		maybeCid = components[2]
 	}
-	e, err := mbase.NewEncoder(mbase.Encoding(v[0]))
+	c, err := cid.Decode(maybeCid)
 	if err != nil {
-		return enc, err
+		// Ok, not a CID-like thing. Keep the current encoder.
+		return cidenc.Encoder{}, fmt.Errorf("no cid in path: %s", p)
 	}
-	return cidenc.Encoder{Base: e, Upgrade: true}, nil
-}
-
-func extractCidString(str string) string {
-	parts := strings.Split(str, "/")
-	if len(parts) > 2 && (parts[1] == "ipfs" || parts[1] == "ipld") {
-		return parts[2]
+	if c.Version() == 0 {
+		// Version 0, use the base58 non-upgrading encoder.
+		return cidenc.Default(), nil
 	}
-	return str
-}
 
-func cidVer(v string) int {
-	if len(v) == 46 && v[:2] == "Qm" {
-		return 0
+	// Version 1+, extract multibase encoding.
+	encoding, _, err := mbase.Decode(maybeCid)
+	if err != nil {
+		// This should be impossible, we've already decoded the cid.
+		panic(fmt.Sprintf("BUG: failed to get multibase decoder for CID %s", maybeCid))
 	}
-	return 1
+
+	return cidenc.Encoder{Base: mbase.MustNewEncoder(encoding), Upgrade: true}, nil
 }

core/commands/cmdenv/cidbase_test.go

@@ -2,29 +2,64 @@ package cmdenv
 
 import (
 	"testing"
+
+	cidenc "gx/ipfs/QmdPQx9fvN5ExVwMhRmh7YpCQJzJrFhd1AjVBwJmRMFJeX/go-cidutil/cidenc"
+	mbase "gx/ipfs/QmekxXDhCxCJRNuzmHreuaT3BsuJcsjcXWNrtV9C8DRHtd/go-multibase"
 )
 
-func TestExtractCidString(t *testing.T) {
-	test := func(path string, cid string) {
-		res := extractCidString(path)
-		if res != cid {
-			t.Errorf("extractCidString(%s) failed: expected '%s' but got '%s'", path, cid, res)
+func TestEncoderFromPath(t *testing.T) {
+	test := func(path string, expected cidenc.Encoder) {
+		actual, err := CidEncoderFromPath(path)
+		if err != nil {
+			t.Error(err)
+		}
+		if actual != expected {
+			t.Errorf("CidEncoderFromPath(%s) failed: expected %#v but got %#v", path, expected, actual)
 		}
 	}
 	p := "QmRqVG8VGdKZ7KARqR96MV7VNHgWvEQifk94br5HpURpfu"
-	test(p, p)
-	test("/ipfs/"+p, p)
+	enc := cidenc.Default()
+	test(p, enc)
+	test("/ipfs/"+p, enc)
+	test("/ipfs/"+p+"/b", enc)
 
 	p = "zb2rhfkM4FjkMLaUnygwhuqkETzbYXnUDf1P9MSmdNjW1w1Lk"
-	test(p, p)
-	test("/ipfs/"+p, p)
-	test("/ipld/"+p, p)
+	enc = cidenc.Encoder{
+		Base:    mbase.MustNewEncoder(mbase.Base58BTC),
+		Upgrade: true,
+	}
+	test(p, enc)
+	test("/ipfs/"+p, enc)
+	test("/ipfs/"+p+"/b", enc)
+	test("/ipld/"+p, enc)
+	test("/ipns/"+p, enc) // even IPNS should work.
 
 	p = "bafyreifrcnyjokuw4i4ggkzg534tjlc25lqgt3ttznflmyv5fftdgu52hm"
-	test(p, p)
-	test("/ipfs/"+p, p)
-	test("/ipld/"+p, p)
+	enc = cidenc.Encoder{
+		Base:    mbase.MustNewEncoder(mbase.Base32),
+		Upgrade: true,
+	}
+	test(p, enc)
+	test("/ipfs/"+p, enc)
+	test("/ipld/"+p, enc)
 
-	// an error is also acceptable in future versions of extractCidString
-	test("/ipfs", "/ipfs")
+	for _, badPath := range []string{
+		"/ipld/",
+		"/ipld",
+		"/ipld//",
+		"ipld//",
+		"ipld",
+		"",
+		"ipns",
+		"/ipfs/asdf",
+		"/ipfs/...",
+		"...",
+		"abcdefg",
+		"boo",
+	} {
+		_, err := CidEncoderFromPath(badPath)
+		if err == nil {
+			t.Errorf("expected error extracting encoder from bad path: %s", badPath)
+		}
+	}
 }

core/commands/dag/dag.go

@@ -14,6 +14,7 @@ import (
 	cmds "gx/ipfs/QmWGm4AbZEbnmdgVTza52MSNpEmBdFVqzmAysRbjrRyGbH/go-ipfs-cmds"
 	files "gx/ipfs/QmXWZCd8jfaHmt4UDSnjKmGcrQMw95bDGWqEeVLVJjoANX/go-ipfs-files"
 	ipld "gx/ipfs/QmcKKBwfz6FyQdHR2jsXrrF6XeSBXYL86anmWNewpFpoF5/go-ipld-format"
+	cidenc "gx/ipfs/QmdPQx9fvN5ExVwMhRmh7YpCQJzJrFhd1AjVBwJmRMFJeX/go-cidutil/cidenc"
 	cmdkit "gx/ipfs/Qmde5VP1qUkyQXKCfmEUA7bP64V2HAptbJ7phuPp7jXWwg/go-ipfs-cmdkit"
 	mh "gx/ipfs/QmerPMzPk1mJVowm8KgmoknWa4yCYvvugMPsgWmDNUvDLW/go-multihash"
 )
@@ -231,12 +232,24 @@ var DagResolveCmd = &cmds.Command{
 	},
 	Encoders: cmds.EncoderMap{
 		cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, out *ResolveOutput) error {
-			enc, err := cmdenv.GetLowLevelCidEncoder(req)
-			if err != nil {
-				return err
-			}
-			if !cmdenv.CidBaseDefined(req) {
-				enc, _ = cmdenv.CidEncoderFromPath(enc, req.Arguments[0])
+			var (
+				enc cidenc.Encoder
+				err error
+			)
+			switch {
+			case !cmdenv.CidBaseDefined(req):
+				// Not specified, check the path.
+				enc, err = cmdenv.CidEncoderFromPath(req.Arguments[0])
+				if err == nil {
+					break
+				}
+				// Nope, fallback on the default.
+				fallthrough
+			default:
+				enc, err = cmdenv.GetLowLevelCidEncoder(req)
+				if err != nil {
+					return err
+				}
 			}
 			p := enc.Encode(out.Cid)
 			if out.RemPath != "" {

core/commands/resolve.go

@@ -16,6 +16,7 @@ import (
 	path "gx/ipfs/QmNYPETsdAu2uQ1k9q9S1jYEGURaLHV6cbYRSVFVRftpF8/go-path"
 
 	cmds "gx/ipfs/QmWGm4AbZEbnmdgVTza52MSNpEmBdFVqzmAysRbjrRyGbH/go-ipfs-cmds"
+	cidenc "gx/ipfs/QmdPQx9fvN5ExVwMhRmh7YpCQJzJrFhd1AjVBwJmRMFJeX/go-cidutil/cidenc"
 	cmdkit "gx/ipfs/Qmde5VP1qUkyQXKCfmEUA7bP64V2HAptbJ7phuPp7jXWwg/go-ipfs-cmdkit"
 )
 
@@ -82,12 +83,21 @@ Resolve the value of an IPFS DAG path:
 		name := req.Arguments[0]
 		recursive, _ := req.Options[resolveRecursiveOptionName].(bool)
 
-		enc, err := cmdenv.GetCidEncoder(req)
-		if err != nil {
-			return err
-		}
-		if !cmdenv.CidBaseDefined(req) {
-			enc, _ = cmdenv.CidEncoderFromPath(enc, name)
+		var enc cidenc.Encoder
+		switch {
+		case !cmdenv.CidBaseDefined(req):
+			// Not specified, check the path.
+			enc, err = cmdenv.CidEncoderFromPath(name)
+			if err == nil {
+				break
+			}
+			// Nope, fallback on the default.
+			fallthrough
+		default:
+			enc, err = cmdenv.GetCidEncoder(req)
+			if err != nil {
+				return err
+			}
 		}
 
 		// the case when ipns is resolved step by step