Skip to content

Hoist a shared query-value encoder — six call sites still use raw .urlQueryAllowed for user-typed values #95

Description

@Adron

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

  • Hoist a single queryValue(_:) into APIClientTransport.swift alongside the other transport
    helpers (internal, not private — private is file-scoped and the per-feature extension
    files could not see it).
    Allowed set: .urlQueryAllowed minus CharacterSet(charactersIn: "+&=?#/").
  • Replace all three local copies with it.
  • Convert the six raw .urlQueryAllowed call sites above.
  • Tests: a search term containing +, &, = and a space round-trips unchanged through
    URLComponents; a base64 cursor containing +, / and = round-trips unchanged.

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

  • Exactly one query-value encoder in the codebase.
  • No remaining addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) on a query
    value (path encoding via .urlPathAllowed is a different thing and stays).
  • Suite green; no behaviour change other than correct encoding.

Files: Services/APIClientTransport.swift, Services/APIClient.swift,
Services/APIClient+Organizations.swift, Services/APIClient+DirectMessages.swift.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtCleanup / dead code / correctness hygienews:papercutsW8 — small parity papercuts

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions