From 5f692a76de01918b202aab1bc5d6657bb177a0ea Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Mon, 14 Dec 2020 19:56:44 +0100 Subject: [PATCH 1/6] fix: normalize remote service endpoint Closes #7826 --- core/commands/pin/remotepin.go | 29 ++++++++++++--- core/commands/pin/remotepin_test.go | 57 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 core/commands/pin/remotepin_test.go diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 9d1e3d60b..7b3de85e2 100644 --- a/core/commands/pin/remotepin.go +++ b/core/commands/pin/remotepin.go @@ -443,12 +443,12 @@ TIP: } name := req.Arguments[0] - url := strings.TrimSuffix(req.Arguments[1], "/pins") // fix /pins/pins :-) + url := req.Arguments[1] 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(url) + if err != nil { + return err } cfg, err := repo.Config() @@ -465,7 +465,7 @@ TIP: cfg.Pinning.RemoteServices[name] = config.RemotePinningService{ Api: config.RemotePinningServiceApi{ - Endpoint: url, + Endpoint: endpoint, Key: key, }, } @@ -708,7 +708,11 @@ func getRemotePinService(env cmds.Environment, name string) (*pinclient.Client, if err != nil { return nil, err } - return pinclient.NewClient(url, key), nil + endpoint, err := normalizeEndpoint(url) + if err != nil { + return nil, err + } + return pinclient.NewClient(endpoint, key), nil } func getRemotePinServiceInfo(env cmds.Environment, name string) (url, key string, err error) { @@ -734,3 +738,16 @@ func getRemotePinServiceInfo(env cmds.Environment, name string) (url, key string } return service.Api.Endpoint, service.Api.Key, nil } + +func normalizeEndpoint(endpoint string) (string, error) { + uri, err := neturl.ParseRequestURI(endpoint) + if err != nil || !strings.HasPrefix(uri.Scheme, "http") { + return "", fmt.Errorf("service endpoint must be a valid HTTP URL") + } + // avoid //pins (https://github.com/ipfs/go-ipfs/issues/7826) + uri.Path = strings.TrimSuffix(uri.Path, "/") + // avoid /pins/pins + uri.Path = strings.TrimSuffix(uri.Path, "/pins") + + 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 000000000..26265da6f --- /dev/null +++ b/core/commands/pin/remotepin_test.go @@ -0,0 +1,57 @@ +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: "", + out: "https://3.example.com", + }, + { + in: "https://4.example.com/pins", + err: "", + out: "https://4.example.com", + }, + { + in: "http://192.168.0.5:45000/pins", + 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 out != tc.out { + t.Errorf("unexpected endpoint for %q: expected %q; got %q", tc.in, tc.out, out) + continue + } + if err != nil && tc.err != err.Error() { + t.Errorf("unexpected error for %q: expected %q; got %q", tc.in, tc.err, err) + continue + } + } + +} From 6851ede8eb2878a967f8052c4b01b6d1fb946db2 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 15 Dec 2020 21:58:36 +0100 Subject: [PATCH 2/6] refactor: normalization with path.Clean --- core/commands/pin/remotepin.go | 17 ++++++++++++++--- core/commands/pin/remotepin_test.go | 28 +++++++++++++++++++--------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 7b3de85e2..b29860a3f 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" @@ -744,10 +745,20 @@ func normalizeEndpoint(endpoint string) (string, error) { if err != nil || !strings.HasPrefix(uri.Scheme, "http") { return "", fmt.Errorf("service endpoint must be a valid HTTP URL") } - // avoid //pins (https://github.com/ipfs/go-ipfs/issues/7826) + + // 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, "/") - // avoid /pins/pins - uri.Path = strings.TrimSuffix(uri.Path, "/pins") + + // remove any query params + if uri.RawQuery != "" || uri.RawFragment != "" { + 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 index 26265da6f..d1dfa8fd5 100644 --- a/core/commands/pin/remotepin_test.go +++ b/core/commands/pin/remotepin_test.go @@ -22,16 +22,26 @@ func TestNormalizeEndpoint(t *testing.T) { }, { in: "https://3.example.com/pins/", - err: "", - out: "https://3.example.com", + err: "service endpoint should be provided without the /pins suffix", + out: "", }, { in: "https://4.example.com/pins", - err: "", - out: "https://4.example.com", + err: "service endpoint should be provided without the /pins suffix", + out: "", }, { - in: "http://192.168.0.5:45000/pins", + 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", }, @@ -44,14 +54,14 @@ func TestNormalizeEndpoint(t *testing.T) { for _, tc := range cases { out, err := normalizeEndpoint(tc.in) - if out != tc.out { - t.Errorf("unexpected endpoint for %q: expected %q; got %q", tc.in, tc.out, out) - continue - } 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 + } } } From 7313a45bc7e58c8880bc0350acc5a5d909474030 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 15 Dec 2020 22:22:24 +0100 Subject: [PATCH 3/6] fix: url.RawFragment does not work in go1.14 I don't think we need to worry about #hash, so just removed it --- core/commands/pin/remotepin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index b29860a3f..5abbc7fb6 100644 --- a/core/commands/pin/remotepin.go +++ b/core/commands/pin/remotepin.go @@ -752,7 +752,7 @@ func normalizeEndpoint(endpoint string) (string, error) { uri.Path = strings.TrimSuffix(uri.Path, "/") // remove any query params - if uri.RawQuery != "" || uri.RawFragment != "" { + if uri.RawQuery != "" { return "", fmt.Errorf("service endpoint should be provided without any query parameters") } From fc4095cfb3f8c2c958a9bcd583698070ed1f38d5 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 15 Dec 2020 23:24:48 +0100 Subject: [PATCH 4/6] =?UTF-8?q?refactor:=20url=20=E2=86=92=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/ipfs/go-ipfs/pull/7828#discussion_r543520089 --- core/commands/pin/remotepin.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 5abbc7fb6..80572bfb8 100644 --- a/core/commands/pin/remotepin.go +++ b/core/commands/pin/remotepin.go @@ -444,13 +444,11 @@ TIP: } name := req.Arguments[0] - url := req.Arguments[1] - key := req.Arguments[2] - - endpoint, err := normalizeEndpoint(url) + endpoint, err := normalizeEndpoint(req.Arguments[1]) if err != nil { return err } + key := req.Arguments[2] cfg, err := repo.Config() if err != nil { @@ -705,18 +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) - if err != nil { - return nil, err - } - endpoint, err := normalizeEndpoint(url) + endpoint, key, err := getRemotePinServiceInfo(env, name) if err != nil { return nil, err } 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 @@ -737,7 +731,11 @@ 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) { From e0d484431adc292e1aa63aeb36cd2298486c743f Mon Sep 17 00:00:00 2001 From: Adin Schmahmann Date: Wed, 27 Jan 2021 14:07:04 -0500 Subject: [PATCH 5/6] test: add sharness tests for remote pinning service endpoints that are not http(s) URIs --- test/sharness/t0700-remotepin.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/sharness/t0700-remotepin.sh b/test/sharness/t0700-remotepin.sh index b03d4177d..cac374193 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 && From 61f3a2f3a2e62b47f058029f77e95f9a11b38b4c Mon Sep 17 00:00:00 2001 From: Adin Schmahmann Date: Wed, 27 Jan 2021 14:09:29 -0500 Subject: [PATCH 6/6] fix: restrict remote pinning service endpoint URIs to HTTP/HTTPS --- core/commands/pin/remotepin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 80572bfb8..606b23f6d 100644 --- a/core/commands/pin/remotepin.go +++ b/core/commands/pin/remotepin.go @@ -740,7 +740,7 @@ func getRemotePinServiceInfo(env cmds.Environment, name string) (endpoint, key s func normalizeEndpoint(endpoint string) (string, error) { uri, err := neturl.ParseRequestURI(endpoint) - if err != nil || !strings.HasPrefix(uri.Scheme, "http") { + if err != nil || !(uri.Scheme == "http" || uri.Scheme == "https") { return "", fmt.Errorf("service endpoint must be a valid HTTP URL") }