Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix double encoding of url #1412

Merged
merged 1 commit into from
Oct 2, 2023
Merged

fix double encoding of url #1412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/1412.txt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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