Merge pull request #1219 from ipfs/refactor/httpGateway-quickfix

Refactor/http gateway quickfix (1191)

core/corehttp/gateway_handler.go

@@ -19,23 +19,15 @@ import (
 	dag "github.com/ipfs/go-ipfs/merkledag"
 	path "github.com/ipfs/go-ipfs/path"
 	"github.com/ipfs/go-ipfs/routing"
-	ufs "github.com/ipfs/go-ipfs/unixfs"
 	uio "github.com/ipfs/go-ipfs/unixfs/io"
 	u "github.com/ipfs/go-ipfs/util"
 )
 
 const (
-	IpfsPathPrefix = "/ipfs/"
+	ipfsPathPrefix = "/ipfs/"
-	IpnsPathPrefix = "/ipns/"
+	ipnsPathPrefix = "/ipns/"
 )
 
-type gateway interface {
-	ResolvePath(string) (*dag.Node, error)
-	NewDagFromReader(io.Reader) (*dag.Node, error)
-	AddNodeToDAG(nd *dag.Node) (u.Key, error)
-	NewDagReader(nd *dag.Node) (uio.ReadSeekCloser, error)
-}
-
 // shortcut for templating
 type webHandler map[string]interface{}
 
@@ -76,80 +68,31 @@ func (i *gatewayHandler) loadTemplate() error {
 	return nil
 }
 
-func (i *gatewayHandler) resolveNamePath(ctx context.Context, p string) (string, error) {
+// TODO(cryptix):  find these helpers somewhere else
-	p = gopath.Clean(p)
+func (i *gatewayHandler) newDagFromReader(r io.Reader) (*dag.Node, error) {
-
+	// TODO(cryptix): change and remove this helper once PR1136 is merged
-	if strings.HasPrefix(p, IpnsPathPrefix) {
+	// return ufs.AddFromReader(i.node, r.Body)
-		elements := strings.Split(p[len(IpnsPathPrefix):], "/")
-		hash := elements[0]
-		rp, err := i.node.Namesys.Resolve(ctx, hash)
-		if err != nil {
-			return "", err
-		}
-
-		elements = append(rp.Segments(), elements[1:]...)
-		p = gopath.Join(elements...)
-	}
-	if !strings.HasPrefix(p, IpfsPathPrefix) {
-		p = gopath.Join(IpfsPathPrefix, p)
-	}
-	return p, nil
-}
-
-func (i *gatewayHandler) ResolvePath(ctx context.Context, p string) (*dag.Node, string, error) {
-	p, err := i.resolveNamePath(ctx, p)
-	if err != nil {
-		return nil, "", err
-	}
-
-	node, err := i.node.Resolver.ResolvePath(ctx, path.Path(p))
-	if err != nil {
-		return nil, "", err
-	}
-	return node, p, err
-}
-
-func (i *gatewayHandler) NewDagFromReader(r io.Reader) (*dag.Node, error) {
 	return importer.BuildDagFromReader(
 		r, i.node.DAG, i.node.Pinning.GetManual(), chunk.DefaultSplitter)
 }
 
-func NewDagEmptyDir() *dag.Node {
+// TODO(btc): break this apart into separate handlers using a more expressive muxer
-	return &dag.Node{Data: ufs.FolderPBData()}
-}
-
-func (i *gatewayHandler) AddNodeToDAG(nd *dag.Node) (u.Key, error) {
-	return i.node.DAG.Add(nd)
-}
-
-func (i *gatewayHandler) NewDagReader(nd *dag.Node) (uio.ReadSeekCloser, error) {
-	return uio.NewDagReader(i.node.Context(), nd, i.node.DAG)
-}
-
-// TODO(btc): break this apart into separate handlers using a more expressive
-// muxer
 func (i *gatewayHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	if i.config.Writable && r.Method == "POST" {
+	if i.config.Writable {
-		i.postHandler(w, r)
+		switch r.Method {
-		return
+		case "POST":
+			i.postHandler(w, r)
+			return
+		case "PUT":
+			i.putHandler(w, r)
+			return
+		case "DELETE":
+			i.deleteHandler(w, r)
+			return
+		}
 	}
 
-	if i.config.Writable && r.Method == "PUT" {
+	if r.Method == "GET" || r.Method == "HEAD" {
-		i.putHandler(w, r)
-		return
-	}
-
-	if i.config.Writable && r.Method == "DELETE" {
-		i.deleteHandler(w, r)
-		return
-	}
-
-	if r.Method == "GET" {
-		i.getOrHeadHandler(w, r)
-		return
-	}
-
-	if r.Method == "HEAD" {
 		i.getOrHeadHandler(w, r)
 		return
 	}
@@ -162,8 +105,8 @@ func (i *gatewayHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusBadRequest)
 		errmsg = errmsg + "bad request for " + r.URL.Path
 	}
-	w.Write([]byte(errmsg))
+	fmt.Fprint(w, errmsg)
-	log.Debug(errmsg)
+	log.Error(errmsg) // TODO(cryptix): log errors until we have a better way to expose these (counter metrics maybe)
 }
 
 func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
@@ -178,19 +121,19 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 		return
 	}
 
-	nd, p, err := i.ResolvePath(ctx, urlPath)
+	nd, err := core.Resolve(ctx, i.node, path.Path(urlPath))
 	if err != nil {
 		webError(w, "Path Resolve error", err, http.StatusBadRequest)
 		return
 	}
 
-	etag := gopath.Base(p)
+	etag := gopath.Base(urlPath)
 	if r.Header.Get("If-None-Match") == etag {
 		w.WriteHeader(http.StatusNotModified)
 		return
 	}
 
-	w.Header().Set("X-IPFS-Path", p)
+	w.Header().Set("X-IPFS-Path", urlPath)
 
 	// Suborigin header, sandboxes apps from each other in the browser (even
 	// though they are served from the same gateway domain). NOTE: This is not
@@ -198,7 +141,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 	pathRoot := strings.SplitN(urlPath, "/", 4)[2]
 	w.Header().Set("Suborigin", pathRoot)
 
-	dr, err := i.NewDagReader(nd)
+	dr, err := uio.NewDagReader(ctx, nd, i.node.DAG)
 	if err != nil && err != uio.ErrIsDir {
 		// not a directory and still an error
 		internalWebError(w, err)
@@ -210,7 +153,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 	// and only if it's /ipfs!
 	// TODO: break this out when we split /ipfs /ipns routes.
 	modtime := time.Now()
-	if strings.HasPrefix(urlPath, IpfsPathPrefix) {
+	if strings.HasPrefix(urlPath, ipfsPathPrefix) {
 		w.Header().Set("Etag", etag)
 		w.Header().Set("Cache-Control", "public, max-age=29030400")
 
@@ -239,12 +182,12 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 			log.Debug("found index")
 			foundIndex = true
 			// return index page instead.
-			nd, _, err := i.ResolvePath(ctx, urlPath+"/index.html")
+			nd, err := core.Resolve(ctx, i.node, path.Path(urlPath+"/index.html"))
 			if err != nil {
 				internalWebError(w, err)
 				return
 			}
-			dr, err := i.NewDagReader(nd)
+			dr, err := uio.NewDagReader(ctx, nd, i.node.DAG)
 			if err != nil {
 				internalWebError(w, err)
 				return
@@ -279,13 +222,13 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 }
 
 func (i *gatewayHandler) postHandler(w http.ResponseWriter, r *http.Request) {
-	nd, err := i.NewDagFromReader(r.Body)
+	nd, err := i.newDagFromReader(r.Body)
 	if err != nil {
 		internalWebError(w, err)
 		return
 	}
 
-	k, err := i.AddNodeToDAG(nd)
+	k, err := i.node.DAG.Add(nd)
 	if err != nil {
 		internalWebError(w, err)
 		return
@@ -293,11 +236,11 @@ func (i *gatewayHandler) postHandler(w http.ResponseWriter, r *http.Request) {
 
 	h := mh.Multihash(k).B58String()
 	w.Header().Set("IPFS-Hash", h)
-	http.Redirect(w, r, IpfsPathPrefix+h, http.StatusCreated)
+	http.Redirect(w, r, ipfsPathPrefix+h, http.StatusCreated)
 }
 
 func (i *gatewayHandler) putEmptyDirHandler(w http.ResponseWriter, r *http.Request) {
-	newnode := NewDagEmptyDir()
+	newnode := uio.NewEmptyDirectory()
 
 	key, err := i.node.DAG.Add(newnode)
 	if err != nil {
@@ -306,7 +249,7 @@ func (i *gatewayHandler) putEmptyDirHandler(w http.ResponseWriter, r *http.Reque
 	}
 
 	w.Header().Set("IPFS-Hash", key.String())
-	http.Redirect(w, r, IpfsPathPrefix+key.String()+"/", http.StatusCreated)
+	http.Redirect(w, r, ipfsPathPrefix+key.String()+"/", http.StatusCreated)
 }
 
 func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
@@ -316,16 +259,16 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
 	urlPath := r.URL.Path
 	pathext := urlPath[5:]
 	var err error
-	if urlPath == IpfsPathPrefix+"QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn/" {
+	if urlPath == ipfsPathPrefix+"QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn/" {
 		i.putEmptyDirHandler(w, r)
 		return
 	}
 
 	var newnode *dag.Node
 	if pathext[len(pathext)-1] == '/' {
-		newnode = NewDagEmptyDir()
+		newnode = uio.NewEmptyDirectory()
 	} else {
-		newnode, err = i.NewDagFromReader(r.Body)
+		newnode, err = i.newDagFromReader(r.Body)
 		if err != nil {
 			webError(w, "Could not create DAG from request", err, http.StatusInternalServerError)
 			return
@@ -335,14 +278,20 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithCancel(i.node.Context())
 	defer cancel()
 
-	ipfspath, err := i.resolveNamePath(ctx, urlPath)
+	ipfsNode, err := core.Resolve(ctx, i.node, path.Path(urlPath))
 	if err != nil {
 		// FIXME HTTP error code
 		webError(w, "Could not resolve name", err, http.StatusInternalServerError)
 		return
 	}
 
-	h, components, err := path.SplitAbsPath(path.Path(ipfspath))
+	k, err := ipfsNode.Key()
+	if err != nil {
+		webError(w, "Could not get key from resolved node", err, http.StatusInternalServerError)
+		return
+	}
+
+	h, components, err := path.SplitAbsPath(path.FromKey(k))
 	if err != nil {
 		webError(w, "Could not split path", err, http.StatusInternalServerError)
 		return
@@ -350,14 +299,13 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
 
 	if len(components) < 1 {
 		err = fmt.Errorf("Cannot override existing object")
-		w.WriteHeader(http.StatusBadRequest)
+		webError(w, "http gateway", err, http.StatusBadRequest)
-		w.Write([]byte(err.Error()))
-		log.Debug("%s", err)
 		return
 	}
 
 	tctx, cancel := context.WithTimeout(ctx, time.Minute)
 	defer cancel()
+	// TODO(cryptix): could this be core.Resolve() too?
 	rootnd, err := i.node.Resolver.DAG.Get(tctx, u.Key(h))
 	if err != nil {
 		webError(w, "Could not resolve root object", err, http.StatusBadRequest)
@@ -366,19 +314,19 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
 
 	// resolving path components into merkledag nodes. if a component does not
 	// resolve, create empty directories (which will be linked and populated below.)
-	path_nodes, err := i.node.Resolver.ResolveLinks(tctx, rootnd, components[:len(components)-1])
+	pathNodes, err := i.node.Resolver.ResolveLinks(tctx, rootnd, components[:len(components)-1])
 	if _, ok := err.(path.ErrNoLink); ok {
 		// Create empty directories, links will be made further down the code
-		for len(path_nodes) < len(components) {
+		for len(pathNodes) < len(components) {
-			path_nodes = append(path_nodes, NewDagEmptyDir())
+			pathNodes = append(pathNodes, uio.NewDirectory(i.node.DAG).GetNode())
 		}
 	} else if err != nil {
 		webError(w, "Could not resolve parent object", err, http.StatusBadRequest)
 		return
 	}
 
-	for i := len(path_nodes) - 1; i >= 0; i-- {
+	for i := len(pathNodes) - 1; i >= 0; i-- {
-		newnode, err = path_nodes[i].UpdateNodeLink(components[i], newnode)
+		newnode, err = pathNodes[i].UpdateNodeLink(components[i], newnode)
 		if err != nil {
 			webError(w, "Could not update node links", err, http.StatusInternalServerError)
 			return
@@ -399,7 +347,7 @@ func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("IPFS-Hash", key.String())
-	http.Redirect(w, r, IpfsPathPrefix+key.String()+"/"+strings.Join(components, "/"), http.StatusCreated)
+	http.Redirect(w, r, ipfsPathPrefix+key.String()+"/"+strings.Join(components, "/"), http.StatusCreated)
 }
 
 func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
@@ -407,14 +355,20 @@ func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithCancel(i.node.Context())
 	defer cancel()
 
-	ipfspath, err := i.resolveNamePath(ctx, urlPath)
+	ipfsNode, err := core.Resolve(ctx, i.node, path.Path(urlPath))
 	if err != nil {
 		// FIXME HTTP error code
 		webError(w, "Could not resolve name", err, http.StatusInternalServerError)
 		return
 	}
 
-	h, components, err := path.SplitAbsPath(path.Path(ipfspath))
+	k, err := ipfsNode.Key()
+	if err != nil {
+		webError(w, "Could not get key from resolved node", err, http.StatusInternalServerError)
+		return
+	}
+
+	h, components, err := path.SplitAbsPath(path.FromKey(k))
 	if err != nil {
 		webError(w, "Could not split path", err, http.StatusInternalServerError)
 		return
@@ -428,21 +382,22 @@ func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	path_nodes, err := i.node.Resolver.ResolveLinks(tctx, rootnd, components[:len(components)-1])
+	pathNodes, err := i.node.Resolver.ResolveLinks(tctx, rootnd, components[:len(components)-1])
 	if err != nil {
 		webError(w, "Could not resolve parent object", err, http.StatusBadRequest)
 		return
 	}
 
-	err = path_nodes[len(path_nodes)-1].RemoveNodeLink(components[len(components)-1])
+	// TODO(cyrptix): assumes len(pathNodes) > 1 - not found is an error above?
+	err = pathNodes[len(pathNodes)-1].RemoveNodeLink(components[len(components)-1])
 	if err != nil {
 		webError(w, "Could not delete link", err, http.StatusBadRequest)
 		return
 	}
 
-	newnode := path_nodes[len(path_nodes)-1]
+	newnode := pathNodes[len(pathNodes)-1]
-	for i := len(path_nodes) - 2; i >= 0; i-- {
+	for i := len(pathNodes) - 2; i >= 0; i-- {
-		newnode, err = path_nodes[i].UpdateNodeLink(components[i], newnode)
+		newnode, err = pathNodes[i].UpdateNodeLink(components[i], newnode)
 		if err != nil {
 			webError(w, "Could not update node links", err, http.StatusInternalServerError)
 			return
@@ -463,7 +418,7 @@ func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("IPFS-Hash", key.String())
-	http.Redirect(w, r, IpfsPathPrefix+key.String()+"/"+strings.Join(components[:len(components)-1], "/"), http.StatusCreated)
+	http.Redirect(w, r, ipfsPathPrefix+key.String()+"/"+strings.Join(components[:len(components)-1], "/"), http.StatusCreated)
 }
 
 func webError(w http.ResponseWriter, message string, err error, defaultCode int) {
@@ -480,15 +435,13 @@ func webError(w http.ResponseWriter, message string, err error, defaultCode int)
 
 func webErrorWithCode(w http.ResponseWriter, message string, err error, code int) {
 	w.WriteHeader(code)
-	log.Debugf("%s: %s", message, err)
+	log.Errorf("%s: %s", message, err) // TODO(cryptix): log errors until we have a better way to expose these (counter metrics maybe)
-	w.Write([]byte(message + ": " + err.Error()))
+	fmt.Fprintf(w, "%s: %s", message, err)
 }
 
 // return a 500 error and log
 func internalWebError(w http.ResponseWriter, err error) {
-	w.WriteHeader(http.StatusInternalServerError)
+	webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError)
-	w.Write([]byte(err.Error()))
-	log.Debug("%s", err)
 }
 
 // Directory listing template

unixfs/io/dirbuilder.go

@@ -15,15 +15,22 @@ type directoryBuilder struct {
 	dirnode *mdag.Node
 }
 
+// NewEmptyDirectory returns an empty merkledag Node with a folder Data chunk
+func NewEmptyDirectory() *mdag.Node {
+	return &mdag.Node{Data: format.FolderPBData()}
+}
+
+// NewDirectory returns a directoryBuilder. It needs a DAGService to add the Children
 func NewDirectory(dserv mdag.DAGService) *directoryBuilder {
 	db := new(directoryBuilder)
 	db.dserv = dserv
-	db.dirnode = new(mdag.Node)
+	db.dirnode = NewEmptyDirectory()
-	db.dirnode.Data = format.FolderPBData()
 	return db
 }
 
+// AddChild adds a (name, key)-pair to the root node.
 func (d *directoryBuilder) AddChild(name string, k u.Key) error {
+	// TODO(cryptix): consolidate context managment
 	ctx, cancel := context.WithTimeout(context.TODO(), time.Minute)
 	defer cancel()
 
@@ -40,6 +47,7 @@ func (d *directoryBuilder) AddChild(name string, k u.Key) error {
 	return nil
 }
 
+// GetNode returns the root of this directoryBuilder
 func (d *directoryBuilder) GetNode() *mdag.Node {
 	return d.dirnode
 }