Skip to content

Commit

Permalink
sysregistriesv2: allow IP addresses
Browse files Browse the repository at this point in the history
Allow IP addresses to be specified in registries configuration.  Prior
to this change, an IP addresses caused a parsing error in net/url due to
the absence of URI schemes.

Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Nov 29, 2018
1 parent fa79e05 commit 7854e94
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 44 deletions.
45 changes: 3 additions & 42 deletions pkg/sysregistriesv2/system_registries_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sysregistriesv2
import (
"fmt"
"io/ioutil"
"net/url"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -83,58 +82,20 @@ func (e *InvalidRegistries) Error() string {
}

// parseURL parses the input string, performs some sanity checks and returns
// the sanitized input string. An error is returned in case parsing fails or
// or if URI scheme or user is set.
// the sanitized input string. An error is returned if the input string is
// empty or if contains an "http{s,}://" prefix.
func parseURL(input string) (string, error) {
trimmed := strings.TrimRight(input, "/")

if trimmed == "" {
return "", &InvalidRegistries{s: "invalid URL: cannot be empty"}
}

// Ultimately, we expect input of the form example.com[/namespace/…], a prefix
// of a fully-expended reference (containers/image/docker/Reference.String()).
// c/image/docker/Reference does not currently provide such a parser.
// So, we use url.Parse("http://"+trimmed) below to ~verify the format, possibly
// letting some invalid input in, trading that off for a simpler parser.
//
// url.Parse("http://"+trimmed) is, sadly, too permissive, notably for
// trimmed == "http://example.com/…", url.Parse("http://http://example.com/…")
// is accepted and parsed as
// {Scheme: "http", Host: "http:", Path: "//example.com/…"}.
//
// So, first we do an explicit check for an unwanted scheme prefix:

// This will parse trimmed=="http://example.com/…" with Scheme: "http". Perhaps surprisingly,
// it also succeeds for the input we want to accept, in different ways:
// "example.com" -> {Scheme:"", Opaque:"", Path:"example.com"}
// "example.com/repo" -> {Scheme:"", Opaque:"", Path:"example.com/repo"}
// "example.com:5000" -> {Scheme:"example.com", Opaque:"5000"}
// "example.com:5000/repo" -> {Scheme:"example.com", Opaque:"5000/repo"}
uri, err := url.Parse(trimmed)
if err != nil {
return "", &InvalidRegistries{s: fmt.Sprintf("invalid URL '%s': %v", input, err)}
}

// Check if a URI Scheme is set.
// Note that URLs that do not start with a slash after the scheme are
// interpreted as `scheme:opaque[?query][#fragment]`; see above for examples.
if uri.Scheme != "" && uri.Opaque == "" {
if strings.HasPrefix(trimmed, "http://") || strings.HasPrefix(trimmed, "https://") {
msg := fmt.Sprintf("invalid URL '%s': URI schemes are not supported", input)
return "", &InvalidRegistries{s: msg}
}

uri, err = url.Parse("http://" + trimmed)
if err != nil {
msg := fmt.Sprintf("invalid URL '%s': sanitized URL did not parse: %v", input, err)
return "", &InvalidRegistries{s: msg}
}

if uri.User != nil {
msg := fmt.Sprintf("invalid URL '%s': user/password are not supported", trimmed)
return "", &InvalidRegistries{s: msg}
}

return trimmed, nil
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/sysregistriesv2/system_registries_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ func TestParseURL(t *testing.T) {
assert.Contains(t, err.Error(), "invalid URL 'https://example.com': URI schemes are not supported")

_, err = parseURL("[email protected]")
assert.NotNil(t, err)
assert.Contains(t, err.Error(), "invalid URL '[email protected]': user/password are not supported")
assert.Nil(t, err)

// valid URLs
url, err = parseURL("example.com")
Expand Down

0 comments on commit 7854e94

Please sign in to comment.