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.

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

purge: fail to purge with query string #353

Merged
merged 3 commits into from Jul 11, 2022
Merged

Conversation

Integralist
Copy link
Collaborator

A customer discovered purging URLs, that contained a query param, would fail to purge.

In the following example, the first URL would purge while the other two would not purge.

purgeURLs := []string{
  "https://example.com/john", 
  "https://example.com/john?payment=success", 
  "https://example.com/john?payment=cancelled",
}

for _, url := range purgeURLs {
  clearCache(url)
}

func (cp *cachePurger) clearCache(purgeURL string) {
  cp.client.Purge(&fastly.PurgeInput{URL: purgeURL})
}

Following a quick review of the API client code I discovered that the .Post() method internally calls .Request(), and that internally calls .RawRequest(). It's in that last method we see this:

var params = make(url.Values)
for k, v := range ro.Params {
params.Add(k, v)
}
request.URL.RawQuery = params.Encode()

Because the top-level Purge() method isn't setting ro.Params (which are passed all the way through to RawRequest()), it means the inner most method will replace any query string values with params.Encode(), which is going to be an empty string as there is no ro.Param content to iterate over and subsequently we never call params.Add to populate the params type, and thus params.Encode() returns an empty string.

The fix is to ensure thePurge() method parses the given URL and extracts its query string (if any) and assigns it to ro.Params so it's passed down to RawRequest() so the internal method can re-apply it as necessary.

@Integralist Integralist requested review from a team, harmony7 and triblondon and removed request for a team and harmony7 July 11, 2022 14:03
@Integralist Integralist merged commit baa0fea into main Jul 11, 2022
@Integralist Integralist deleted the integralist/purge-bug branch July 11, 2022 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants