From ccff6c18be0b77992101bd4f3b38b5b2d646629c Mon Sep 17 00:00:00 2001
From: Mike Goldsmith <goldsmith.mike@gmail.com>
Date: Wed, 5 Jun 2024 11:58:28 +0100
Subject: [PATCH] feat: URL encode dataset (#242)

## Which problem is this PR solving?

When sending data, the dataset name forms part of the URL path and
should be URL encoded so restricted characters are preserved. This is
particularly important if the dataset name included a forward slash `/`
which results in an invalid URL path and the Honeycomb API rejecting the
request.

## Short description of the changes
- URL encode the dataset before sending data
- Add tests to verify dataset is encoded correctly for use in the
request path
---
 transmission/transmission.go      |  6 ++++-
 transmission/transmission_test.go | 41 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/transmission/transmission.go b/transmission/transmission.go
index 2ecc597..aec5ded 100644
--- a/transmission/transmission.go
+++ b/transmission/transmission.go
@@ -422,7 +422,7 @@ func (b *batchAgg) fireBatch(events []*Event) {
 	}
 
 	// build the HTTP request
-	url.Path = path.Join(url.Path, "/1/batch", dataset)
+	url.Path = buildReqestPath(url.Path, dataset)
 
 	// One retry allowed for connection timeouts.
 	var resp *http.Response
@@ -742,3 +742,7 @@ func buildReqReader(jsonEncoded []byte, compress bool) (io.Reader, bool) {
 type nower interface {
 	Now() time.Time
 }
+
+func buildReqestPath(existingPath, dataset string) string {
+	return path.Join(existingPath, "/1/batch", url.PathEscape(dataset))
+}
diff --git a/transmission/transmission_test.go b/transmission/transmission_test.go
index 6731df5..b1ae3bd 100644
--- a/transmission/transmission_test.go
+++ b/transmission/transmission_test.go
@@ -1358,3 +1358,44 @@ func TestFireBatchWithUnauthorizedResponse(t *testing.T) {
 		testEquals(t, l.logs[0], "APIKey 'written' was rejected. Please verify APIKey is correct.")
 	})
 }
+
+func TestBuildRequestPath(t *testing.T) {
+	testCases := []struct {
+		datasetName  string
+		expectedPath string
+	}{
+		{
+			datasetName:  "foobar",
+			expectedPath: "/1/batch/foobar",
+		},
+		{
+			datasetName:  "foo.bar",
+			expectedPath: "/1/batch/foo.bar",
+		},
+		{
+			datasetName:  "foo-bar",
+			expectedPath: "/1/batch/foo-bar",
+		},
+		{
+			datasetName:  "foo/bar",
+			expectedPath: "/1/batch/foo%2Fbar",
+		},
+		{
+			datasetName:  "foo(bar)",
+			expectedPath: "/1/batch/foo%28bar%29",
+		},
+		{
+			datasetName:  "foo[bar]",
+			expectedPath: "/1/batch/foo%5Bbar%5D",
+		},
+		{
+			datasetName:  "foo{bar}",
+			expectedPath: "/1/batch/foo%7Bbar%7D",
+		},
+	}
+
+	for _, tc := range testCases {
+		path := buildReqestPath("", tc.datasetName)
+		assert.Equal(t, tc.expectedPath, path)
+	}
+}