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)
 
- file := files.NewSliceFile("", "", fileArgs)
- req.SetFiles(file)
+ 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