Skip to content

Commit

Permalink
Exit early on invalid URLs
Browse files Browse the repository at this point in the history
When creating a new client, invalid URLs are skipped and the problem is
only seen when e.g. the health checks set in. This commit changes the
code so that invalid URLs are found early in the cycle, when setting
them with `SetURL`.

Close olivere#1339
  • Loading branch information
olivere committed Jun 6, 2020
1 parent 7925644 commit b0c9e6a
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
13 changes: 9 additions & 4 deletions canonicalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

package elastic

import "net/url"
import (
"net/url"
"strings"
)

// canonicalize takes a list of URLs and returns its canonicalized form, i.e.
// remove anything but scheme, userinfo, host, path, and port.
Expand All @@ -14,15 +17,17 @@ import "net/url"
// Example:
// http://127.0.0.1:9200/?query=1 -> http://127.0.0.1:9200
// http://127.0.0.1:9200/db1/ -> http://127.0.0.1:9200/db1
// http://127.0.0.1:9200/db1/// -> http://127.0.0.1:9200/db1
func canonicalize(rawurls ...string) []string {
var canonicalized []string
for _, rawurl := range rawurls {
u, err := url.Parse(rawurl)
if err == nil {
if u.Scheme == "http" || u.Scheme == "https" {
// Trim trailing slashes
for len(u.Path) > 0 && u.Path[len(u.Path)-1] == '/' {
u.Path = u.Path[0 : len(u.Path)-1]
// Trim trailing slashes. Notice that strings.TrimSuffix will only remove the last slash,
// not all slashes from the suffix, so we'll loop over the path to remove all slashes.
for strings.HasSuffix(u.Path, "/") {
u.Path = u.Path[:len(u.Path)-1]
}
u.Fragment = ""
u.RawQuery = ""
Expand Down
6 changes: 6 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,12 @@ func SetURL(urls ...string) ClientOptionFunc {
default:
c.urls = urls
}
// Check URLs
for _, urlStr := range c.urls {
if _, err := url.Parse(urlStr); err != nil {
return err
}
}
return nil
}
}
Expand Down
13 changes: 13 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,19 @@ func TestClientWithMultipleURLs(t *testing.T) {
}
}

func TestClientWithInvalidURLs(t *testing.T) {
client, err := NewClient(SetURL(" http://foo.com", "http://[fe80::%31%25en0]:8080/"))
if err == nil {
t.Fatal("expected error, got nil")
}
if want, have := `parse " http://foo.com": first path segment in URL cannot contain colon`, err.Error(); want != have {
t.Fatalf("expected error %q, have %q", want, have)
}
if client != nil {
t.Fatal("expected client == nil")
}
}

func TestClientWithBasicAuth(t *testing.T) {
client, err := NewClient(SetBasicAuth("user", "secret"))
if err != nil {
Expand Down

0 comments on commit b0c9e6a

Please sign in to comment.