Merge pull request #7828 from ipfs/fix/normalize-service-endpoint

fix: normalize remote service endpoint

core/commands/pin/remotepin.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	neturl "net/url"
+	gopath "path"
 
 	"golang.org/x/sync/errgroup"
 
@@ -443,13 +444,11 @@ TIP:
 		}
 
 		name := req.Arguments[0]
-		url := strings.TrimSuffix(req.Arguments[1], "/pins") // fix /pins/pins :-)
-		key := req.Arguments[2]
-
-		u, err := neturl.ParseRequestURI(url)
-		if err != nil || !strings.HasPrefix(u.Scheme, "http") {
-			return fmt.Errorf("service endpoint must be a valid HTTP URL")
+		endpoint, err := normalizeEndpoint(req.Arguments[1])
+		if err != nil {
+			return err
 		}
+		key := req.Arguments[2]
 
 		cfg, err := repo.Config()
 		if err != nil {
@@ -465,7 +464,7 @@ TIP:
 
 		cfg.Pinning.RemoteServices[name] = config.RemotePinningService{
 			Api: config.RemotePinningServiceApi{
-				Endpoint: url,
+				Endpoint: endpoint,
 				Key:      key,
 			},
 		}
@@ -704,14 +703,14 @@ func getRemotePinService(env cmds.Environment, name string) (*pinclient.Client,
 	if name == "" {
 		return nil, fmt.Errorf("remote pinning service name not specified")
 	}
-	url, key, err := getRemotePinServiceInfo(env, name)
+	endpoint, key, err := getRemotePinServiceInfo(env, name)
 	if err != nil {
 		return nil, err
 	}
-	return pinclient.NewClient(url, key), nil
+	return pinclient.NewClient(endpoint, key), nil
 }
 
-func getRemotePinServiceInfo(env cmds.Environment, name string) (url, key string, err error) {
+func getRemotePinServiceInfo(env cmds.Environment, name string) (endpoint, key string, err error) {
 	cfgRoot, err := cmdenv.GetConfigRoot(env)
 	if err != nil {
 		return "", "", err
@@ -732,5 +731,32 @@ func getRemotePinServiceInfo(env cmds.Environment, name string) (url, key string
 	if !present {
 		return "", "", fmt.Errorf("service not known")
 	}
-	return service.Api.Endpoint, service.Api.Key, nil
+	endpoint, err = normalizeEndpoint(service.Api.Endpoint)
+	if err != nil {
+		return "", "", err
+	}
+	return endpoint, service.Api.Key, nil
+}
+
+func normalizeEndpoint(endpoint string) (string, error) {
+	uri, err := neturl.ParseRequestURI(endpoint)
+	if err != nil || !(uri.Scheme == "http" || uri.Scheme == "https") {
+		return "", fmt.Errorf("service endpoint must be a valid HTTP URL")
+	}
+
+	// cleanup trailing and duplicate slashes (https://github.com/ipfs/go-ipfs/issues/7826)
+	uri.Path = gopath.Clean(uri.Path)
+	uri.Path = strings.TrimSuffix(uri.Path, ".")
+	uri.Path = strings.TrimSuffix(uri.Path, "/")
+
+	// remove any query params
+	if uri.RawQuery != "" {
+		return "", fmt.Errorf("service endpoint should be provided without any query parameters")
+	}
+
+	if strings.HasSuffix(uri.Path, "/pins") {
+		return "", fmt.Errorf("service endpoint should be provided without the /pins suffix")
+	}
+
+	return uri.String(), nil
 }

core/commands/pin/remotepin_test.go

@@ -0,0 +1,67 @@
+package pin
+
+import (
+	"testing"
+)
+
+func TestNormalizeEndpoint(t *testing.T) {
+	cases := []struct {
+		in  string
+		err string
+		out string
+	}{
+		{
+			in:  "https://1.example.com",
+			err: "",
+			out: "https://1.example.com",
+		},
+		{
+			in:  "https://2.example.com/",
+			err: "",
+			out: "https://2.example.com",
+		},
+		{
+			in:  "https://3.example.com/pins/",
+			err: "service endpoint should be provided without the /pins suffix",
+			out: "",
+		},
+		{
+			in:  "https://4.example.com/pins",
+			err: "service endpoint should be provided without the /pins suffix",
+			out: "",
+		},
+		{
+			in:  "https://5.example.com/./some//nonsense/../path/../path/",
+			err: "",
+			out: "https://5.example.com/some/path",
+		},
+		{
+			in:  "https://6.example.com/endpoint/?query=val",
+			err: "service endpoint should be provided without any query parameters",
+			out: "",
+		},
+		{
+			in:  "http://192.168.0.5:45000/",
+			err: "",
+			out: "http://192.168.0.5:45000",
+		},
+		{
+			in:  "foo://4.example.com/pins",
+			err: "service endpoint must be a valid HTTP URL",
+			out: "",
+		},
+	}
+
+	for _, tc := range cases {
+		out, err := normalizeEndpoint(tc.in)
+		if err != nil && tc.err != err.Error() {
+			t.Errorf("unexpected error for %q: expected %q; got %q", tc.in, tc.err, err)
+			continue
+		}
+		if out != tc.out {
+			t.Errorf("unexpected endpoint for %q: expected %q; got %q", tc.in, tc.out, out)
+			continue
+		}
+	}
+
+}

test/sharness/t0700-remotepin.sh

@@ -37,6 +37,12 @@ test_expect_success "creating test user on remote pinning service" '
   ipfs pin remote service add test_invalid_url_dns_svc https://invalid-service.example.com fake_api_key
 '
 
+# add a service with a invalid endpoint
+test_expect_success "adding remote service with invalid endpoint" '
+  test_expect_code 1 ipfs pin remote service add test_endpoint_no_protocol invalid-service.example.com fake_api_key &&
+  test_expect_code 1 ipfs pin remote service add test_endpoint_bad_protocol xyz://invalid-service.example.com fake_api_key
+'
+
 test_expect_success "test 'ipfs pin remote service ls'" '
   ipfs pin remote service ls | tee ls_out &&
   grep -q test_pin_svc ls_out &&