fix: count a pct-triplet as one character under every operator
Addresses Copilot's round-11 review on #2035, which corrects my own round-9
fix rather than the original code.
That fix grouped pct-triplets only under `+` and `#`, on the reasoning that
every other operator escapes a `%` anyway, so `%61` is three characters
rather than one. Wrong, in a way the narrower rule made visible one operator
over -- measured on 2e06b369:
x/{v:1} v = "%61%62" => "x/%25" truncated to a `%` that was never
a character of the value
RFC 6570 2.4.1 counts characters and says a pct-encoded triplet counts as a
single one, without reference to the operator. Grouping per-operator also
made the same input mean two different things: `%61` indivisible under `+`,
three loose characters under `{v}`. What the operator decides is how a
retained unit is *encoded* afterwards -- a simple expansion still escapes
the kept triplet's `%`, so `%61` goes out as `%2561` -- not what counts as
one character.
`prefixUnits` no longer takes an operator, and the test that codified the
old split now asserts the uniform rule across simple, named, and reserved
expansion.
npm run ci passes.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
2e06b369f4
commit
c6fb627142
2 changed files with 38 additions and 19 deletions
|
|
@ -714,11 +714,25 @@ describe("prefix modifier truncation", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("counts a triplet as three characters where the encoder escapes it", () => {
|
||||
// Under a non-reserved operator `%` is escaped anyway (`%61` -> `%2561`),
|
||||
// so the three characters really are three and grouping them would
|
||||
// mis-count the prefix.
|
||||
expect(expandUriTemplate("x/{v:3}", { v: "%61%62" })).toBe("x/%2561");
|
||||
it.each([
|
||||
// The counting rule is operator-INDEPENDENT: a triplet is one character
|
||||
// wherever it appears. An earlier revision grouped only under `+`/`#`,
|
||||
// which made the same input mean two things and truncated `{v:1}` to a `%`
|
||||
// that was never a character of the value (it emitted "%25").
|
||||
["x/{v:1}", "x/%2561"],
|
||||
["x/{v:2}", "x/%2561%2562"],
|
||||
["x/{v:3}", "x/%2561%2562"],
|
||||
])(
|
||||
"counts a triplet as one character under a simple expansion (%s)",
|
||||
(template, expected) => {
|
||||
// What the operator decides is the *encoding* afterwards -- a simple
|
||||
// expansion still escapes the retained triplet's `%`, so `%61` -> `%2561`.
|
||||
expect(expandUriTemplate(template, { v: "%61%62" })).toBe(expected);
|
||||
},
|
||||
);
|
||||
|
||||
it("counts a triplet as one character under a named operator too", () => {
|
||||
expect(expandUriTemplate("x{?v:1}", { v: "%61%62" })).toBe("x?v=%2561");
|
||||
});
|
||||
|
||||
it("truncates by code point, not UTF-16 code unit", () => {
|
||||
|
|
|
|||
|
|
@ -506,23 +506,28 @@ function encodeValue(value: string, operator: string): string {
|
|||
/**
|
||||
* Splits a value into the units a prefix modifier counts.
|
||||
*
|
||||
* RFC 6570 §2.4.1 counts *characters*, and is explicit that "a pct-encoded
|
||||
* triplet counts as a single character" — so `{+v:5}` over `%61%62%63%64%65%66`
|
||||
* must stop after five whole triplets. Splitting on code points alone cut the
|
||||
* sixth in half, and the trailing `%6` was then no longer a triplet, so
|
||||
* `encodeAllowReserved` correctly encoded its `%`: the value went out as
|
||||
* `%61%256`, a byte the server never asked for.
|
||||
* RFC 6570 §2.4.1 counts *characters*, and is explicit that a pct-encoded
|
||||
* triplet counts as a single one — so `{+v:5}` over `%61%62%63%64%65%66` stops
|
||||
* after five whole triplets. Splitting on code points cut the sixth in half,
|
||||
* and the orphaned `%6` was no longer a triplet, so `encodeAllowReserved`
|
||||
* correctly escaped its `%`: the value went out as `%61%256`, a byte the caller
|
||||
* never asked for.
|
||||
*
|
||||
* Triplets are units only under `+` and `#`. Everywhere else the encoder
|
||||
* escapes a `%` anyway (`%61` becomes `%2561`), so the three characters are
|
||||
* three characters and grouping them would mis-count the prefix.
|
||||
* The rule is **operator-independent**, which is a correction of the narrower
|
||||
* version this shipped first. Grouping only under `+`/`#` made the same input
|
||||
* mean two different things — `%61` an indivisible character under `+`, three
|
||||
* loose characters under a simple expression — and produced the matching
|
||||
* defect one operator over: `{v:1}` over `%61%62` kept just the `%` and emitted
|
||||
* `%25`, a truncation to a character that was never in the value. What the
|
||||
* operator decides is how a unit is *encoded* afterwards (a simple expansion
|
||||
* still escapes the `%`, so the retained `%61` goes out as `%2561`), not what
|
||||
* counts as one.
|
||||
*
|
||||
* The `u` flag makes the alternation match by code point, so an astral
|
||||
* character is one unit rather than a surrogate pair that truncation could
|
||||
* split — the reason `Array.from` was used here in the first place.
|
||||
* split.
|
||||
*/
|
||||
function prefixUnits(value: string, operator: string): string[] {
|
||||
if (!allowsReserved(operator)) return Array.from(value);
|
||||
function prefixUnits(value: string): string[] {
|
||||
return value.match(/%[0-9A-Fa-f]{2}|[\s\S]/gu) ?? [];
|
||||
}
|
||||
|
||||
|
|
@ -537,7 +542,7 @@ function renderValue(value: string, spec: VarSpec, operator: string): string {
|
|||
const truncated =
|
||||
spec.maxLength === undefined
|
||||
? value
|
||||
: prefixUnits(value, operator).slice(0, spec.maxLength).join("");
|
||||
: prefixUnits(value).slice(0, spec.maxLength).join("");
|
||||
return encodeValue(truncated, operator);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue