Skip to content

Commit

Permalink
feat: URL encode dataset (#242)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
MikeGoldsmith authored Jun 5, 2024
1 parent fcd9c4e commit ccff6c1
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
6 changes: 5 additions & 1 deletion transmission/transmission.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
41 changes: 41 additions & 0 deletions transmission/transmission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit ccff6c1

Please sign in to comment.