Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace normalizeURL with urlx.Parse #1217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+8,480 −70
Open

Replace normalizeURL with urlx.Parse #1217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 3 additions & 38 deletions api/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"net"
"net/http"
"net/url"
"strconv"
"strings"
"time"

Expand All @@ -20,6 +19,7 @@ import (
"github.com/cloudflare/cfssl/errors"
"github.com/cloudflare/cfssl/info"
"github.com/cloudflare/cfssl/log"
"github.com/goware/urlx"
)

// A server points to a single remote CFSSL instance.
Expand Down Expand Up @@ -329,43 +329,8 @@ func (ar *AuthRemote) Sign(req []byte) ([]byte, error) {
return ar.AuthSign(req, nil, ar.provider)
}

// normalizeURL checks for http/https protocol, appends "http" as default protocol if not defined in url
// normalizeURL trims spaces and appends "http" as default protocol if not defined in url
func normalizeURL(addr string) (*url.URL, error) {
addr = strings.TrimSpace(addr)

u, err := url.Parse(addr)
if err != nil {
return nil, err
}

if u.Opaque != "" {
u.Host = net.JoinHostPort(u.Scheme, u.Opaque)
u.Opaque = ""
} else if u.Path != "" && !strings.Contains(u.Path, ":") {
u.Host = net.JoinHostPort(u.Path, "8888")
u.Path = ""
} else if u.Scheme == "" {
u.Host = u.Path
u.Path = ""
}

if u.Scheme != "https" {
u.Scheme = "http"
}

_, port, err := net.SplitHostPort(u.Host)
if err != nil {
_, port, err = net.SplitHostPort(u.Host + ":8888")
if err != nil {
return nil, err
}
}

if port != "" {
_, err = strconv.Atoi(port)
if err != nil {
return nil, err
}
}
return u, nil
return urlx.Parse(addr)
}
Comment on lines 333 to 336
Copy link
Contributor

@thaJeztah thaJeztah Nov 22, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a quick glance, this would make the tests pass (without needing the new dependency);

Suggested change
func normalizeURL(addr string) (*url.URL, error) {
addr = strings.TrimSpace(addr)
u, err := url.Parse(addr)
if err != nil {
return nil, err
}
if u.Opaque != "" {
u.Host = net.JoinHostPort(u.Scheme, u.Opaque)
u.Opaque = ""
} else if u.Path != "" && !strings.Contains(u.Path, ":") {
u.Host = net.JoinHostPort(u.Path, "8888")
u.Path = ""
} else if u.Scheme == "" {
u.Host = u.Path
u.Path = ""
}
if u.Scheme != "https" {
u.Scheme = "http"
}
_, port, err := net.SplitHostPort(u.Host)
if err != nil {
_, port, err = net.SplitHostPort(u.Host + ":8888")
if err != nil {
return nil, err
}
}
if port != "" {
_, err = strconv.Atoi(port)
if err != nil {
return nil, err
}
}
return u, nil
return urlx.Parse(addr)
}
// normalizeURL trims spaces and appends "http" as default protocol if not defined in url
func normalizeURL(addr string) (*url.URL, error) {
addr = strings.TrimSpace(addr)
if strings.Index(addr, "://") == -1 {
// use http:// if no scheme is provided
addr = "http://" + addr
}
u, err := url.Parse(addr)
if err != nil {
return nil, err
}
_, port, err := net.SplitHostPort(u.Host)
if err != nil {
var err2 error
_, port, err2 = net.SplitHostPort(u.Host + ":8888")
if err2 != nil {
return nil, err
}
}
if port != "" {
v, err := strconv.Atoi(port)
if err != nil {
return nil, fmt.Errorf("invalid port: %w", err)
}
if v < 0 || v > 65535 {
return nil, fmt.Errorf("invalid port %q", port)
}
}
return u, nil
}

64 changes: 32 additions & 32 deletions api/client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package client

import (
"crypto/tls"
"github.com/cloudflare/cfssl/auth"
"github.com/cloudflare/cfssl/helpers"
"net"
"strings"
"testing"

"github.com/cloudflare/cfssl/auth"
"github.com/cloudflare/cfssl/helpers"
)

var (
Expand All @@ -27,28 +27,28 @@ func TestNewServer(t *testing.T) {

}

_, port, _ := net.SplitHostPort("")
if port != "" {
t.Fatalf("%v", port)

}

s = NewServer("http://127.0.0.1:8888")
hosts := s.Hosts()
if len(hosts) != 1 || hosts[0] != "http://127.0.0.1:8888" {
t.Fatalf("expected [http://127.0.0.1:8888], but have %v", hosts)
}

s = NewServer("http://1.1.1.1:9999")
hosts = s.Hosts()
if len(hosts) != 1 || hosts[0] != "http://1.1.1.1:9999" {
t.Fatalf("expected [http://1.1.1.1:9999], but have %v", hosts)
}

s = NewServer("https://1.1.1.1:8080")
hosts = s.Hosts()
if len(hosts) != 1 || hosts[0] != "https://1.1.1.1:8080" {
t.Fatalf("expected [https://1.1.1.1:8080], but have %v", hosts)
tests := []struct {
url string
want string
}{
{url: "http://127.0.0.1:8888", want: "http://127.0.0.1:8888"},
{url: "http://1.1.1.1:9999", want: "http://1.1.1.1:9999"},
{url: "https://1.1.1.1:8080", want: "https://1.1.1.1:8080"},
{url: "https://1.1.1.1:8181/ ", want: "https://1.1.1.1:8181/"},
{url: "https://1.1.1.1/foo", want: "https://1.1.1.1/foo"},
{url: " some.host/path", want: "http://some.host/path"},
{url: "https://some.host:8888", want: "https://some.host:8888"},
{url: "1.1.1.1:8/foo", want: "http://1.1.1.1:8/foo"},
{url: " 1.1.1.1/", want: "http://1.1.1.1/"},
{url: "1.1.1.1", want: "http://1.1.1.1"},
{url: "ca1.local", want: "http://ca1.local"},
}
for _, tt := range tests {
s := NewServer(tt.url)
hosts := s.Hosts()
if len(hosts) != 1 || hosts[0] != tt.want {
t.Errorf("expected [%s], but got %v", tt.want, hosts)
}
}
}

Expand All @@ -60,7 +60,7 @@ func TestInvalidPort(t *testing.T) {
}

func TestAuthSign(t *testing.T) {
s := NewServer(".X")
s := NewServer("1.1.1.1")
testProvider, _ = auth.New(testKey, nil)
testRequest := []byte(`testing 1 2 3`)
as, err := s.AuthSign(testRequest, testAD, testProvider)
Expand All @@ -71,7 +71,7 @@ func TestAuthSign(t *testing.T) {

func TestDefaultAuthSign(t *testing.T) {
testProvider, _ = auth.New(testKey, nil)
s := NewAuthServer(".X", nil, testProvider)
s := NewAuthServer("1.1.1.1", nil, testProvider)
testRequest := []byte(`testing 1 2 3`)
as, err := s.Sign(testRequest)
if as != nil || err == nil {
Expand All @@ -80,7 +80,7 @@ func TestDefaultAuthSign(t *testing.T) {
}

func TestSign(t *testing.T) {
s := NewServer(".X")
s := NewServer("1.1.1.1")
sign, err := s.Sign([]byte{5, 5, 5, 5})
if sign != nil || err == nil {
t.Fatalf("expected error with sign function")
Expand All @@ -103,7 +103,7 @@ func TestNewMutualTLSServer(t *testing.T) {
}

func TestNewServerGroup(t *testing.T) {
s := NewServer("cfssl1.local:8888, cfssl2.local:8888, http://cfssl3.local:8888, http://cfssl4.local:8888")
s := NewServer("cfssl1.local:8888, cfssl2.local:8888/, http://cfssl3.local:8888, http://cfssl4.local:8888")

ogl, ok := s.(*orderedListGroup)
if !ok {
Expand All @@ -124,8 +124,8 @@ func TestNewServerGroup(t *testing.T) {
hosts[0])
}

if hosts[1] != "http://cfssl2.local:8888" {
t.Fatalf("expected to see http://cfssl2.local:8888, but saw %s",
if hosts[1] != "http://cfssl2.local:8888/" {
t.Fatalf("expected to see http://cfssl2.local:8888/, but saw %s",
hosts[1])
}

Expand Down Expand Up @@ -187,7 +187,7 @@ func TestNewOGLGroup(t *testing.T) {
t.Fatalf("expected StrategyOrderedList (%d) but have %d", StrategyOrderedList, strategy)
}

rem, err := NewGroup([]string{"ca1.local,", "ca2.local"}, nil, strategy)
rem, err := NewGroup([]string{"ca1.local", "ca2.local"}, nil, strategy)
if err != nil {
t.Fatalf("%v", err)
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/certificate-transparency-go v1.1.2-0.20210511102531-373a877eec92
github.com/google/uuid v1.2.0 // indirect
github.com/goware/urlx v0.3.1
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
github.com/jmhodges/clock v0.0.0-20160418191101-880ee4c33548
Expand Down
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ github.com/Masterminds/semver/v3 v3.1.0/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0
github.com/Masterminds/sprig v2.15.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/PuerkitoBio/purell v1.1.1 h1:WEQqlqaGbrPkxLJWfBwQmfEAE1Z7ONdDLqrN38tNFfI=
github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g=
Expand Down Expand Up @@ -368,6 +372,8 @@ github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/goware/urlx v0.3.1 h1:BbvKl8oiXtJAzOzMqAQ0GfIhf96fKeNEZfm9ocNSUBI=
github.com/goware/urlx v0.3.1/go.mod h1:h8uwbJy68o+tQXCGZNa9D73WN8n0r9OBae5bUnLcgjw=
github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-middleware v1.2.2/go.mod h1:EaizFBKfUKtMIF5iaDEhniwNedqGo9FuLFzppDr3uwI=
Expand Down
5 changes: 5 additions & 0 deletions vendor/github.com/PuerkitoBio/purell/.gitignore
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions vendor/github.com/PuerkitoBio/purell/.travis.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions vendor/github.com/PuerkitoBio/purell/LICENSE
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.