diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 9d1e3d60bfa..606b23f6d55 100644 --- a/core/commands/pin/remotepin.go +++ b/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 } diff --git a/core/commands/pin/remotepin_test.go b/core/commands/pin/remotepin_test.go new file mode 100644 index 00000000000..d1dfa8fd5e4 --- /dev/null +++ b/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 + } + } + +} diff --git a/test/sharness/t0700-remotepin.sh b/test/sharness/t0700-remotepin.sh index b03d4177d5b..cac374193ad 100755 --- a/test/sharness/t0700-remotepin.sh +++ b/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 &&