Merge pull request #3219 from ipfs/fix/api-file-bad

cmd: validate repo/api file and print nicer error message

cmd/ipfs/daemon.go

@@ -471,7 +471,7 @@ func serveHTTPApi(req cmds.Request) (error, <-chan error) {
 		return fmt.Errorf("serveHTTPApi: ConstructNode() failed: %s", err), nil
 	}
 
-	if err := node.Repo.SetAPIAddr(apiMaddr.String()); err != nil {
+	if err := node.Repo.SetAPIAddr(apiMaddr); err != nil {
 		return fmt.Errorf("serveHTTPApi: SetAPIAddr() failed: %s", err), nil
 	}
 

cmd/ipfs/main.go

@@ -2,6 +2,7 @@
 package main
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -16,13 +17,6 @@ import (
 	"syscall"
 	"time"
 
-	manet "gx/ipfs/QmT6Cp31887FpAc25z25YHgpFJohZedrYLWPPspRtj1Brp/go-multiaddr-net"
-	ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
-
-	context "context"
-	logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
-	u "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"
-
 	cmds "github.com/ipfs/go-ipfs/commands"
 	cmdsCli "github.com/ipfs/go-ipfs/commands/cli"
 	cmdsHttp "github.com/ipfs/go-ipfs/commands/http"
@@ -31,7 +25,13 @@ import (
 	repo "github.com/ipfs/go-ipfs/repo"
 	config "github.com/ipfs/go-ipfs/repo/config"
 	fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo"
+
+	logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
+	manet "gx/ipfs/QmT6Cp31887FpAc25z25YHgpFJohZedrYLWPPspRtj1Brp/go-multiaddr-net"
 	loggables "gx/ipfs/QmTMy4hVSY28DdwJ9kBz6y7q6MuioFzPcpM3Ma3aPjo1i3/go-libp2p-loggables"
+	ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
+	osh "gx/ipfs/QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93/go-os-helper"
+	u "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"
 )
 
 // log is the command logger
@@ -590,23 +590,51 @@ func profileIfEnabled() (func(), error) {
 	return func() {}, nil
 }
 
+var apiFileErrorFmt string = `Failed to parse '%[1]s/api' file.
+	error: %[2]s
+If you're sure go-ipfs isn't running, you can just delete it.
+Otherwise check:
+`
+var checkIPFSUnixFmt = "\tps aux | grep ipfs"
+var checkIPFSWinFmt = "\ttasklist | findstr ipfs"
+
 // getApiClient checks the repo, and the given options, checking for
 // a running API service. if there is one, it returns a client.
 // otherwise, it returns errApiNotRunning, or another error.
 func getApiClient(repoPath, apiAddrStr string) (cmdsHttp.Client, error) {
+	var apiErrorFmt string
+	switch {
+	case osh.IsUnix():
+		apiErrorFmt = apiFileErrorFmt + checkIPFSUnixFmt
+	case osh.IsWindows():
+		apiErrorFmt = apiFileErrorFmt + checkIPFSWinFmt
+	default:
+		apiErrorFmt = apiFileErrorFmt
+	}
 
-	if apiAddrStr == "" {
-		var err error
-		if apiAddrStr, err = fsrepo.APIAddr(repoPath); err != nil {
+	var addr ma.Multiaddr
+	var err error
+	if len(apiAddrStr) != 0 {
+		addr, err = ma.NewMultiaddr(apiAddrStr)
+		if err != nil {
+			return nil, err
+		}
+		if len(addr.Protocols()) == 0 {
+			return nil, fmt.Errorf("mulitaddr doesn't provide any protocols")
+		}
+	} else {
+		addr, err = fsrepo.APIAddr(repoPath)
+		if err == repo.ErrApiNotRunning {
 			return nil, err
 		}
-	}
 
-	addr, err := ma.NewMultiaddr(apiAddrStr)
-	if err != nil {
-		return nil, err
+		if err != nil {
+			return nil, fmt.Errorf(apiErrorFmt, repoPath, err.Error())
+		}
+	}
+	if len(addr.Protocols()) == 0 {
+		return nil, fmt.Errorf(apiErrorFmt, repoPath, "multiaddr doesn't provide any protocols")
 	}
-
 	return apiClientForAddr(addr)
 }
 

package.json

@@ -287,6 +287,12 @@
       "hash": "QmQfeKxQtBN721pekQh6Jq24adFUjnU65YdY3GNczfuG2T",
       "name": "dir-index-html",
       "version": "1.0.3"
+    },
+    {
+      "author": "Kubuxu",
+      "hash": "QmXuBJ7DR6k3rmUEKtvVMhwjmXDuJgXXPUt4LQXKBMsU93",
+      "name": "go-os-helper",
+      "version": "0.0.0"
     }
   ],
   "gxVersion": "0.4.0",
@@ -295,4 +301,3 @@
   "name": "go-ipfs",
   "version": "0.4.5-dev"
 }
-

repo/fsrepo/fsrepo.go

@@ -18,7 +18,9 @@ import (
 	mfsr "github.com/ipfs/go-ipfs/repo/fsrepo/migrations"
 	serialize "github.com/ipfs/go-ipfs/repo/fsrepo/serialize"
 	dir "github.com/ipfs/go-ipfs/thirdparty/dir"
+
 	logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
+	ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
 	util "gx/ipfs/Qmb912gdngC1UWwTkhuW8knyRbcWeu5kqkxBpveLmW8bSr/go-ipfs-util"
 	"gx/ipfs/QmeqtHtxGfcsfXiou7wqHJARWPKUTUcPdtSfSYYHp48dtQ/go-ds-measure"
 )
@@ -274,7 +276,7 @@ func LockedByOtherProcess(repoPath string) (bool, error) {
 // in the fsrepo. This is a concurrent operation, meaning that any
 // process may read this file. modifying this file, therefore, should
 // use "mv" to replace the whole file and avoid interleaved read/writes.
-func APIAddr(repoPath string) (string, error) {
+func APIAddr(repoPath string) (ma.Multiaddr, error) {
 	repoPath = filepath.Clean(repoPath)
 	apiFilePath := filepath.Join(repoPath, apiFile)
 
@@ -282,9 +284,9 @@ func APIAddr(repoPath string) (string, error) {
 	f, err := os.Open(apiFilePath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			return "", repo.ErrApiNotRunning
+			return nil, repo.ErrApiNotRunning
 		}
-		return "", err
+		return nil, err
 	}
 	defer f.Close()
 
@@ -293,23 +295,23 @@ func APIAddr(repoPath string) (string, error) {
 	buf := make([]byte, 2048)
 	n, err := f.Read(buf)
 	if err != nil && err != io.EOF {
-		return "", err
+		return nil, err
 	}
 
 	s := string(buf[:n])
 	s = strings.TrimSpace(s)
-	return s, nil
+	return ma.NewMultiaddr(s)
 }
 
 // SetAPIAddr writes the API Addr to the /api file.
-func (r *FSRepo) SetAPIAddr(addr string) error {
+func (r *FSRepo) SetAPIAddr(addr ma.Multiaddr) error {
 	f, err := os.Create(filepath.Join(r.path, apiFile))
 	if err != nil {
 		return err
 	}
 	defer f.Close()
 
-	_, err = f.WriteString(addr)
+	_, err = f.WriteString(addr.String())
 	return err
 }
 

repo/mock.go

@@ -4,6 +4,8 @@ import (
 	"errors"
 
 	"github.com/ipfs/go-ipfs/repo/config"
+
+	ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
 )
 
 var errTODO = errors.New("TODO: mock repo")
@@ -37,4 +39,4 @@ func (m *Mock) GetStorageUsage() (uint64, error) { return 0, nil }
 
 func (m *Mock) Close() error { return errTODO }
 
-func (m *Mock) SetAPIAddr(addr string) error { return errTODO }
+func (m *Mock) SetAPIAddr(addr ma.Multiaddr) error { return errTODO }

repo/repo.go

@@ -5,6 +5,8 @@ import (
 	"io"
 
 	config "github.com/ipfs/go-ipfs/repo/config"
+
+	ma "gx/ipfs/QmUAQaWbKxGCUTuoQVvvicbQNZ9APF5pDGWyAZSe93AtKH/go-multiaddr"
 	ds "gx/ipfs/QmbzuUusHqaLLoNTDEVLcSF6vZDHZDLPC7p4bztRvvkXxU/go-datastore"
 )
 
@@ -23,7 +25,7 @@ type Repo interface {
 	GetStorageUsage() (uint64, error)
 
 	// SetAPIAddr sets the API address in the repo.
-	SetAPIAddr(addr string) error
+	SetAPIAddr(addr ma.Multiaddr) error
 
 	io.Closer
 }