Basic revocation credential uses encodeURIComponent, not form-urlencoding, so a '+' in a secret breaks it

[cliffhall]

Found in the v2.5.0 milestone-merge review (#2215), in code that shipped on v2/main during the milestone (#2144 / #2186). Filed here rather than fixed in the merge PR, whose tree is byte-identical to origin/v2/main.

The bug

core/auth/revocation.ts builds the RFC 6749 §2.3.1 Basic credential with encodeURIComponent:

const credentials = `${encodeURIComponent(client.client_id)}:${encodeURIComponent(client.client_secret)}`;
headers.Authorization = `Basic ${base64Encode(credentials)}`;

§2.3.1 requires each half to be encoded with the application/x-www-form-urlencoded algorithm (RFC 6749 Appendix B), and encodeURIComponent is not that algorithm. The comment above the line is right about why the halves are encoded; it is the choice of encoder that is wrong.

The failing character is +, not a space

Worth pinning down precisely, because the obvious example does not actually break:

• A space does round-trip. encodeURIComponent(" ") yields %20, and a form-urldecoder accepts %20 as a space just as it accepts +. Compliant servers decode this correctly.
• A literal + does not. encodeURIComponent leaves + unescaped; a form-urldecoder reads + as a space. So a client secret of ab+cd arrives at the authorization server as ab cd, and revocation fails with invalid_client.

That is not an exotic case: client secrets are very often base64, and + is in the base64 alphabet. A meaningful fraction of real secrets contain one.

(!*'() also differ between the two algorithms — encodeURIComponent leaves them bare while Appendix B escapes them — but a decoder passes them through unchanged either way, so they are harmless.)

The fixture masks it

test-servers/src/test-server-oauth.ts's authenticateRevocationClient decodes with decodeURIComponent, the exact inverse of the encoder under test. The round trip therefore succeeds in our tests for every input, including the + that a compliant server would mis-decode. The unit expectation has the same symmetry.

So the tests are not merely failing to cover the case — they are constructed so the case cannot fail. That is the part worth fixing carefully: making the fixture a real RFC decoder is what turns this into a regression test rather than a restatement.

Fix

Form-urlencode each half before the colon. URLSearchParams already implements the algorithm, so there is no need to hand-roll it:

const formEncode = (value: string): string =>
  new URLSearchParams({ v: value }).toString().slice("v=".length);

const credentials = `${formEncode(client.client_id)}:${formEncode(client.client_secret)}`;

Then change the fixture to decode the way a compliant server does — + to space before percent-decoding:

const formDecode = (value: string): string =>
  decodeURIComponent(value.replace(/\+/g, "%20"));

Keep the existing try/catch around it: a malformed escape must still produce the invalid_client 401 rather than an Express 500.

Update the unit expectation alongside both, and add cases for a credential containing +, one containing a space, and one containing : (the case the original encoding change was made for — it must keep working).

Reported by Copilot on #2215; the spec citation is correct, though its worked example (a space) is one of the characters that does survive. The + case is the one that breaks.