Skip to content

Commit

Permalink
Prevent double-escaped URL query params
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas11 committed Sep 24, 2024
1 parent 41a95f7 commit 0968499
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
14 changes: 13 additions & 1 deletion provider/pkg/azure/client_azcore.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,19 @@ func (c *azCoreClient) initRequest(ctx context.Context, method, id string, query
c.setHeaders(req, method == http.MethodPost || method == http.MethodPut || method == http.MethodPatch)

urlValues := MapToValues(queryParams)
// TODO,tkappler go-autorest WithQueryParameters in preparer.go does an additional query-unescape for each value - why?
// URL-unescape each value before encoding the URL (which encodes all values). Presumably, this
// prevents double-encoding. It's not clear to me that this is necessary but Autorest does it and
// we want to match behavior here as closely as possible.
for key, value := range urlValues {
for i := range value {
d, err := url.QueryUnescape(value[i])
if err != nil {
return nil, err
}
value[i] = d
}
urlValues[key] = value
}
req.Raw().URL.RawQuery = urlValues.Encode()

return req, nil
Expand Down
12 changes: 8 additions & 4 deletions provider/pkg/azure/client_azcore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,21 @@ func TestInitRequestQueryParams(t *testing.T) {
client := c.(*azCoreClient)

queryParams := map[string]any{
"api-version": "2022-09-01",
"bool": true,
"int": 42,
"slice": []string{"a", "b"},
"api-version": "2022-09-01",
"string_with_unescaped_slash": "a/path",
"string_with_escaped_slash": "a%2Fpath",
"bool": true,
"int": 42,
"slice": []string{"a", "b"},
}
req, err := client.initRequest(context.Background(), http.MethodGet, "/subscriptions/123", queryParams)
require.NoError(t, err)

query := req.Raw().URL.Query()
assert.Len(t, query, len(queryParams))
assert.Equal(t, "2022-09-01", query.Get("api-version"))
assert.Equal(t, "a/path", query.Get("string_with_unescaped_slash"))
assert.Equal(t, "a/path", query.Get("string_with_escaped_slash"))
assert.Equal(t, "true", query.Get("bool"))
assert.Equal(t, "42", query.Get("int"))
assert.Equal(t, "a", query.Get("slice"))
Expand Down

0 comments on commit 0968499

Please sign in to comment.