Surfaced while fixing #77 (see PR #93).
The hazard
CharacterSet.urlQueryAllowed permits +, &, =, ?, # and /. Percent-encoding a query
value with it therefore leaves those characters intact, and:
- a raw
+ decodes to a space on the server, and
- a raw
& or = splits the value into another query parameter.
For a search box, that means a term like a & b or c++ silently searches for something else, or
truncates. For an opaque cursor it means paging quietly breaks — the page repeats or ends early,
with no error anywhere.
Call sites still using the raw set
All passing user-typed or server-issued opaque values (Services/APIClient.swift, approx. lines):
| ~Line |
What it encodes |
| 640 |
search term |
| 709 |
search term |
| 1097 |
search term |
| 1209 |
search term |
| 1325 |
search term |
| 1467 |
pagination cursor |
The cursor one is the most likely to be actively broken today, since base64 routinely contains +
and =.
Three copies of the fix already exist
Each was written locally because the branches were independent:
Three occurrences is past the point where it should be one function.
Build this
Sequencing
Touches APIClient.swift and both extension files, so it will conflict with anything else in flight
there. Best run after #86 and #93 merge, so all three copies exist in one tree and can be
collapsed in a single pass.
Acceptance criteria
Files: Services/APIClientTransport.swift, Services/APIClient.swift,
Services/APIClient+Organizations.swift, Services/APIClient+DirectMessages.swift.
Surfaced while fixing #77 (see PR #93).
The hazard
CharacterSet.urlQueryAllowedpermits+,&,=,?,#and/. Percent-encoding a queryvalue with it therefore leaves those characters intact, and:
+decodes to a space on the server, and&or=splits the value into another query parameter.For a search box, that means a term like
a & borc++silently searches for something else, ortruncates. For an opaque cursor it means paging quietly breaks — the page repeats or ends early,
with no error anywhere.
Call sites still using the raw set
All passing user-typed or server-issued opaque values (
Services/APIClient.swift, approx. lines):The cursor one is the most likely to be actively broken today, since base64 routinely contains
+and
=.Three copies of the fix already exist
Each was written locally because the branches were independent:
APIClient+Organizations.swift— file-privateorgQueryValueAllowed(also strips#)APIClient+DirectMessages.swift—encodedQueryValue(on the feat(messages): group the DM inbox by conversation #86 branch)APIClient.swift—queryValueAllowed(added by fix(identities): send unlink provider as a query param #93)Three occurrences is past the point where it should be one function.
Build this
queryValue(_:)intoAPIClientTransport.swiftalongside the other transporthelpers (
internal, notprivate—privateis file-scoped and the per-feature extensionfiles could not see it).
Allowed set:
.urlQueryAllowedminusCharacterSet(charactersIn: "+&=?#/")..urlQueryAllowedcall sites above.+,&,=and a space round-trips unchanged throughURLComponents; a base64 cursor containing+,/and=round-trips unchanged.Sequencing
Touches
APIClient.swiftand both extension files, so it will conflict with anything else in flightthere. Best run after #86 and #93 merge, so all three copies exist in one tree and can be
collapsed in a single pass.
Acceptance criteria
addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed)on a queryvalue (path encoding via
.urlPathAllowedis a different thing and stays).Files:
Services/APIClientTransport.swift,Services/APIClient.swift,Services/APIClient+Organizations.swift,Services/APIClient+DirectMessages.swift.