fix parsing of and tests for s3 url

sda/cmd/s3inbox/healthchecks.go

@@ -2,7 +2,9 @@ package main
 
 import (
 	"fmt"
+	"net"
 	"net/http"
+	"net/url"
 	"path"
 	"strconv"
 
@@ -54,7 +56,14 @@ func (p *Proxy) CheckHealth(w http.ResponseWriter, _ *http.Request) {
 	}
 
 	// Check that s3 backend responds
-	err = p.httpsGetCheck(p.getS3ReadyPath())
+	s3url, err := p.getS3ReadyPath()
+	if err != nil {
+		log.Errorf("Incorrect S3 health url: %v", err)
+		w.WriteHeader(http.StatusServiceUnavailable)
+
+		return
+	}
+	err = p.httpsGetCheck(s3url)
 	if err != nil {
 		log.Error(err)
 		w.WriteHeader(http.StatusServiceUnavailable)
@@ -78,14 +87,18 @@ func (p *Proxy) httpsGetCheck(url string) error {
 	return nil
 }
 
-func (p *Proxy) getS3ReadyPath() string {
-	s3URL := p.s3.URL
+func (p *Proxy) getS3ReadyPath() (string, error) {
+
+	s3URL, err := url.Parse(p.s3.URL)
+	if err != nil {
+		return "", err
+	}
 	if p.s3.Port != 0 {
-		s3URL = path.Join(s3URL, strconv.Itoa(p.s3.Port))
+		s3URL.Host = net.JoinHostPort(s3URL.Hostname(), strconv.Itoa(p.s3.Port))
 	}
 	if p.s3.Readypath != "" {
-		s3URL += path.Join(s3URL, p.s3.Readypath)
+		s3URL.Path = path.Join(s3URL.Path, p.s3.Readypath)
 	}
 
-	return s3URL
+	return s3URL.String(), nil
 }

sda/cmd/s3inbox/healthchecks_test.go

@@ -35,11 +35,23 @@ func (suite *HealthcheckTestSuite) TearDownTest() {
 func (suite *HealthcheckTestSuite) TestHttpsGetCheck() {
 	p := NewProxy(suite.S3conf, &helper.AlwaysAllow{}, suite.messenger, suite.database, new(tls.Config))
 
-	url := p.getS3ReadyPath()
+	url, _ := p.getS3ReadyPath()
 	assert.NoError(suite.T(), p.httpsGetCheck(url))
 	assert.Error(suite.T(), p.httpsGetCheck("http://127.0.0.1:8888/nonexistent"), "404 should fail")
 }
 
+func (suite *HealthcheckTestSuite) TestS3URL() {
+	p := NewProxy(suite.S3conf, &helper.AlwaysAllow{}, suite.messenger, suite.database, new(tls.Config))
+
+	_, err := p.getS3ReadyPath()
+	assert.NoError(suite.T(), err)
+
+	p.s3.URL = "://badurl"
+	url, err := p.getS3ReadyPath()
+	assert.Empty(suite.T(), url)
+	assert.Error(suite.T(), err)
+}
+
 func (suite *HealthcheckTestSuite) TestHealthchecks() {
 	// Setup
 	database, _ := database.NewSDAdb(suite.DBConf)