Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixing putHandler #1886

Merged
merged 2 commits into from
Oct 31, 2015
+59 −87
Merged

fixing putHandler #1886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8b4e45
fixing putHandler for --writable http gateway
cryptix Oct 22, 2015
c1847e9
putHandler: addressed CR from @jbenet
cryptix Oct 31, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 51 additions & 80 deletions core/corehttp/gateway_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/ipfs/go-ipfs/importer"
chunk "github.com/ipfs/go-ipfs/importer/chunk"
dag "github.com/ipfs/go-ipfs/merkledag"
dagutils "github.com/ipfs/go-ipfs/merkledag/utils"
path "github.com/ipfs/go-ipfs/path"
"github.com/ipfs/go-ipfs/routing"
uio "github.com/ipfs/go-ipfs/unixfs/io"
Expand Down Expand Up @@ -273,116 +274,86 @@ func (i *gatewayHandler) postHandler(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, ipfsPathPrefix+k.String(), http.StatusCreated)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened to this part? do empty dirs still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup - works by virtue of core.Resolve and friends iirc. There is also a sharness test that checks against the hash.

func (i *gatewayHandler) putEmptyDirHandler(w http.ResponseWriter, r *http.Request) {
newnode := uio.NewEmptyDirectory()
func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should derive a ctx here and cancel it to ensure everything is wiped at the end.

ctx, cancel := context.WithCancel(i.node.Context())
defer cancel()

// TODO(cryptix): move me to ServeHTTP and pass into all handlers
ctx, cancel := context.WithCancel(i.node.Context())
defer cancel()

key, err := i.node.DAG.Add(newnode)
rootPath, err := path.ParsePath(r.URL.Path)
if err != nil {
webError(w, "Could not recursively add new node", err, http.StatusInternalServerError)
webError(w, "putHandler: ipfs path not valid", err, http.StatusBadRequest)
return
}

i.addUserHeaders(w) // ok, _now_ write user's headers.
w.Header().Set("IPFS-Hash", key.String())
http.Redirect(w, r, ipfsPathPrefix+key.String()+"/", http.StatusCreated)
}

func (i *gatewayHandler) putHandler(w http.ResponseWriter, r *http.Request) {
// TODO(cryptix): either ask mildred about the flow of this or rewrite it
webErrorWithCode(w, "Sorry, PUT is bugged right now, closing request", errors.New("handler disabled"), http.StatusInternalServerError)
return
urlPath := r.URL.Path
pathext := urlPath[5:]
var err error
if urlPath == ipfsPathPrefix+"QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn/" {
i.putEmptyDirHandler(w, r)
rsegs := rootPath.Segments()
if rsegs[0] == ipnsPathPrefix {
webError(w, "putHandler: updating named entries not supported", errors.New("WritableGateway: ipns put not supported"), http.StatusBadRequest)
return
}

var newnode *dag.Node
if pathext[len(pathext)-1] == '/' {
if rsegs[len(rsegs)-1] == "QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn" {
newnode = uio.NewEmptyDirectory()
} else {
newnode, err = i.newDagFromReader(r.Body)
putNode, err := i.newDagFromReader(r.Body)
if err != nil {
webError(w, "Could not create DAG from request", err, http.StatusInternalServerError)
webError(w, "putHandler: Could not create DAG from request", err, http.StatusInternalServerError)
return
}
newnode = putNode
}

ctx, cancel := context.WithCancel(i.node.Context())
defer cancel()

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
}

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
}

if len(components) < 1 {
err = fmt.Errorf("Cannot override existing object")
webError(w, "http gateway", err, http.StatusBadRequest)
return
var newPath string
if len(rsegs) > 1 {
newPath = strings.Join(rsegs[2:], "/")
}

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, key.Key(h))
if err != nil {
webError(w, "Could not resolve root object", err, http.StatusBadRequest)
return
}
var newkey key.Key
rnode, err := core.Resolve(ctx, i.node, rootPath)
switch ev := err.(type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a switch from err casting.
This is cleaner, and should be used across the code.

In current master, where cast is used:

$ grep -rE 'err\.\(' . | grep -v Godeps
./routing/dht/ext_test.go:      if merr, ok := err.(u.MultiErr); ok && len(merr) > 0 {
./routing/dht/ext_test.go:      if merr, ok := err.(u.MultiErr); ok && len(merr) > 0 {
./routing/dht/ext_test.go:      if merr, ok := err.(u.MultiErr); ok && len(merr) > 0 {
./core/corehttp/gateway_handler.go: if _, ok := err.(path.ErrNoLink); ok {
./core/corehttp/gateway_handler.go: if _, ok := err.(path.ErrNoLink); ok {
./core/commands/add.go:     if _, ok := err.(*hiddenFileError); ok {
./p2p/net/conn/dial_test.go:            if ne, ok := err.(net.Error); ok && ne.Temporary() {
./p2p/net/conn/dial.go: if nerr, ok := err.(net.Error); ok && nerr.Timeout() {
./p2p/net/conn/dial.go: errno, ok := err.(syscall.Errno)
./cmd/ipfs/main.go: switch e := err.(type) {

case path.ErrNoLink:
// ev.Node < node where resolve failed
// ev.Name < new link
// but we need to patch from the root
rnode, err := i.node.DAG.Get(ctx, key.B58KeyDecode(rsegs[1]))
if err != nil {
webError(w, "putHandler: Could not create DAG from request", err, http.StatusInternalServerError)
return
}

// resolving path components into merkledag nodes. if a component does not
// resolve, create empty directories (which will be linked and populated below.)
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(pathNodes) < len(components) {
pathNodes = append(pathNodes, uio.NewDirectory(i.node.DAG).GetNode())
e := dagutils.NewDagEditor(i.node.DAG, rnode)
err = e.InsertNodeAtPath(ctx, newPath, newnode, uio.NewEmptyDirectory)
if err != nil {
webError(w, "putHandler: InsertNodeAtPath failed", err, http.StatusInternalServerError)
return
}
} else if err != nil {
webError(w, "Could not resolve parent object", err, http.StatusBadRequest)
return
}

for i := len(pathNodes) - 1; i >= 0; i-- {
newnode, err = pathNodes[i].UpdateNodeLink(components[i], newnode)
newkey, err = e.GetNode().Key()
if err != nil {
webError(w, "Could not update node links", err, http.StatusInternalServerError)
webError(w, "putHandler: could not get key of edited node", err, http.StatusInternalServerError)
return
}
}

if err := i.node.DAG.AddRecursive(newnode); err != nil {
webError(w, "Could not add recursively new node", err, http.StatusInternalServerError)
return
}
case nil:
// object set-data case
rnode.Data = newnode.Data

// Redirect to new path
key, err := newnode.Key()
if err != nil {
webError(w, "Could not get key of new node", err, http.StatusInternalServerError)
newkey, err = i.node.DAG.Add(rnode)
if err != nil {
nnk, _ := newnode.Key()
rk, _ := rnode.Key()
webError(w, fmt.Sprintf("putHandler: Could not add newnode(%q) to root(%q)", nnk.B58String(), rk.B58String()), err, http.StatusInternalServerError)
return
}
default:
log.Warningf("putHandler: unhandled resolve error %T", ev)
webError(w, "could not resolve root DAG", ev, http.StatusInternalServerError)
return
}

i.addUserHeaders(w) // ok, _now_ write user's headers.
w.Header().Set("IPFS-Hash", key.String())
http.Redirect(w, r, ipfsPathPrefix+key.String()+"/"+strings.Join(components, "/"), http.StatusCreated)
w.Header().Set("IPFS-Hash", newkey.String())
http.Redirect(w, r, gopath.Join(ipfsPathPrefix, newkey.String(), newPath), http.StatusCreated)
}

func (i *gatewayHandler) deleteHandler(w http.ResponseWriter, r *http.Request) {
Expand Down
8 changes: 4 additions & 4 deletions path/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ var ErrNoComponents = errors.New(

// ErrNoLink is returned when a link is not found in a path
type ErrNoLink struct {
name string
node mh.Multihash
Name string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 to introspectable errors

Node mh.Multihash
}

func (e ErrNoLink) Error() string {
return fmt.Sprintf("no link named %q under %s", e.name, e.node.B58String())
return fmt.Sprintf("no link named %q under %s", e.Name, e.Node.B58String())
}

// Resolver provides path resolution to IPFS
Expand Down Expand Up @@ -124,7 +124,7 @@ func (s *Resolver) ResolveLinks(ctx context.Context, ndd *merkledag.Node, names

if next == "" {
n, _ := nd.Multihash()
return result, ErrNoLink{name: name, node: n}
return result, ErrNoLink{Name: name, Node: n}
}

if nlink.Node == nil {
Expand Down
7 changes: 4 additions & 3 deletions test/sharness/x0111-gateway-writable.sh → test/sharness/t0111-gateway-writeable.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ test_launch_ipfs_daemon

port=$PORT_GWAY

test_expect_success "ipfs daemon listening to TCP port $port" '
test_wait_open_tcp_port_10_sec "$PORT_GWAY"
test_expect_success "ipfs daemon up" '
pollEndpoint -host $ADDR_GWAY -ep=/version -v -tout=1s -tries=60 2>poll_apierr > poll_apiout ||
test_fsh cat poll_apierr || test_fsh cat poll_apiout
'

test_expect_success "HTTP gateway gives access to sample file" '
Expand Down Expand Up @@ -44,7 +45,7 @@ test_expect_success "HTTP PUT empty directory" '
curl -svX PUT "$URL" 2>curl_putEmpty.out &&
cat curl_putEmpty.out &&
grep "Ipfs-Hash: $HASH_EMPTY_DIR" curl_putEmpty.out &&
grep "Location: /ipfs/$HASH_EMPTY_DIR/" curl_putEmpty.out &&
grep "Location: /ipfs/$HASH_EMPTY_DIR" curl_putEmpty.out &&
grep "HTTP/1.1 201 Created" curl_putEmpty.out
'

Expand Down