fix double encoding of url

[broswen]

Description

This fixes a bug where buildURI would double encode forward slashes in the the URL parameter.

Let me know if there is a better way to handle this.

Has your change been tested?

I have tested with a personal zone with URLs that include multiple path segments.
I updated the unit tests to use a URL that has a longer path with multiple segments.

Screenshots (if appropriate):

Types of changes

What sort of change does your code introduce/modify?

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.
• This change is using publicly documented in cloudflare/api-schemas
  and relies on stable APIs.

[github-actions]

changelog detected ✅

[jacobbednarz]

ah, i see what is happening here; the v.Encode() is double encoding it. while your fix works, it does make the usage a little unconventional and if we need to fix core bugs in the future with URL parsing (just like this one) we won't have a single chokepoint to patch it.

instead, i'd prefer we look at a way to address this in buildURI itself. here is a a side by side comparison of the outputs with my idea for a fix - https://go.dev/play/p/8CE_hPEifE3. to make this backwards compatible, i'd probably want to introduce a third parameter of configuration options to buildURI.

type buildURIConfiguration struct {
	UnescapeQueryParams bool
}

func buildURI(path string, options interface{}, config buildURIConfiguration) string {
	v, _ := query.Values(options)

	queryParams := v.Encode()
	if config.UnescapeQueryParams {
		queryParams, _ = url.QueryUnescape(queryParams)
	}

	return (&url.URL{Path: path, RawQuery: queryParams}).String()
}

WDYT? i'd love to make this the default but i think it may potentially break some usages of it (but i could be wrong!)

[broswen]

That seems like it could also be a problem, but specifically the issue isn't with double encoding a query param but with a path parameter.

The endpoints take the url as a path parameter. For example if the url was example.com/subpath the endpoint would be.
https://api.cloudflare.com/client/v4 /zones/{zone_identifier}/speed_api/schedule/example.com%2Fsubpath

Since / is a valid delimiter it doesn't get path escaped by url.URL. But if you path escape the URL beforehand it contains a %2F and url.URL attempts to escape the % a second time leaving you with %252F.

[jacobbednarz]

ah sorry. I must have misread the sprintf as I thought this was an issue in the query parameter.

if this is in the path, any reason why we need to be manually escaping it? any reason we don't just pass it though as is and have the server side handle any security concerns?

[broswen]

Because a request the the endpoint
https://api.cloudflare.com/client/v4 /zones/{zone_identifier}/speed_api/schedule/example.com%2Fsubpath
Is different from
https://api.cloudflare.com/client/v4 /zones/{zone_identifier}/speed_api/schedule/example.com/subpath

The endpoint routing gets confused and doesn't send it to the proper upstream.
Plus there are other endpoints such as the following which will break if the url path parameter isn't escaped.
https://api.cloudflare.com/client/v4 /zones/{zone_identifier}/speed_api/pages/{url}/tests

/pages/example.com%2Fsubpath/tests vs /pages/example.com/subpath/tests

This just seems to be an annoyance with accepting a URL as a path parameter instead of query param or JSON body.

[jacobbednarz]

👍 for now, let's leave this as is and when i'm back we'll have a chat about this. in a generated SDK world, this will be making your maintenance/life alot harder than what it needs to be so we'll see what we can come up with to minimise that.

[github-actions]

This functionality has been released in v0.79.0.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!