From 9bc948154ac31f2a994f26d7cbac6a1c58a1a13a Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 13 Jun 2024 16:44:32 +0100 Subject: [PATCH 1/4] fix: Build the URL using path.Join and not appending to url.Path --- transmission/transmission.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/transmission/transmission.go b/transmission/transmission.go index aec5ded..951e84d 100644 --- a/transmission/transmission.go +++ b/transmission/transmission.go @@ -399,7 +399,8 @@ func (b *batchAgg) fireBatch(events []*Event) { } } - url, err := url.Parse(apiHost) + // make sure the api host is valid, if not, error out all the events + _, err := url.Parse(apiHost) if err != nil { end := time.Now().UTC() if b.testNower != nil { @@ -421,8 +422,8 @@ func (b *batchAgg) fireBatch(events []*Event) { return } - // build the HTTP request - url.Path = buildReqestPath(url.Path, dataset) + // build the HTTP request URL + url := buildReqestPath(apiHost, dataset) // One retry allowed for connection timeouts. var resp *http.Response @@ -437,9 +438,9 @@ func (b *batchAgg) fireBatch(events []*Event) { // Pass the underlying bytes.Reader to http.Request so that // GetBody and ContentLength fields are populated on Request. // See https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/net/http/request.go;l=898 - req, err = http.NewRequest("POST", url.String(), &reader.Reader) + req, err = http.NewRequest("POST", url, &reader.Reader) } else { - req, err = http.NewRequest("POST", url.String(), reqBody) + req, err = http.NewRequest("POST", url, reqBody) } req.Header.Set("Content-Type", contentType) if zipped { @@ -743,6 +744,6 @@ type nower interface { Now() time.Time } -func buildReqestPath(existingPath, dataset string) string { - return path.Join(existingPath, "/1/batch", url.PathEscape(dataset)) +func buildReqestPath(apiHost, dataset string) string { + return path.Join(apiHost, "/1/batch", url.PathEscape(dataset)) } From 144fc0de562ccebda25235632fceb293f6bdd40f Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 13 Jun 2024 18:01:44 +0100 Subject: [PATCH 2/4] rename buildReqestPath to buildRequestURL --- transmission/transmission.go | 4 ++-- transmission/transmission_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/transmission/transmission.go b/transmission/transmission.go index 951e84d..64859fb 100644 --- a/transmission/transmission.go +++ b/transmission/transmission.go @@ -423,7 +423,7 @@ func (b *batchAgg) fireBatch(events []*Event) { } // build the HTTP request URL - url := buildReqestPath(apiHost, dataset) + url := buildRequestURL(apiHost, dataset) // One retry allowed for connection timeouts. var resp *http.Response @@ -744,6 +744,6 @@ type nower interface { Now() time.Time } -func buildReqestPath(apiHost, dataset string) string { +func buildRequestURL(apiHost, dataset string) string { return path.Join(apiHost, "/1/batch", url.PathEscape(dataset)) } diff --git a/transmission/transmission_test.go b/transmission/transmission_test.go index b1ae3bd..46b37bb 100644 --- a/transmission/transmission_test.go +++ b/transmission/transmission_test.go @@ -1395,7 +1395,7 @@ func TestBuildRequestPath(t *testing.T) { } for _, tc := range testCases { - path := buildReqestPath("", tc.datasetName) + path := buildRequestURL("", tc.datasetName) assert.Equal(t, tc.expectedPath, path) } } From b3b663b0506541e624807a4d1172f669509415d9 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 13 Jun 2024 18:11:31 +0100 Subject: [PATCH 3/4] move build into parse --- transmission/transmission.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/transmission/transmission.go b/transmission/transmission.go index 64859fb..7191b35 100644 --- a/transmission/transmission.go +++ b/transmission/transmission.go @@ -400,7 +400,7 @@ func (b *batchAgg) fireBatch(events []*Event) { } // make sure the api host is valid, if not, error out all the events - _, err := url.Parse(apiHost) + url, err := url.Parse(buildRequestURL(apiHost, dataset)) if err != nil { end := time.Now().UTC() if b.testNower != nil { @@ -422,9 +422,6 @@ func (b *batchAgg) fireBatch(events []*Event) { return } - // build the HTTP request URL - url := buildRequestURL(apiHost, dataset) - // One retry allowed for connection timeouts. var resp *http.Response for try := 0; try < 2; try++ { @@ -438,9 +435,9 @@ func (b *batchAgg) fireBatch(events []*Event) { // Pass the underlying bytes.Reader to http.Request so that // GetBody and ContentLength fields are populated on Request. // See https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/net/http/request.go;l=898 - req, err = http.NewRequest("POST", url, &reader.Reader) + req, err = http.NewRequest("POST", url.String(), &reader.Reader) } else { - req, err = http.NewRequest("POST", url, reqBody) + req, err = http.NewRequest("POST", url.String(), reqBody) } req.Header.Set("Content-Type", contentType) if zipped { From fdef7d6dcfcd351f5f862c0d718d7116147885f6 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 13 Jun 2024 18:30:01 +0100 Subject: [PATCH 4/4] use url.PathJoin --- transmission/transmission.go | 12 +++++------- transmission/transmission_test.go | 5 +++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/transmission/transmission.go b/transmission/transmission.go index 7191b35..243b193 100644 --- a/transmission/transmission.go +++ b/transmission/transmission.go @@ -19,7 +19,6 @@ import ( "io/ioutil" "net/http" "net/url" - "path" "runtime" "strings" "sync" @@ -399,8 +398,7 @@ func (b *batchAgg) fireBatch(events []*Event) { } } - // make sure the api host is valid, if not, error out all the events - url, err := url.Parse(buildRequestURL(apiHost, dataset)) + url, err := buildRequestURL(apiHost, dataset) if err != nil { end := time.Now().UTC() if b.testNower != nil { @@ -435,9 +433,9 @@ func (b *batchAgg) fireBatch(events []*Event) { // Pass the underlying bytes.Reader to http.Request so that // GetBody and ContentLength fields are populated on Request. // See https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/net/http/request.go;l=898 - req, err = http.NewRequest("POST", url.String(), &reader.Reader) + req, err = http.NewRequest("POST", url, &reader.Reader) } else { - req, err = http.NewRequest("POST", url.String(), reqBody) + req, err = http.NewRequest("POST", url, reqBody) } req.Header.Set("Content-Type", contentType) if zipped { @@ -741,6 +739,6 @@ type nower interface { Now() time.Time } -func buildRequestURL(apiHost, dataset string) string { - return path.Join(apiHost, "/1/batch", url.PathEscape(dataset)) +func buildRequestURL(apiHost, dataset string) (string, error) { + return url.JoinPath(apiHost, "/1/batch", url.PathEscape(dataset)) } diff --git a/transmission/transmission_test.go b/transmission/transmission_test.go index 46b37bb..3b025b5 100644 --- a/transmission/transmission_test.go +++ b/transmission/transmission_test.go @@ -1395,7 +1395,8 @@ func TestBuildRequestPath(t *testing.T) { } for _, tc := range testCases { - path := buildRequestURL("", tc.datasetName) - assert.Equal(t, tc.expectedPath, path) + url, err := buildRequestURL("http://fakeHost:8080", tc.datasetName) + assert.NoError(t, err) + assert.Equal(t, "http://fakeHost:8080"+tc.expectedPath, url) } }