Skip to content

Commit bd51161

Browse files
matteogastaldellomatteogastaldello
authored
fix: url encoding of path and query parameters (#22)
1 parent f0831f2 commit bd51161

File tree

6 files changed

+368
-5
lines changed

6 files changed

+368
-5
lines changed

‎internal/client/clienttools.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,30 @@ func ToType(ty string) (AuthType, error) {
4848

4949
func buildPath(baseUrl string, path string, parameters map[string]string, query map[string]string) *url.URL {
5050
for key, param := range parameters {
51-
path = strings.Replace(path, fmt.Sprintf("{%s}", key), fmt.Sprintf("%v", param), 1)
51+
param = url.PathEscape(param)
52+
path = strings.Replace(path, fmt.Sprintf("{%s}", key), param, 1)
5253
}
5354

5455
params := url.Values{}
55-
5656
for key, param := range query {
57-
params.Add(key, param)
57+
queryParam := url.QueryEscape(param)
58+
params.Add(key, queryParam)
5859
}
5960

6061
parsed, err := url.Parse(baseUrl)
6162
if err != nil {
6263
return nil
6364
}
6465

65-
parsed.Path = pathutil.Join(parsed.Path, path)
66+
// Remove trailing slash from base path if present
67+
parsed.Opaque = "//" + pathutil.Join(parsed.Host, parsed.Path, path)
6668
parsed.RawQuery = params.Encode()
69+
parsed, err = url.Parse(parsed.String())
70+
if err != nil {
71+
return nil
72+
}
6773
return parsed
6874
}
69-
7075
func getValidResponseCode(codes *orderedmap.Map[string, *v3.Response]) ([]int, error) {
7176
var validCodes []int
7277
for code := codes.First(); code != nil; code = code.Next() {

‎internal/client/clienttools_test.go‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
package restclient
2+
3+
import (
4+
"fmt"
5+
"net/url"
6+
"reflect"
7+
"testing"
8+
)
9+
10+
func TestBuildPath_Basic(t *testing.T) {
11+
baseUrl := "https://api.example.com/v1"
12+
path := "/users/{userId}/posts/{postId}"
13+
parameters := map[string]string{
14+
"userId": "42",
15+
"postId": "99",
16+
}
17+
query := map[string]string{
18+
"sort": "desc",
19+
"page": "2",
20+
}
21+
22+
got := buildPath(baseUrl, path, parameters, query)
23+
if got == nil {
24+
t.Fatalf("buildPath returned nil")
25+
}
26+
27+
got, err := url.Parse(got.String())
28+
if err != nil {
29+
t.Fatalf("failed to parse base URL: %v", err)
30+
}
31+
32+
expectedPath := "/v1/users/42/posts/99"
33+
if got.Path != "/v1/users/42/posts/99" {
34+
t.Errorf("expected path %q, got %q", expectedPath, got.Path)
35+
}
36+
37+
wantQuery := url.Values{"sort": {"desc"}, "page": {"2"}}.Encode()
38+
if got.RawQuery != wantQuery && got.RawQuery != "page=2&sort=desc" {
39+
t.Errorf("expected query %q, got %q", wantQuery, got.RawQuery)
40+
}
41+
}
42+
43+
func TestBuildPath_WithPathParamsWithSlashes(t *testing.T) {
44+
baseUrl := "https://api.example.com/v1"
45+
path := "/users/{userId}/posts/{postId}"
46+
parameters := map[string]string{
47+
"userId": "42/123",
48+
"postId": "99",
49+
}
50+
query := map[string]string{}
51+
52+
got := buildPath(baseUrl, path, parameters, query)
53+
if got == nil {
54+
t.Fatalf("buildPath returned nil")
55+
}
56+
57+
fmt.Println("Got Path:", got.RawQuery)
58+
59+
wantPath := baseUrl + "/users/42%2F123/posts/99"
60+
if got.String() != wantPath {
61+
t.Errorf("expected path %q, got %q", wantPath, got.String())
62+
}
63+
}
64+
65+
func TestBuildPath_NoParams(t *testing.T) {
66+
baseUrl := "https://api.example.com"
67+
path := "/status"
68+
parameters := map[string]string{}
69+
query := map[string]string{}
70+
71+
got := buildPath(baseUrl, path, parameters, query)
72+
if got == nil {
73+
t.Fatalf("buildPath returned nil")
74+
}
75+
got, err := url.Parse(got.String())
76+
if err != nil {
77+
t.Fatalf("failed to parse base URL: %v", err)
78+
}
79+
80+
wantPath := "/status"
81+
if got.Path != wantPath {
82+
t.Errorf("expected path %q, got %q", wantPath, got.Path)
83+
}
84+
if got.RawQuery != "" {
85+
t.Errorf("expected empty query, got %q", got.RawQuery)
86+
}
87+
}
88+
89+
func TestBuildPath_InvalidBaseURL(t *testing.T) {
90+
baseUrl := "://bad-url"
91+
path := "/foo"
92+
parameters := map[string]string{}
93+
query := map[string]string{}
94+
95+
got := buildPath(baseUrl, path, parameters, query)
96+
if got != nil {
97+
t.Errorf("expected nil for invalid baseUrl, got %v", got)
98+
}
99+
}
100+
101+
func TestBuildPath_MultipleQueryParams(t *testing.T) {
102+
baseUrl := "https://api.example.com"
103+
path := "/search"
104+
parameters := map[string]string{}
105+
query := map[string]string{
106+
"q": "golang",
107+
"lang": "en",
108+
}
109+
110+
got := buildPath(baseUrl, path, parameters, query)
111+
if got == nil {
112+
t.Fatalf("buildPath returned nil")
113+
}
114+
115+
got, err := url.Parse(got.String())
116+
if err != nil {
117+
t.Fatalf("failed to parse base URL: %v", err)
118+
}
119+
120+
wantPath := "/search"
121+
if got.Path != wantPath {
122+
t.Errorf("expected path %q, got %q", wantPath, got.Path)
123+
}
124+
125+
parsedQuery, _ := url.ParseQuery(got.RawQuery)
126+
expectedQuery := url.Values{"q": {"golang"}, "lang": {"en"}}
127+
if !reflect.DeepEqual(parsedQuery, expectedQuery) {
128+
t.Errorf("expected query %v, got %v", expectedQuery, parsedQuery)
129+
}
130+
}
131+
132+
func TestBuildPath_PathWithNoLeadingSlash(t *testing.T) {
133+
baseUrl := "https://api.example.com/api"
134+
path := "foo/bar"
135+
parameters := map[string]string{}
136+
query := map[string]string{}
137+
138+
got := buildPath(baseUrl, path, parameters, query)
139+
if got == nil {
140+
t.Fatalf("buildPath returned nil")
141+
}
142+
143+
got, err := url.Parse(got.String())
144+
if err != nil {
145+
t.Fatalf("failed to parse base URL: %v", err)
146+
}
147+
148+
wantPath := "/api/foo/bar"
149+
if got.Path != wantPath {
150+
t.Errorf("expected path %q, got %q", wantPath, got.Path)
151+
}
152+
}

‎internal/client/restclient_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"net/http"
88
"net/http/httptest"
9+
"net/url"
910
"os"
1011
"testing"
1112

@@ -42,6 +43,25 @@ func TestCallWithRecorder(t *testing.T) {
4243
expected interface{}
4344
expectedError string
4445
}{
46+
{
47+
name: "path with slash in path parameter",
48+
handler: func(w http.ResponseWriter, r *http.Request) {
49+
assert.Equal(t, "GET", r.Method)
50+
r.URL, _ = url.Parse(r.URL.String())
51+
assert.Equal(t, "/api/test/123%2F456", r.URL.Path)
52+
w.Header().Set("Content-Type", "application/json")
53+
w.WriteHeader(http.StatusOK)
54+
json.NewEncoder(w).Encode(map[string]interface{}{"message": "success"})
55+
},
56+
path: "/api/test/{id}",
57+
opts: &RequestConfiguration{
58+
Method: "GET",
59+
Parameters: map[string]string{
60+
"id": "123%2F456",
61+
},
62+
},
63+
expected: map[string]interface{}{"message": "success"},
64+
},
4565
{
4666
name: "successful GET request",
4767
handler: func(w http.ResponseWriter, r *http.Request) {

‎internal/client/testdata/openapi.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,30 @@ servers:
77
description: Test server
88

99
paths:
10+
/api/test/{id}:
11+
get:
12+
summary: Get test data by ID
13+
operationId: getTestById
14+
parameters:
15+
- name: id
16+
in: path
17+
required: true
18+
schema:
19+
type: string
20+
responses:
21+
'200':
22+
description: Successful operation
23+
content:
24+
application/json:
25+
schema:
26+
type: object
27+
properties:
28+
id:
29+
type: string
30+
name:
31+
type: string
32+
'404':
33+
description: Not found
1034
/api/test:
1135
get:
1236
summary: Get test data

‎internal/restResources/restResources.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c
6161
if mg == nil {
6262
return controller.ExternalObservation{}, fmt.Errorf("custom resource is nil")
6363
}
64+
6465
log := h.logger.WithValues("op", "Observe").
6566
WithValues("apiVersion", mg.GetAPIVersion()).
6667
WithValues("kind", mg.GetKind()).

0 commit comments

Comments
 (0)