expandURL double-encodes every query value, corrupting page tokens and free-text search
Context: API Makeathon participant. Found while reviewing the URL builder; independently confirmed by a second review pass.
expandURL escapes each value with url.QueryEscape, places the escaped string into url.Values, then calls Values.Encode(), which escapes again. The %253A -> %3A replacement at the end is proof the double-encoding was noticed and patched for colons only.
Evidence
utils.go:24-66 (and the cmd/utils.go copy):
for k, v := range expansions {
expansions[k] = url.QueryEscape(v) // first encoding
}
...
values.Set(k, v) // v already escaped
u.RawQuery = values.Encode() // second encoding
u.RawQuery = strings.Replace(u.RawQuery, "%253A", ":", -1) // patches only colons
Observed round-trips (what the caller sent vs what the server decodes):
| Input |
Server sees |
a b |
a+b |
tok= |
tok%3D |
a/b |
a%2Fb |
user+x@zoo.dev |
user%2Bx%40zoo.dev |
by:created_at |
by:created_at (works, via the hack) |
Concrete failure
Any pageToken containing =, +, or / (typical of base64 tokens) is corrupted, so pagination 400s or restarts from page one. Free-text params are corrupted too: SearchDatasetConversions(..., q="bracket mount", ...) searches for the literal bracket+mount.
Verify
Call expandURL with q="a b" or page_token="tok=", parse the result with url.Parse, and inspect parsed.Query().Get(...): it does not equal the input.
Suggested fix
Drop the pre-escaping loop; url.Values.Encode() already escapes values correctly exactly once.
Environment
Reviewed against the current main of KittyCAD/kittycad.go.
expandURLdouble-encodes every query value, corrupting page tokens and free-text searchContext: API Makeathon participant. Found while reviewing the URL builder; independently confirmed by a second review pass.
expandURLescapes each value withurl.QueryEscape, places the escaped string intourl.Values, then callsValues.Encode(), which escapes again. The%253A -> %3Areplacement at the end is proof the double-encoding was noticed and patched for colons only.Evidence
utils.go:24-66(and thecmd/utils.gocopy):Observed round-trips (what the caller sent vs what the server decodes):
a ba+btok=tok%3Da/ba%2Fbuser+x@zoo.devuser%2Bx%40zoo.devby:created_atby:created_at(works, via the hack)Concrete failure
Any
pageTokencontaining=,+, or/(typical of base64 tokens) is corrupted, so pagination 400s or restarts from page one. Free-text params are corrupted too:SearchDatasetConversions(..., q="bracket mount", ...)searches for the literalbracket+mount.Verify
Call
expandURLwithq="a b"orpage_token="tok=", parse the result withurl.Parse, and inspectparsed.Query().Get(...): it does not equal the input.Suggested fix
Drop the pre-escaping loop;
url.Values.Encode()already escapes values correctly exactly once.Environment
Reviewed against the current
mainof KittyCAD/kittycad.go.