Skip to content

Commit

Permalink
fix double encoding of url
Browse files Browse the repository at this point in the history
  • Loading branch information
@broswen
broswen committed Sep 29, 2023
1 parent d83ea8d commit b1fbbd8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .changelog/1412.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
observatory: fix double url encoding
```
29 changes: 21 additions & 8 deletions observatory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/goccy/go-json"
"github.com/google/go-querystring/query"
)

var (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:"-"`
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 13 additions & 1 deletion observatory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import (
"fmt"
"io"
"net/http"
"net/url"
"strings"
"testing"
"time"

"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"
Expand Down Expand Up @@ -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": [],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b1fbbd8

Please sign in to comment.