diff --git a/.changelog/1412.txt b/.changelog/1412.txt new file mode 100644 index 00000000000..fa44dd47e37 --- /dev/null +++ b/.changelog/1412.txt @@ -0,0 +1,3 @@ +```release-note:bug +observatory: fix double url encoding +``` diff --git a/observatory.go b/observatory.go index 488640fcef1..d1f7d08ed72 100644 --- a/observatory.go +++ b/observatory.go @@ -9,6 +9,7 @@ import ( "time" "github.com/goccy/go-json" + "github.com/google/go-querystring/query" ) var ( @@ -137,7 +138,9 @@ func (api *API) GetObservatoryPageTrend(ctx context.Context, rc *ResourceContain if params.URL == "" { return nil, ErrMissingObservatoryUrl } - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/pages/%s/trend", rc.Identifier, url.PathEscape(params.URL)), params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/pages/%s/trend?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return nil, err @@ -184,7 +187,9 @@ func (api *API) ListObservatoryPageTests(ctx context.Context, rc *ResourceContai var tests []ObservatoryPageTest var lastResultInfo ResultInfo for { - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/pages/%s/tests", rc.Identifier, url.PathEscape(params.URL)), params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/pages/%s/tests?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return nil, nil, err @@ -255,7 +260,9 @@ func (api *API) DeleteObservatoryPageTests(ctx context.Context, rc *ResourceCont if params.URL == "" { return nil, ErrMissingObservatoryUrl } - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/pages/%s/tests", rc.Identifier, url.PathEscape(params.URL)), params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/pages/%s/tests?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, nil) if err != nil { return nil, err @@ -298,7 +305,7 @@ func (api *API) GetObservatoryPageTest(ctx context.Context, rc *ResourceContaine type CreateObservatoryScheduledPageTestParams struct { URL string `url:"-" json:"-"` - Region string `url:"region" json:"region"` + Region string `url:"region" json:"-"` Frequency string `url:"frequency" json:"-"` } @@ -319,8 +326,10 @@ func (api *API) CreateObservatoryScheduledPageTest(ctx context.Context, rc *Reso if params.URL == "" { return nil, ErrMissingObservatoryUrl } - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/schedule/%s", rc.Identifier, url.PathEscape(params.URL)), params) - res, err := api.makeRequestContext(ctx, http.MethodPost, uri, params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/schedule/%s?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) + res, err := api.makeRequestContext(ctx, http.MethodPost, uri, nil) if err != nil { return nil, err } @@ -349,7 +358,9 @@ func (api *API) GetObservatoryScheduledPageTest(ctx context.Context, rc *Resourc if params.URL == "" { return nil, ErrMissingObservatoryUrl } - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/schedule/%s", rc.Identifier, url.PathEscape(params.URL)), params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/schedule/%s?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return nil, err @@ -374,7 +385,9 @@ func (api *API) DeleteObservatoryScheduledPageTest(ctx context.Context, rc *Reso if params.URL == "" { return nil, ErrMissingObservatoryUrl } - uri := buildURI(fmt.Sprintf("/zones/%s/speed_api/schedule/%s", rc.Identifier, url.PathEscape(params.URL)), params) + // cannot use buildURI because params.URL contains "/" that should be encoded and buildURI will double encode %2F into %252F + v, _ := query.Values(params) + uri := fmt.Sprintf("/zones/%s/speed_api/schedule/%s?%s", rc.Identifier, url.PathEscape(params.URL), v.Encode()) res, err := api.makeRequestContext(ctx, http.MethodDelete, uri, nil) if err != nil { return nil, err diff --git a/observatory_test.go b/observatory_test.go index 98ff512098b..fa897141d04 100644 --- a/observatory_test.go +++ b/observatory_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "testing" "time" @@ -12,7 +13,8 @@ import ( "github.com/stretchr/testify/assert" ) -var testURL = "example.com" +var testURL = "example.com/a/b" +var escapedTestURL = url.PathEscape(testURL) var region = "us-central1" var regionLabel = "Iowa, USA" var frequency = "DAILY" @@ -173,6 +175,7 @@ func TestListObservatoryPages(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method, "Expected method 'GET', got %s", r.Method) w.Header().Set("content-type", "application/json") + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages", r.URL.EscapedPath()) fmt.Fprintf(w, `{ "success": true, "errors": [], @@ -202,6 +205,7 @@ func TestObservatoryPageTrend(t *testing.T) { assert.Equal(t, "DESKTOP", r.URL.Query().Get("deviceType")) assert.Equal(t, "America/Chicago", r.URL.Query().Get("tz")) assert.Equal(t, "fcp,lcp", r.URL.Query().Get("metrics")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages/"+escapedTestURL+"/trend", r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -254,6 +258,7 @@ func TestListObservatoryPageTests(t *testing.T) { assert.Equal(t, region, r.URL.Query().Get("region")) assert.Equal(t, "1", r.URL.Query().Get("page")) assert.Equal(t, "10", r.URL.Query().Get("per_page")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages/"+escapedTestURL+"/tests", r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -289,6 +294,7 @@ func TestCreateObservatoryPageTest(t *testing.T) { b, err := io.ReadAll(r.Body) assert.NoError(t, err) assert.True(t, strings.Contains(string(b), region)) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages/"+escapedTestURL+"/tests", r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -318,6 +324,7 @@ func TestDeleteObservatoryPageTests(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodDelete, r.Method, "Expected method 'DELETE', got %s", r.Method) assert.Equal(t, region, r.URL.Query().Get("region")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages/"+escapedTestURL+"/tests", r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -346,6 +353,8 @@ func TestGetObservatoryPageTest(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method, "Expected method 'GET', got %s", r.Method) + + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/pages/"+escapedTestURL+"/tests/"+observatoryTestID, r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -374,6 +383,7 @@ func TestCreateObservatoryScheduledPageTest(t *testing.T) { assert.Equal(t, http.MethodPost, r.Method, "Expected method 'POST', got %s", r.Method) assert.Equal(t, frequency, r.URL.Query().Get("frequency")) assert.Equal(t, region, r.URL.Query().Get("region")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/schedule/"+escapedTestURL, r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -402,6 +412,7 @@ func TestObservatoryScheduledPageTest(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method, "Expected method 'GET', got %s", r.Method) assert.Equal(t, region, r.URL.Query().Get("region")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/schedule/"+escapedTestURL, r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true, @@ -429,6 +440,7 @@ func TestDeleteObservatoryScheduledPageTest(t *testing.T) { handler := func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodDelete, r.Method, "Expected method 'DELETE', got %s", r.Method) assert.Equal(t, region, r.URL.Query().Get("region")) + assert.Equal(t, "/zones/"+testZoneID+"/speed_api/schedule/"+escapedTestURL, r.URL.EscapedPath()) w.Header().Set("content-type", "application/json") fmt.Fprintf(w, `{ "success": true,