Merge pull request #2032 from ipfs/fix/close-notify

fix close notify

commands/cli/parse.go

@@ -49,8 +49,10 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c
 	}
 	req.SetArguments(stringArgs)
 
+	if len(fileArgs) > 0 {
 		file := files.NewSliceFile("", "", fileArgs)
 		req.SetFiles(file)
+	}
 
 	err = cmd.CheckArguments(req)
 	if err != nil {

commands/http/client.go

@@ -23,6 +23,10 @@ const (
 	ApiPath      = "/api/v0" // TODO: make configurable
 )
 
+var OptionSkipMap = map[string]bool{
+	"api": true,
+}
+
 // Client is the commands HTTP client interface.
 type Client interface {
 	Send(req cmds.Request) (cmds.Response, error)
@@ -79,10 +83,6 @@ func (c *client) Send(req cmds.Request) (cmds.Response, error) {
 	if req.Files() != nil {
 		fileReader = NewMultiFileReader(req.Files(), true)
 		reader = fileReader
-	} else {
-		// if we have no file data, use an empty Reader
-		// (http.NewRequest panics when a nil Reader is used)
-		reader = strings.NewReader("")
 	}
 
 	path := strings.Join(req.Path(), "/")
@@ -147,6 +147,9 @@ func (c *client) Send(req cmds.Request) (cmds.Response, error) {
 func getQuery(req cmds.Request) (string, error) {
 	query := url.Values{}
 	for k, v := range req.Options() {
+		if OptionSkipMap[k] {
+			continue
+		}
 		str := fmt.Sprintf("%v", v)
 		query.Set(k, str)
 	}

commands/http/handler.go

@@ -6,8 +6,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"os"
-	"runtime"
+	"runtime/debug"
 	"strconv"
 	"strings"
 	"sync"
@@ -92,7 +91,7 @@ func skipAPIHeader(h string) bool {
 	}
 }
 
-func NewHandler(ctx cmds.Context, root *cmds.Command, cfg *ServerConfig) *Handler {
+func NewHandler(ctx cmds.Context, root *cmds.Command, cfg *ServerConfig) http.Handler {
 	if cfg == nil {
 		panic("must provide a valid ServerConfig")
 	}
@@ -114,14 +113,33 @@ func (i internalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	defer func() {
 		if r := recover(); r != nil {
+			log.Error("A panic has occurred in the commands handler!")
 			log.Error(r)
 
-			buf := make([]byte, 4096)
-			n := runtime.Stack(buf, false)
-			fmt.Fprintln(os.Stderr, string(buf[:n]))
+			debug.PrintStack()
 		}
 	}()
 
+	// get the node's context to pass into the commands.
+	node, err := i.ctx.GetNode()
+	if err != nil {
+		s := fmt.Sprintf("cmds/http: couldn't GetNode(): %s", err)
+		http.Error(w, s, http.StatusInternalServerError)
+		return
+	}
+
+	ctx, cancel := context.WithCancel(node.Context())
+	defer cancel()
+	if cn, ok := w.(http.CloseNotifier); ok {
+		go func() {
+			select {
+			case <-cn.CloseNotify():
+			case <-ctx.Done():
+			}
+			cancel()
+		}()
+	}
+
 	if !allowOrigin(r, i.cfg) || !allowReferer(r, i.cfg) {
 		w.WriteHeader(http.StatusForbidden)
 		w.Write([]byte("403 - Forbidden"))
@@ -140,29 +158,9 @@ func (i internalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// get the node's context to pass into the commands.
-	node, err := i.ctx.GetNode()
-	if err != nil {
-		s := fmt.Sprintf("cmds/http: couldn't GetNode(): %s", err)
-		http.Error(w, s, http.StatusInternalServerError)
-		return
-	}
-
 	//ps: take note of the name clash - commands.Context != context.Context
 	req.SetInvocContext(i.ctx)
 
-	ctx, cancel := context.WithCancel(node.Context())
-	defer cancel()
-	if cn, ok := w.(http.CloseNotifier); ok {
-		go func() {
-			select {
-			case <-cn.CloseNotify():
-			case <-ctx.Done():
-			}
-			cancel()
-		}()
-	}
-
 	err = req.SetRootContext(ctx)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)

core/corehttp/gateway_handler.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/http"
 	gopath "path"
+	"runtime/debug"
 	"strings"
 	"time"
 
@@ -55,6 +56,14 @@ func (i *gatewayHandler) newDagFromReader(r io.Reader) (*dag.Node, error) {
 
 // TODO(btc): break this apart into separate handlers using a more expressive muxer
 func (i *gatewayHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	defer func() {
+		if r := recover(); r != nil {
+			log.Error("A panic occurred in the gateway handler!")
+			log.Error(r)
+			debug.PrintStack()
+		}
+	}()
+
 	if i.config.Writable {
 		switch r.Method {
 		case "POST":

test/sharness/t0235-cli-request.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Jeromy Johnson
+# MIT Licensed; see the LICENSE file in this repository.
+#
+
+test_description="test http requests made by cli"
+
+. lib/test-lib.sh
+
+test_init_ipfs
+test_launch_ipfs_daemon
+
+test_expect_success "can make http request against nc server" '
+	go-sleep 0.5s | nc -l 5005 > nc_out &
+	ipfs cat /ipfs/Qmabcdef --api /ip4/127.0.0.1/tcp/5005 || true
+'
+
+test_expect_success "output does not contain multipart info" '
+	test_expect_code 1 grep multipart nc_out
+'
+
+test_expect_success "request looks good" '
+	grep "POST /api/v0/cat" nc_out
+'
+
+test_expect_success "api flag does not appear in request" '
+	test_expect_code 1 grep "api=/ip4" nc_out
+'
+
+test_kill_ipfs_daemon
+
+test_done