diff --git a/docs/0002-spike.md b/docs/0002-spike.md index 63ea6d4..56477ee 100644 --- a/docs/0002-spike.md +++ b/docs/0002-spike.md @@ -36,7 +36,12 @@ All 427 upstream tests across the monorepo's 41 test files still pass. The only An earlier full-spec parse appeared to hang; investigation showed the concatenated test file had grown to 32 GB — the concatenation shell loop's `*.cddl` glob had matched the output file itself, so `cat` appended the file to itself forever. The parser was never hanging. Any future "parser hangs on big input" claim from this spike should be read in that light: it doesn't. +## A fourth upstream bug, found generating wire-mesh's `core/room` domain + +`readString()` in the lexer sliced a text-string literal's raw source between its two quotes with no escape processing at all — a backslash written to escape a quote, or itself, stayed in the token's `Literal` unchanged rather than being consumed. Per RFC 8610's SESC production, `\X` inside a CDDL text string means "the literal character X", not "a literal backslash followed by X". A `.regexp` rule whose pattern needs an escaped backslash (`\\.`, `\\+`) therefore parsed with an extra literal backslash still attached, which cddl.js's emitter then re-embedded into a generated `new RegExp(...)` call, double-escaping it and silently changing what the pattern matches — confirmed directly with `namespacedDomainIdSchema` (already shipped, already affected) and the new `dm-room-path` rule (`[0-9a-f]{64}\\+[0-9a-f]{64}`), both fixed by the same one-line change to `readString()`: consume the backslash and append only the escaped character, instead of copying both through unchanged. Fixed at `docs/0002-spike.md`'s own source of truth — the vendored fork — with matching lexer and end-to-end round-trip test coverage; tracked upstream at [webdriverio/cddl#91](https://github.com/webdriverio/cddl/issues/91) / [#92](https://github.com/webdriverio/cddl/pull/92). + ## Consequences - Task order for the build (see the repository README's chosen approach) stands: upstream PR with the patch plus tests for the new operators first; vendored fork as fallback; wire-mesh spec promotes its `.cbor` relationships as each position becomes supported. - The array-member operator bug goes into the upstream PR too (or a sibling PR), since `[ bstr .size 3, bstr ]` crashing is a plain upstream defect independent of `.cbor`. +- The string-literal escape bug above follows the same pattern: reported and fixed upstream first, vendored into this fork in the meantime, since it blocks every existing and future `.regexp` rule whose pattern needs an escaped backslash from validating correctly at runtime. diff --git a/test/regexp-escapes.test.ts b/test/regexp-escapes.test.ts new file mode 100644 index 0000000..be47a12 --- /dev/null +++ b/test/regexp-escapes.test.ts @@ -0,0 +1,64 @@ +// Regression coverage for a real lexer bug: readString() used to return a CDDL text-string literal's raw, unescaped source slice, so a `.regexp` pattern containing an escaped backslash (`\\.`, `\\+`) carried an extra literal backslash all the way into the emitted `new RegExp(...)` call, silently changing what the pattern matches (e.g. `\\+` reads as "one or more backslashes" instead of a literal `+`). Caught against wire-mesh's real `namespaced-domain-id` rule, which has shipped with this bug since before this project existed -- these tests pin both that rule and the DM-room-path shape that surfaced it against wire-mesh's real fixture spec, so a regression here is caught immediately rather than rediscovered by hand in a downstream consumer. + +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { describe, expect, it } from "vitest"; +import { emitModule } from "../src/emitter.ts"; +import { parse } from "../vendor/cddl/dist/index.js"; + +function parseAndEmit(cddlSource: string): string { + const dir = mkdtempSync(join(tmpdir(), "cddl-regexp-escape-")); + const path = join(dir, "spec.cddl"); + writeFileSync(path, cddlSource); + const parsed = parse(path); + return emitModule(parsed); +} + +describe("a CDDL .regexp literal's escaped backslash survives to the emitted RegExp unchanged", () => { + it("a rule pairing two 64-hex components with an escaped literal '+' matches a real pair", async () => { + const source = + 'dm-room-path = tstr .regexp "[0-9a-f]{64}\\\\+[0-9a-f]{64}"\n'; + const generatedDir = mkdtempSync(join(tmpdir(), "cddl-regexp-escape-out-")); + const outPath = join(generatedDir, "generated.ts"); + writeFileSync(outPath, parseAndEmit(source)); + const imported: unknown = await import(pathToFileURL(outPath).href); + if ( + typeof imported !== "object" || + imported === null || + !("dmRoomPathSchema" in imported) + ) { + throw new Error("generated module is missing dmRoomPathSchema"); + } + const schema = imported.dmRoomPathSchema as { + safeParse: (value: unknown) => { success: boolean }; + }; + const deviceIdHexLength = 64; // matches the {64} quantifier in the pattern under test + const a = "a".repeat(deviceIdHexLength); + const b = "b".repeat(deviceIdHexLength); + expect(schema.safeParse(`${a}+${b}`).success).toBe(true); + expect(schema.safeParse(`${a}\\+${b}`).success).toBe(false); + }); + + it("wire-mesh's real namespaced-domain-id rule matches a genuine dotted registrant domain", async () => { + const source = + 'namespaced-domain-id = tstr .regexp "[a-z0-9]([a-z0-9-]*[a-z0-9])?(\\\\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+/[A-Za-z0-9_.-]+"\n'; + const generatedDir = mkdtempSync(join(tmpdir(), "cddl-regexp-escape-out-")); + const outPath = join(generatedDir, "generated.ts"); + writeFileSync(outPath, parseAndEmit(source)); + const imported: unknown = await import(pathToFileURL(outPath).href); + if ( + typeof imported !== "object" || + imported === null || + !("namespacedDomainIdSchema" in imported) + ) { + throw new Error("generated module is missing namespacedDomainIdSchema"); + } + const schema = imported.namespacedDomainIdSchema as { + safeParse: (value: unknown) => { success: boolean }; + }; + expect(schema.safeParse("exadev.io/agent-comms").success).toBe(true); + expect(schema.safeParse("not-a-domain").success).toBe(false); + }); +}); diff --git a/vendor/cddl/README.md b/vendor/cddl/README.md index 94b8dfe..923b0fa 100644 --- a/vendor/cddl/README.md +++ b/vendor/cddl/README.md @@ -1,12 +1,13 @@ # vendor/cddl/ -A vendored copy of [webdriverio/cddl](https://github.com/webdriverio/cddl)'s `packages/cddl/src` (the CDDL parser this project's [foundation decision](../../docs/0001-foundation.md) is built on), MIT-licensed, unmodified except for three upstream fixes cddl.js needs that aren't released yet: +A vendored copy of [webdriverio/cddl](https://github.com/webdriverio/cddl)'s `packages/cddl/src` (the CDDL parser this project's [foundation decision](../../docs/0001-foundation.md) is built on), MIT-licensed, unmodified except for four upstream fixes cddl.js needs that aren't released yet: - `fix: operators on array members no longer crash the parser` -- [webdriverio/cddl#88](https://github.com/webdriverio/cddl/pull/88), needs a maintainer to sign the project's EasyCLA before it can merge. - `feat: add .cbor and .cborseq control operators` -- stacked on the above, tracking [webdriverio/cddl#89](https://github.com/webdriverio/cddl/issues/89), PR at [Mearman/cddl#1](https://github.com/Mearman/cddl/pull/1). - `fix: ? occurrence indicator no longer parses as unbounded` -- stacked on the above two, tracking [webdriverio/cddl#90](https://github.com/webdriverio/cddl/issues/90), PR at [Mearman/cddl#2](https://github.com/Mearman/cddl/pull/2). `?` and `*` shared the same default upper bound (`Infinity`), so an arrow-syntax entry keyed by a rule reference (`? cose-header-alg => int`) and the generic open-map-tail pattern (`* tstr => any`) parsed with an identical occurrence, making them indistinguishable -- found while generating Zod schemas against wire-mesh's real `cose-token-headers` rule. +- `fix: resolve backslash-escaped characters in string literals` -- tracking [webdriverio/cddl#91](https://github.com/webdriverio/cddl/issues/91), PR at [Mearman/cddl#92](https://github.com/webdriverio/cddl/pull/92) (branch `fix/string-literal-escape-sequences` on the fork). `readString()` returned a text-string literal's raw source slice with no RFC 8610 SESC escape processing, so a `.regexp` value carrying an escaped backslash (`\\.`, `\\+`) kept an extra literal backslash all the way into the emitted `new RegExp(...)` call -- found generating wire-mesh's `dm-room-path` rule, and confirmed to already affect the already-shipped `namespaced-domain-id` rule too. -All three are recorded in full, with their own test coverage, in [docs/0002-spike.md](../../docs/0002-spike.md) and the commits on [Mearman/cddl](https://github.com/Mearman/cddl) (`fix/array-member-operators`, `feat/cbor-operator`, `fix/optional-occurrence-defaults-to-one`). +All four are recorded in full, with their own test coverage, in [docs/0002-spike.md](../../docs/0002-spike.md) and the commits on [Mearman/cddl](https://github.com/Mearman/cddl) (`fix/array-member-operators`, `feat/cbor-operator`, `fix/optional-occurrence-defaults-to-one`, `fix/string-literal-escape-sequences`). ## Why vendored, not a dependency @@ -14,7 +15,7 @@ cddl.js's foundation decision commits to attempting the fix upstream first, with ## Removing this once upstream ships -Once webdriverio/cddl#88 and the `.cbor` PR both merge and a release goes out: +Once webdriverio/cddl#88, the `.cbor` PR, the `?` occurrence-indicator fix, and #92 all merge and a release goes out: 1. Delete this directory. 2. Add `cddl` as an ordinary npm dependency instead. diff --git a/vendor/cddl/src/lexer.ts b/vendor/cddl/src/lexer.ts index b3cd311..1df1da8 100644 --- a/vendor/cddl/src/lexer.ts +++ b/vendor/cddl/src/lexer.ts @@ -190,14 +190,35 @@ export default class Lexer { } private readString (): string { - const position = this.position + let result = '' this.readChar() // eat " while (this.ch && String.fromCharCode(this.ch) !== Tokens.QUOT) { - this.readChar() // eat any character until " + // RFC 8610 SESC: a backslash escapes the single character that + // follows it, representing that character literally -- CDDL text + // strings have no JSON-style named escapes (\n, \t, \uXXXX and + // so on), the backslash exists only so a quote or a backslash + // itself can appear inside the literal. Consuming the escaped + // character here, rather than leaving both characters in the + // output verbatim, is what makes `\\+` in source read back as a + // single literal backslash followed by a plus instead of two + // backslashes -- the previous raw-slice implementation left the + // escape unresolved, so a caller re-embedding the value (e.g. + // building a RegExp source string) double-escaped it. + if (String.fromCharCode(this.ch) === '\\') { + this.readChar() // eat the backslash + if (this.ch) { + result += String.fromCharCode(this.ch) + this.readChar() // eat the escaped character + } + continue + } + + result += String.fromCharCode(this.ch) + this.readChar() } - return this.input.slice(position + 1, this.position).trim() + return result.trim() } private readNumberOrFloat (): string {