Merge pull request #1412 from broswen/bswenson/observatory

fix double encoding of url

.changelog/1412.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+observatory: fix double url encoding
+```

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

observatory_test.go

@@ -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"
@@ -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,