From 426496ab8d321d90794f4d854110b4e347b47b95 Mon Sep 17 00:00:00 2001 From: Salman Aljammaz Date: Tue, 19 May 2015 14:44:30 +0300 Subject: [PATCH] pkg/client: set default port to 443 when using self-verified tls. Fixes #616 Change-Id: Id6c8771d06aa7ccb024459cabd9afb4433a3d011 --- pkg/client/client.go | 35 ++++++++++++++++++++----- pkg/client/client_test.go | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 pkg/client/client_test.go diff --git a/pkg/client/client.go b/pkg/client/client.go index f7c602764..94fb74aea 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -874,15 +874,36 @@ func (c *Client) selfVerifiedSSL() bool { return c.useTLS() && len(c.getTrustedCerts()) > 0 } -// condRewriteURL changes "https://" to "http://" if we are in -// selfVerifiedSSL mode. We need to do that because we do the TLS -// dialing ourselves, and we do not want the http transport layer -// to redo it. -func (c *Client) condRewriteURL(url string) string { +// condRewriteURL changes "https://" to "http://" and adds ":443" to +// the host (if no port was specified) when we are in selfVerifiedSSL +// mode. We need to do that because we do the TLS dialing ourselves, +// and we do not want the http transport layer to redo it. +func (c *Client) condRewriteURL(urlStr string) string { if c.selfVerifiedSSL() || c.insecureTLS() { - return strings.Replace(url, "https://", "http://", 1) + // url.Parse fails for mismached IPv6 brackets on Go 1.5, but + // not 1.4. See https://github.com/golang/go/issues/6530. + // SplitHostPort below always fails on mismatched IPv6 brackets, + // so overall we get the same behaviour on both 1.4 & 1.5. + u, err := url.Parse(urlStr) + if err != nil { + return urlStr + } + if u.Scheme == "https" { + // Keep the port 443 if no explicit port was specified. + _, _, err := net.SplitHostPort(u.Host) + if err == nil { + u.Scheme = "http" + return u.String() + } + addrerr, ok := err.(*net.AddrError) + if ok && addrerr.Err == "missing port in address" { + u.Scheme = "http" + u.Host += ":443" + return u.String() + } + } } - return url + return urlStr } // TLSConfig returns the correct tls.Config depending on whether diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go new file mode 100644 index 000000000..1967f09f9 --- /dev/null +++ b/pkg/client/client_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2015 The Camlistore Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client + +import "testing" + +var rewritetests = []struct { + in string + out string +}{ + // Valid URLs change the scheme, and add :433 iff there's no port. + {"https://foo.bar:443", "http://foo.bar:443"}, + {"https://foo.bar", "http://foo.bar:443"}, + {"https://foo.bar/", "http://foo.bar:443/"}, + {"https://foo.bar:443/", "http://foo.bar:443/"}, + {"https://foo.bar:/", "http://foo.bar:/"}, + {"https://foo.bar:baz/", "http://foo.bar:baz/"}, + {"https://[::0]/", "http://[::0]:443/"}, + {"https://[::0]:82/", "http://[::0]:82/"}, + {"https://[2001:DB8::1]:80/", "http://[2001:DB8::1]:80/"}, + {"https://[2001:DB8::1]:/", "http://[2001:DB8::1]:/"}, + {"https://[2001:DB8:0:1]/", "http://[2001:DB8:0:1]:443/"}, + {"https://192.0.2.3/", "http://192.0.2.3:443/"}, + {"https://192.0.2.3:60/", "http://192.0.2.3:60/"}, + // Invalid URLs stay exactly the same. + {"https://[2001:DB8::1:/", "https://[2001:DB8::1:/"}, + {"https://foo.bar:443:baz/", "https://foo.bar:443:baz/"}, +} + +func TestCondRewriteURL(t *testing.T) { + c := &Client{InsecureTLS: true} + c.initTrustedCertsOnce.Do(c.initTrustedCerts) // Initialise an empty list of trusted certs. + c.server = "https://example.com/" + for _, tt := range rewritetests { + s := c.condRewriteURL(tt.in) + if s != tt.out { + t.Errorf("c.condRewriteURL(%q) => %q, want %q", tt.in, s, tt.out) + } + } +}