Skip to content

Commit 5e96c8c

Browse files
committed
Recover round-trip for associative keys with non-varname characters
`isExplodedPairBoundary` was using RFC 6570 varname rules (`isVarcharAt` plus the point-continuation logic in `readPairKeyEnd`) to detect the start of the next exploded associative entry. Associative keys are emitted through `encodeValue` on the expansion side, so the legal key character set is the full unreserved/reserved alphabet, not just varname characters. Round-trip matches against keys like `b-c`, `~x`, or any key containing `-`, `!`, `~`, etc. silently failed: the boundary detector skipped past the next separator and the entire tail of the body was treated as one value, leaving the round-trip check to reject the only decomposition the matcher had reached. Switch the boundary check to a value-driven scan: at each separator, walk forward until the next `=` (boundary) or the next separator (no boundary). The detector still fires only at separator positions, so an `=` inside an `allowReserved` value cannot be mistaken for a key/value split. The now-unused `readPairKeyEnd` helper and the `isVarcharAt` import are removed. Adds a regression test covering `{keys*}` against `a=1,b-c=2`, plus a companion test that pins the parser's rejection of the seven actual 1-byte control characters (NUL/SOH/TAB/LF/CR/US/DEL) via `String.fromCodePoint`. The CTL test documents the boundary that the `wrong.json` fixture *names* but does not exercise (since JSON parses the double-escaped `` sequences in those fixtures to 6-character backslash strings, not control bytes). Reported in PR #758 (CodeRabbit, 2026-05-11). Assisted-by: Claude Opus 4.7 (1M context)
1 parent c611249 commit 5e96c8c

2 files changed

Lines changed: 38 additions & 21 deletions

File tree

packages/uri-template/src/template/match.ts

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type {
88
Token,
99
VarSpec,
1010
} from "../types.ts";
11-
import { encodeName, isVarcharAt, truncateValue } from "./encoding.ts";
11+
import { encodeName, truncateValue } from "./encoding.ts";
1212
import expand from "./expand.ts";
1313

1414
/**
@@ -351,27 +351,12 @@ const isExplodedPairBoundary = (
351351
if (!body.startsWith(separator, index)) return false;
352352

353353
const keyStart = index + separator.length;
354-
const keyEnd = readPairKeyEnd(body, keyStart);
355-
return keyEnd > keyStart && body[keyEnd] === "=";
356-
};
357-
358-
function readPairKeyEnd(body: string, start: number): number {
359-
let index = start;
360-
let expectVarchar = true;
361-
while (index < body.length) {
362-
const varcharLength = isVarcharAt(body, index);
363-
if (varcharLength > 0) {
364-
index += varcharLength;
365-
expectVarchar = false;
366-
continue;
367-
}
368-
if (body[index] !== ".") break;
369-
if (expectVarchar || isVarcharAt(body, index + 1) < 1) break;
370-
index++;
371-
expectVarchar = true;
354+
for (let i = keyStart; i < body.length; i++) {
355+
if (body[i] === "=") return i > keyStart;
356+
if (body.startsWith(separator, i)) return false;
372357
}
373-
return index;
374-
}
358+
return false;
359+
};
375360

376361
/**
377362
* Yields candidate readings of a single value string under non-exploded

packages/uri-template/src/template/template.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,35 @@ test("Template#match — unnamed minLength must allow current var to consume one
129129
"matcher should reach the binding with x consuming one part",
130130
);
131131
});
132+
133+
// Regression for PR #758 review item 36: `expand` encodes associative keys
134+
// with the full unreserved/reserved set, but `isExplodedPairBoundary` in
135+
// match.ts uses RFC 6570 varname rules to detect the next key. Keys that are
136+
// valid in URIs but outside the varname class (e.g. containing `-` or `~`)
137+
// expand cleanly yet fail to round-trip.
138+
test("Template#match — associative keys with non-varname characters round-trip", () => {
139+
const template = new Template("{keys*}");
140+
const uri = template.expand({ keys: { a: "1", "b-c": "2" } });
141+
equal(uri, "a=1,b-c=2");
142+
const m = template.match(uri);
143+
ok(m != null, "matcher returned null for a round-trippable exploded URI");
144+
deepEqual(m, { keys: { a: "1", "b-c": "2" } });
145+
});
146+
147+
// Regression for PR #758 review item 37: the *wrong.json* fixtures for
148+
// "Invalid characters in literals" store double-escaped sequences such as
149+
// "\\u0000", which JSON-decode to 6-character backslash strings rather than
150+
// the 1-byte control characters they claim to test. The parser does reject
151+
// actual CTL bytes — this test pins that behavior directly, so future
152+
// refactors cannot silently regress on the CTL branch.
153+
test("Template — actual control-character literals throw InvalidLiteralError", () => {
154+
for (const codePoint of [0x00, 0x01, 0x09, 0x0a, 0x0d, 0x1f, 0x7f]) {
155+
throws(
156+
() => new Template(String.fromCodePoint(codePoint)),
157+
InvalidLiteralError,
158+
`code point 0x${
159+
codePoint.toString(16).padStart(2, "0")
160+
} should be rejected`,
161+
);
162+
}
163+
});

0 commit comments

Comments
 (0)