diff --git a/json/lexers/yaml-lexer/CONFORMANCE.md b/json/lexers/yaml-lexer/CONFORMANCE.md index 5dd1ce0..76aa1ac 100644 --- a/json/lexers/yaml-lexer/CONFORMANCE.md +++ b/json/lexers/yaml-lexer/CONFORMANCE.md @@ -4,12 +4,17 @@ Baseline measured against the vendored [YAML Test Suite](testdata/yaml-test-suit (revision `da267a5c`, 351 files → **406 cases**), by `TestConformanceYAML`. ``` -accept + token stream matches the expected JSON 204 / 272 (75%) +accept + token stream matches the expected JSON 226 / 272 (83%) invalid document rejected 85 / 94 (90%) recorded only (no single-root JSON equivalent) 63 -known divergences (conformanceXFail) 54 +known divergences (conformanceXFail) 32 + of which out of scope by design 14 ``` +Resolved-scalar mapping keys landed (22 cases): a key carrying an explicit-key marker, +a tag, an anchor or an alias now resolves to the string it denotes. See §"Resolved keys" +below for what remains. + ## What is being measured `YL` reads YAML **as JSON**, so the suite's `json` field — the JSON a document is @@ -19,7 +24,8 @@ streams are identical. That framing matters for reading the numbers: a case `YL` fails is not necessarily a YAML bug. It may be a construct JSON cannot express, in which case being unable to lex -it is correct behavior. The 54 divergences below are split accordingly. +it is correct behavior. The 32 xfail entries below are split accordingly — 14 of them are the design boundary +rather than work. Three buckets, mirroring the JSON suite's `y_`/`n_`/`i_` split: @@ -42,8 +48,8 @@ over the same suite — "is the loaded data equal to the `json` field" — which question we ask. Its denominator is 401 (308 valid + 93 invalid). Our comparable slice is 366 cases (272 with a single-root JSON expectation + 94 invalid), -scoring **289 → 79%**. Counting the 40 cases with no JSON equivalent as misses instead — -a stricter reading — gives **289/406 → 71%**. So we sit somewhere in **71–79%** depending +scoring **311 → 85%**. Counting the 40 cases with no JSON equivalent as misses instead — +a stricter reading — gives **311/406 → 77%**. So we sit somewhere in **77–85%** depending on how the out-of-scope cases are counted. | implementation | JSON comparison | @@ -52,9 +58,9 @@ on how the out-of-scope cases are counted. | JS yaml | 92% | | Perl YAML::PP | 92% | | Haskell HsYAML | 91% | +| **`YL` (this lexer)** | **77–85%** | | Python ruamel.yaml | 78% | | Perl YAML::PP+libyaml | 78% | -| **`YL` (this lexer)** | **71–79%** | | JS js-yaml | 76% | | Ruby psych | 76% | | Go go-yaml | 76% | @@ -75,42 +81,47 @@ Read that carefully rather than as a ranking — three things make it not apples over-permissive accepts and the 3 parse failures — are goccy's behavior, not ours, and we cannot beat it without pre-validating ourselves. -The useful takeaway: **we are in the same band as the mainstream loaders** (PyYAML, psych, -go-yaml, ruamel, js-yaml) and clearly behind the four leaders. And the single biggest gap is -ours to close: fixing resolved-scalar keys (group 1 below, 23 cases) would put the -comparable slice at roughly **85%**, above every implementation in the middle band. +The useful takeaway: **we are now above the mainstream loaders** (PyYAML 75%, psych 76%, +go-yaml 76%, ruamel 78%) and below the four leaders. Resolved-scalar keys were the single +biggest gap and are now closed, which is what moved 79% → 85%. -## The 54 divergences +## The 32 remaining entries -### 1. Non-scalar mapping keys — 23 cases +### Resolved keys — done (22 cases) -`YL` requires an object key to be a scalar, because JSON keys are strings. Most of -these, however, are keys that *resolve* to a string: +A JSON object key is a string, so `YL` only accepts a key denoting a scalar. YAML lets a key +carry node properties and indirection that JSON has no place for but that do not stop it being +a string: ```yaml -top3: &node3 - *alias1 : scalar3 # the suite expects {"scalar1": "scalar3"} +? explicit key : v # MappingKeyNode +!!str key : v # TagNode +&anchor key : v # AnchorNode +*alias : v # AliasNode -- resolves through the anchor table ``` -An alias, an anchored scalar (`&a key : v`) or a tagged one (`!!str key : v`) is -rejected today, though the resolved key is an ordinary string and the suite's own JSON -shows it as such. **This is a feature gap, not a scope boundary**, and it is the -largest single group. +and combinations (`? !!str &a foo`). These were rejected as "complex". `walk.go:resolveKey` +now peels each wrapper until a scalar is reached, which is exactly what the suite's own JSON +equivalent shows for these documents. An anchor written on a *key* is registered too, so a +later `*a` resolves to it. -Genuinely out of scope within this group: explicit complex keys (`? [a, b] : c`), -which JSON cannot express at all. +What stays rejected: a key that really denotes a sequence or mapping. Note that written out +directly (`? [a, b] : v`) goccy refuses the document before we see it — our own `ErrComplexKey` +is reached only through indirection, an alias naming a collection. -### 2. Multiple documents — 14 cases +### 1. Multiple documents — 14 cases (out of scope by design) A JSON token stream has one root, so a `%YAML`/`---` multi-document stream is rejected. -Out of scope until the ND-JSON work on the roadmap lands, at which point these become -an NDJSON mode rather than an error. +`YL`'s model is structurally single-document; this is an acknowledged boundary, not a +defect, and these 14 are listed in the xfail table only so the suite stays green and their +verdict cannot drift unnoticed. Counting divergences we might actually act on, the number +is **40, not 54**. (Note these are cases where the *expectation* is still single-root — a directive plus one document. Streams whose expectation is genuinely several JSON values fall into the recorded bucket instead.) -### 3. Invalid documents we accept — 9 cases +### 2. Invalid documents we accept — 9 cases The most valuable group: we are **more permissive than YAML allows**. @@ -130,47 +141,65 @@ All are inherited from the underlying `goccy/go-yaml` parser rather than introdu our walk — flow collections, comment placement and tabs. Each needs confirming against goccy upstream before deciding whether to report it there or pre-validate ourselves. -### 4. Accepted, but the token stream differs — 5 cases +### 3. Accepted, but the token stream differs — 6 cases ``` -565N Construct Binary (!!binary tag) +565N Construct Binary (!!binary tag) L24T/1 Trailing line of spaces -LE5A Spec Example 7.24. Flow Nodes -S4JQ Spec Example 6.28. Non-Specific Tags -UGM3 Spec Example 2.27. Invoice +LE5A Spec Example 7.24. Flow Nodes +S4JQ Spec Example 6.28. Non-Specific Tags +UGM3 Spec Example 2.27. Invoice +RR7F Mixed Block Mapping (see below -- not ours) ``` -Scalar-resolution divergences. The subtlest group, because nothing errors — the -document is accepted and the values are simply not the ones the suite expects. +The first five are scalar-resolution divergences, and the subtlest group: nothing errors, +the document is accepted and the values are simply not the ones the suite expects. + +`RR7F` is a **defect in the fixture**. Its YAML is `a: 4.2 / ? d / : 23`; its own event +stream (`+MAP =VAL :a =VAL :4.2 =VAL :d =VAL :23`) and its canonical `dump` both order the +keys `a, d`, but its `json` field writes `d` first. The suite compares loaded data as +unordered maps, so nothing there ever checked its JSON text against its own tree. We keep +key order — a JSON token stream is ordered, and so is our model — so we match the event +stream and diverge from the JSON text. + +Reported as [yaml/yaml-test-suite#179](https://github.com/yaml/yaml-test-suite/issues/179). +That repository appears largely unmaintained, so this entry is **permanent, not pending** — +nothing is waiting on it. If the fixture is ever corrected the xfail will report itself as +an unexpected pass, which is the signal to remove it. -### 5. Valid documents the parser rejects — 3 cases +### 4. Valid documents the parser rejects — 3 cases -`goccy` fails to parse these at all (`4MUZ/2` flow mapping with the colon on the next -line; two `DK95` tab cases). The fix is upstream, or a pre-pass of our own. +`goccy` fails to parse these at all, so the fix is upstream (or a pre-pass of our own): + +- `4MUZ/2` (`{foo\n: bar}`) and `VJP3/1` (a flow mapping spread over five lines) are the + same defect twice — inside a **flow** mapping a line break between the key and its `:` + is legal, but the parser applies the block-context rule that they share a line; +- `DK95/4` (`foo: 1\n\t\nbar: 2`) is a line holding only a tab between two entries. ## Divergences by suite tag Which YAML *features* are involved (a case carries several tags, so these overlap): ``` -spec 22 mapping 21 tag 16 explicit-key 12 flow 10 sequence 10 -directive 8 alias 7 comment 7 unknown-tag 7 whitespace 7 error 6 -header 6 indent 6 anchor 5 1.3-err 4 double 4 … +spec 14 tag 11 directive 8 flow 7 error 6 header 6 sequence 6 +indent 5 unknown-tag 5 whitespace 5 double 4 alias 3 comment 3 … ``` +`mapping`, `explicit-key` and `anchor` have dropped out of the top of this list entirely — +that is the resolved-keys work showing up. + ## Reading this as a work list In rough order of value per unit of effort: -1. **Resolved-scalar keys** (group 1) — 23 cases, one coherent feature: resolve an - alias/anchor/tag on the key side and use the resulting scalar. Takes the comparable - slice from 79% to **85%**, above every implementation in the matrix's middle band. - Adding groups 3 and 4 on top would reach **89%**, within reach of the leaders. -2. **Over-permissiveness** (group 3) — 9 cases, and the only group where we accept - documents we should reject. Worth confirming against goccy first. -3. **Scalar resolution** (group 4) — 5 cases, each needing individual reading. -4. **Multi-document** (group 2) — 14 cases, blocked on the ND-JSON decision. -5. **Upstream parse failures** (group 5) — 3 cases, not ours to fix directly. +1. ~~**Resolved-scalar keys**~~ — **done**, 22 cases, 79% → 85%. +2. **Over-permissiveness** (group 2) — 9 cases, and the only group where we accept + documents we should reject. All goccy's behaviour; confirm upstream before working + around them (see `PROPOSALS-go-openapi.md` in the goccy checkout). +3. **Scalar resolution** (group 3) — 5 cases, each needing individual reading. Closing + these and group 2 would reach roughly **89%**, within reach of the leaders. +4. ~~**Multi-document** (group 1)~~ — not on the list: out of scope by design (see above). +5. **Upstream parse failures** (group 4) — 3 cases, not ours to fix directly. ## Maintaining this diff --git a/json/lexers/yaml-lexer/DESIGN.md b/json/lexers/yaml-lexer/DESIGN.md index cedd206..2f1a290 100644 --- a/json/lexers/yaml-lexer/DESIGN.md +++ b/json/lexers/yaml-lexer/DESIGN.md @@ -66,6 +66,17 @@ a non-issue. them. Object = `{ key value key value }`, array = `[ value value ]`. (Unlike `L`, this is not an option — a verbatim/separator mode would only matter to a round-tripping consumer, which `YL` explicitly is not.) + + This is a decision, not a limitation, and it has been revisited: goccy exposes every + separator we would need — `MappingValueNode.Start` is the `:` of each pair, and + `SequenceEntryNode.Start` the `-` of each block item — so emitting them would be easy. + It is declined because a separator mode is error-prone for no gain: consumers index by + the *value* tokens (JSON pointer, position, colour by kind), and YAML's separators do not + map onto JSON's anyway. A block `-` has no JSON counterpart, so it would need either a + new delimiter kind in the shared `token` package — a JSON-side change for a YAML-only + need — or a `Comma` token whose `String()` reports the wrong character. Neither is worth + it. (For the record, goccy also does not expose the `,` between the pairs of a *flow* + mapping at all, so even a separator mode could not be complete.) - **`IndentLevel` matches `L`'s convention.** Openers and interior tokens sit at the container's level; a **closing** `}` / `]` reports the *enclosing* level (the level it returns to), because `L` pops the container before emitting the closer. `YL` mirrors that diff --git a/json/lexers/yaml-lexer/README.md b/json/lexers/yaml-lexer/README.md index afd0023..07ff4d8 100644 --- a/json/lexers/yaml-lexer/README.md +++ b/json/lexers/yaml-lexer/README.md @@ -75,8 +75,14 @@ for each is in **[DESIGN.md](DESIGN.md)**; in short: - **Positions on expanded content.** `Offset` / `Line` / `Column` for tokens produced by expanding an alias or a merge key point at the anchor **definition** site (expansion re-walks the original node), not the usage site. -- **Block-style closers have no position.** goccy records no position for block `}` / `]` - (only flow-style closers carry one), so they report `0/0/0`. +- **Block-style delimiters share a position with their contents.** A block collection has no + `{` / `[` characters to point at, so its opening and closing delimiters report the span of + what they enclose — the first and last token inside. Order positions by *non-decreasing*, + not strictly increasing. Flow-style delimiters report their own real characters. +- **`Offset` is not the same quantity as `L.Offset`.** `YL.Offset` is the 0-based byte offset + at which the token *starts*, so `src[Offset()]` is its first byte. The JSON lexer's `L.Offset` + is a consumption cursor that points *past* the token it just returned. Do not assume code + moving between the two lexers can keep the same arithmetic. - **Invalid UTF-8 in strings.** The JSON lexer `L` rejects unescaped control characters, but does *not* validate UTF-8 well-formedness — it passes malformed high bytes (e.g. a lone `0xA2`) through raw. goccy instead replaces them with U+FFFD. This is the one place the two diff --git a/json/lexers/yaml-lexer/bom_test.go b/json/lexers/yaml-lexer/bom_test.go new file mode 100644 index 0000000..acbeef6 --- /dev/null +++ b/json/lexers/yaml-lexer/bom_test.go @@ -0,0 +1,168 @@ +package lexer_test + +import ( + "strconv" + "strings" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + jsonlexer "github.com/go-openapi/core/json/lexers/default-lexer" + "github.com/go-openapi/core/json/lexers/token" + yamllexer "github.com/go-openapi/core/json/lexers/yaml-lexer" +) + +// A leading UTF-8 byte order mark. +// +// YAML 1.2 allows a document to be prefixed by a BOM, and it is not content. goccy does not +// strip it, and the consequence is not a dirty value but a different parse: the mark becomes the +// first character of the first token, so "{}" comes back as the SCALAR "{}" instead of +// an empty mapping. YL therefore strips it before parsing (walk.go:stripBOM). +// +// Found by the fuzzer as a JSON-subset divergence — L consumes a leading BOM, so the two lexers +// disagreed on documents that are valid to both. + +const bomPrefix = "\uFEFF" + +func TestLeadingBOMDoesNotChangeTheParse(t *testing.T) { + cases := []struct { + name string + body string + }{ + {name: "number", body: "0"}, + {name: "number after a tab", body: "\t0"}, + {name: "empty flow mapping", body: "{}"}, + {name: "flow sequence", body: "[1]"}, + {name: "flow mapping", body: "{a: 1}"}, + {name: "block mapping", body: "a: 1\n"}, + {name: "block sequence", body: "- 1\n- 2\n"}, + {name: "quoted scalar", body: `"x"`}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + want := lexKinds(t, tc.body) + got := lexKinds(t, bomPrefix+tc.body) + + assert.Equal( + t, + want, + got, + "a leading BOM is a document prefix, not content: it must not change the token stream", + ) + }) + } +} + +// TestLeadingBOMMatchesTheJSONLexer is the invariant the fuzzer checks, pinned directly: for a +// document both lexers accept, they must produce the same stream — BOM or no BOM. +func TestLeadingBOMMatchesTheJSONLexer(t *testing.T) { + for _, src := range []string{ + bomPrefix + "\t0", + bomPrefix + "0", + bomPrefix + "{}", + bomPrefix + "[1]", + bomPrefix + `{"a": 1}`, + bomPrefix + `"x"`, + } { + t.Run(src, func(t *testing.T) { + jl := jsonlexer.NewWithBytes([]byte(src)) + var want []string + for tok := range jl.Tokens() { + if tok.Kind() != token.EOF { + want = append(want, tok.Kind().String()+"("+string(tok.Value())+")") + } + } + require.NoError( + t, + jl.Err(), + "the JSON lexer must accept this for the comparison to mean anything", + ) + + assert.Equal(t, want, lexKinds(t, src)) + }) + } +} + +// TestBOMOnlyAtTheStart pins the boundary: U+FEFF anywhere else is an ordinary character. +func TestBOMOnlyAtTheStart(t *testing.T) { + t.Run("inside a scalar", func(t *testing.T) { + assert.Equal( + t, + []string{"delimiter()", "key(a)", "string(" + bomPrefix + "x)", "delimiter()"}, + lexKinds(t, "a: "+bomPrefix+"x\n"), + ) + }) + + t.Run("doubled: the second is content", func(t *testing.T) { + got := lexKinds(t, bomPrefix+bomPrefix+"0") + assert.Equal(t, []string{"string(" + bomPrefix + "0)"}, got, + "only ONE leading mark is a prefix; the second belongs to the scalar") + }) +} + +// lexKinds renders YL's stream as "kind(value)" strings. +func lexKinds(t *testing.T, src string) []string { + t.Helper() + + l := yamllexer.NewWithBytes([]byte(src)) + var out []string + for tok := range l.Tokens() { + if tok.Kind() == token.EOF { + continue + } + out = append(out, tok.Kind().String()+"("+string(tok.Value())+")") + } + require.NoErrorf(t, l.Err(), "must lex: %q", src) + + return out +} + +// TestOffsetAddressesTheSource pins that Offset is a 0-based index into the caller's bytes: the +// token's first byte is src[Offset()]. goccy counts from 1, so the walk converts; the property +// asserted here is the one consumers rely on (slicing the source), not the conversion. +// +// Note this deliberately does NOT match L.Offset, which is a consumption cursor pointing past the +// token it returned. See the doc on YL.Offset. +func TestOffsetAddressesTheSource(t *testing.T) { + for _, src := range []string{ + "abc: 1\n", + "a:\n - 10\n - twenty\n", + "{a: 1, bb: 22}\n", + "\ufeffabc: 1\n", // with a BOM, offsets stay on the caller's bytes + "# lead\nk: v\n", // goccy #856: its own offset is short by one per comment line + "# c1\n# c2\nk: v\n", + "a: 1 # trailing\nb: 2\n", + "a:\n # inner\n b: 2\n", + "é: 1\nb: 2\n", // multi-byte runes: goccy's offset counts runes, not bytes + "k: héllo wörld\nb: 2\n", + "ключ: значение\nb: 2\n", + "k: \U0001F600\nlonger: 2\n", // outside the BMP: 4 bytes, one column + "# ünïcode\nk: v\n", // both defects at once + } { + t.Run(strconv.Quote(src), func(t *testing.T) { + for _, got := range lexPositions(t, src) { + val := got.val + if val == "" { + continue // delimiters carry no text to find + } + + require.LessOrEqual(t, got.off, uint64(len(src)), + "offset past end of input for %q", val) + assert.Truef(t, strings.HasPrefix(src[got.off:], val) || + strings.HasPrefix(src[got.off:], `"`+val) || + strings.HasPrefix(src[got.off:], "'"+val), + "src[%d:] should start with %q, got %q", got.off, val, truncate(src[got.off:])) + } + }) + } +} + +func truncate(s string) string { + if len(s) > 12 { + return s[:12] + "..." + } + + return s +} diff --git a/json/lexers/yaml-lexer/conformance_xfail_test.go b/json/lexers/yaml-lexer/conformance_xfail_test.go index 36a2029..9a25850 100644 --- a/json/lexers/yaml-lexer/conformance_xfail_test.go +++ b/json/lexers/yaml-lexer/conformance_xfail_test.go @@ -7,45 +7,15 @@ package lexer_test // against NEW regressions. An entry that starts passing is reported as an error ("remove from xfail"), so the list // cannot rot into an excuse. // -// Baseline: 204/272 documents with a JSON equivalent are accepted AND lex to that JSON; 85/94 invalid documents are -// rejected. The 54 entries below group into five causes, in descending order of how much work they represent. +// Baseline: 226/272 documents with a JSON equivalent are accepted AND lex to that JSON; 85/94 invalid documents are +// rejected. The 32 entries below group into four causes; 14 of them (multiple documents) are a design boundary +// rather than work. // // See CONFORMANCE.md for the full analysis. func conformanceXFail() map[string]string { return map[string]string{ // --------------------------------------------------------------------------------------------------- - // 1. Non-scalar mapping keys (23). YL requires an object key to be a scalar, since JSON keys are - // strings. But most of these are keys that RESOLVE to a string -- an alias (`*a : v`), an anchored or - // tagged scalar (`&a k : v`, `!!str k : v`) -- and the suite's own JSON expectation shows the resolved - // string as the key. Supporting them is a real feature gap, not a scope boundary. The genuinely - // out-of-scope ones are complex keys (`? [a, b] : c`), which JSON cannot express at all. - // --------------------------------------------------------------------------------------------------- - "26DV": "non-scalar mapping key", - "2SXE": "non-scalar mapping key", - "2XXW": "non-scalar mapping key", - "5WE3": "non-scalar mapping key", - "74H7": "non-scalar mapping key", - "7BMT": "non-scalar mapping key", - "7FWL": "non-scalar mapping key", - "7W2P": "non-scalar mapping key", - "A2M4": "non-scalar mapping key", - "CN3R": "non-scalar mapping key", - "CT4Q": "non-scalar mapping key", - "E76Z": "non-scalar mapping key", - "GH63": "non-scalar mapping key", - "HMQ5": "non-scalar mapping key", - "JTV5": "non-scalar mapping key", - "L94M": "non-scalar mapping key", - "RR7F": "non-scalar mapping key", - "S9E8": "non-scalar mapping key", - "U3XV": "non-scalar mapping key", - "WZ62": "non-scalar mapping key", - "X8DW": "non-scalar mapping key", - "ZH7C": "non-scalar mapping key", - "ZWK4": "non-scalar mapping key", - - // --------------------------------------------------------------------------------------------------- - // 2. Multiple documents (14). A JSON token stream has one root, so YL rejects a multi-document stream. + // 1. Multiple documents (14). A JSON token stream has one root, so YL rejects a multi-document stream. // The suite's json field for these holds several JSON values in sequence. Out of scope until the // ND-JSON work lands (see the lexers README roadmap), at which point they become an NDJSON mode. // --------------------------------------------------------------------------------------------------- @@ -65,7 +35,7 @@ func conformanceXFail() map[string]string { "ZYU8": "multiple documents", // --------------------------------------------------------------------------------------------------- - // 3. Invalid documents we ACCEPT (9). Real conformance bugs, and the most valuable group: we are more + // 2. Invalid documents we ACCEPT (9). Real conformance bugs, and the most valuable group: we are more // permissive than YAML allows, mostly around flow collections, comments and tabs. Inherited from the // underlying goccy parser rather than introduced by our walk, so each needs to be confirmed against // goccy upstream before we decide whether to pre-validate ourselves. @@ -81,10 +51,16 @@ func conformanceXFail() map[string]string { "YJV2": "we accept an invalid document", // --------------------------------------------------------------------------------------------------- - // 4. Accepted, but the token stream differs from the expected JSON (5). Semantic divergences in how a - // scalar is resolved -- tags (!!binary, !!str), trailing whitespace, flow nodes. Each needs reading - // individually; they are the subtlest of the five groups because nothing errors. + // 3. Accepted, but the token stream differs from the expected JSON (6). Semantic divergences in how a + // scalar is resolved -- tags (!!binary, !!str), trailing whitespace, flow nodes. + // + // RR7F is not one of them: it is a defect in the FIXTURE. Its yaml is "a: 4.2 / ? d / : 23", and its own + // event stream and canonical dump both order the keys a, d -- but its json field writes d first. The + // suite compares loaded data as unordered maps, so nothing there ever checked its json text against its + // own tree. We keep key order (a JSON token stream is ordered, and so is our model), so we match the + // event stream and diverge from the json text. Reported upstream rather than worked around. // --------------------------------------------------------------------------------------------------- + "RR7F": "fixture defect (yaml-test-suite#179): its json contradicts its own event stream", "565N": "token stream differs from the expected JSON", "L24T/1": "token stream differs from the expected JSON", "LE5A": "token stream differs from the expected JSON", @@ -92,7 +68,7 @@ func conformanceXFail() map[string]string { "UGM3": "token stream differs from the expected JSON", // --------------------------------------------------------------------------------------------------- - // 5. Valid documents rejected by the underlying parser (3). goccy fails to parse these at all, so the + // 4. Valid documents rejected by the underlying parser (3). goccy fails to parse these at all, so the // fix is upstream (or a pre-pass of our own). // --------------------------------------------------------------------------------------------------- "4MUZ/2": "goccy fails to parse a valid document", diff --git a/json/lexers/yaml-lexer/fuzz_test.go b/json/lexers/yaml-lexer/fuzz_test.go index 3f159ec..d731f89 100644 --- a/json/lexers/yaml-lexer/fuzz_test.go +++ b/json/lexers/yaml-lexer/fuzz_test.go @@ -182,6 +182,14 @@ func FuzzYL(f *testing.F) { "e: &b\n a: 1\n <<: *b\n c: 3\n", "e: &b\n <<: [*b]\n", "a: &x\n - *x\n", + // a leading UTF-8 BOM is a document prefix, not content. goccy does not strip it, so it + // used to become the first character of the first token and change the parse outright: + // "{}" came back as a SCALAR. Caught by the JSON-subset differential, since L does + // consume a leading BOM. + "\uFEFF\t0", + "\uFEFF{}", + "\uFEFF[1]", + "\uFEFFa: 1\n", } for _, s := range seeds { f.Add([]byte(s)) @@ -230,6 +238,26 @@ func FuzzYL(f *testing.F) { } } + // Positions stay inside the caller's buffer. Offset is DERIVED from goccy's line/column + // (see walk.go:byteOffset) rather than taken from its Position.Offset, so it is arithmetic + // of ours over adversarial input -- exactly what a fuzzer should be pointed at. A caller + // slicing the source with it must never be handed an out-of-range index. + posl := yamllexer.NewWithBytes(data, opts...) + n := 0 + for tok := range posl.Tokens() { + if n++; n > budget { + break + } + if off := posl.Offset(); off > uint64(len(data)) { + t.Fatalf("offset %d out of range for a %d-byte input (token %v)", + off, len(data), tok.Kind()) + } + if posl.Line() < 1 || posl.Column() < 1 { + t.Fatalf("non-positive position L%dC%d (token %v)", + posl.Line(), posl.Column(), tok.Kind()) + } + } + // JSON-subset differential (only for valid UTF-8, and only when both accept). // On INVALID UTF-8 inside a string the two libraries legitimately differ: the JSON // lexer passes the raw bytes through, while goccy replaces them with U+FFFD. That is diff --git a/json/lexers/yaml-lexer/lexer.go b/json/lexers/yaml-lexer/lexer.go index abd43a8..5e58972 100644 --- a/json/lexers/yaml-lexer/lexer.go +++ b/json/lexers/yaml-lexer/lexer.go @@ -37,6 +37,16 @@ type YL struct { built bool // whether toks has been built for the current input err error // sticky lexer error state + // lineStarts holds the byte offset of each source line and lineASCII whether that line is + // free of multi-byte runes, together turning a (line, column) pair back into a byte offset. + // Build-time transient (nil outside build). See walk.go:byteOffset. + lineStarts []int + lineASCII []bool + + // bomBytes is 3 when the input opened with a UTF-8 byte order mark, which is stripped + // before parsing and added back to reported positions. See walk.go:build. + bomBytes int + // snapshot of the most-recently-returned token, feeding Offset/IndentLevel/Line/Column cur emit @@ -139,14 +149,24 @@ func (l *YL) Tokens() iter.Seq[token.T] { } } -// Offset yields the byte offset of the most-recently-returned token in the input. +// Offset yields the 0-based byte offset at which the most-recently-returned token STARTS, as an +// index into the bytes the caller supplied: src[Offset()] is the token's first byte, so the source +// text can be sliced with it directly. +// +// Note this is not the same quantity as [lexer.L.Offset], despite the shared name: L reports how +// many bytes it has consumed so far, a cursor that points PAST the token it just returned. YL walks +// a parsed document rather than a byte cursor, so it can report the token's own start, which is the +// more useful of the two. Code moving between the two lexers must not assume they agree. // // Two YAML-specific caveats apply to all of Offset/Line/Column: // - tokens produced by expanding an alias (*x) or a merge key (<<) report the position of // the anchor DEFINITION, not the alias/merge usage site (expansion re-walks the original // node); -// - a block-style closing delimiter (} or ]) reports 0: goccy records no position for it -// (only flow-style {…}/[…] closers carry one). +// - a block collection has no delimiter characters, so its opening and closing delimiters +// report the span of what they enclose — the first and last token inside it — and share +// that neighbour's position rather than sitting strictly before/after it. Order by +// non-decreasing position, not strictly increasing. Flow-style {…}/[…] delimiters report +// their own real characters. func (l *YL) Offset() uint64 { return l.cur.off } @@ -206,6 +226,7 @@ func (l *YL) reset() { l.toks = l.toks[:0] l.pos = 0 l.built = false + l.bomBytes = 0 l.cur = emit{} l.err = nil diff --git a/json/lexers/yaml-lexer/position_test.go b/json/lexers/yaml-lexer/position_test.go new file mode 100644 index 0000000..0c9eed8 --- /dev/null +++ b/json/lexers/yaml-lexer/position_test.go @@ -0,0 +1,304 @@ +package lexer_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/core/json/lexers/token" + yamllexer "github.com/go-openapi/core/json/lexers/yaml-lexer" +) + +// Source positions (Line/Column/Offset). +// +// Block collections have no "{" / "[" / "}" / "]" characters, so goccy has no token to point a +// container delimiter at: it hands us the first entry's separator (the ":" of the first pair, the +// "-" of the first item) as the node's Start, and nothing at all as its End. Used literally that +// put the opening delimiter AFTER the key it precedes and the closing one at line 0 column 0 — +// so a consumer laying tokens out by position had to sort them back and discard the closers. +// +// walk.go:patchBlockSpan gives them the span of what they enclose instead. These tests pin that, +// and pin that FLOW collections keep their real delimiter positions untouched. + +// posTok is one token with the position YL reported for it. +type posTok struct { + kind token.Kind + val string + line int + col int + off uint64 +} + +func (p posTok) String() string { + return fmt.Sprintf("L%dC%d %v(%q)", p.line, p.col, p.kind, p.val) +} + +func lexPositions(t *testing.T, src string, opts ...yamllexer.Option) []posTok { + t.Helper() + + l := yamllexer.NewWithBytes([]byte(src), opts...) + var out []posTok + for tok := range l.Tokens() { + if tok.Kind() == token.EOF { + break + } + out = append(out, posTok{ + kind: tok.Kind(), val: string(tok.Value()), + line: l.Line(), col: l.Column(), off: l.Offset(), + }) + } + require.NoError(t, l.Err(), "input must lex: %q", src) + + return out +} + +// TestBlockContainerSpan pins where a block collection's delimiters land: the opening one on the +// first token it encloses, the closing one on the last. +func TestBlockContainerSpan(t *testing.T) { + t.Run("nested block mappings", func(t *testing.T) { + got := lexPositions(t, "info:\n title: Petstore\n version: 1\n") + + assert.Equal(t, []string{ + `L1C1 delimiter("")`, // root mapping opens on `info`, not on its colon + `L1C1 key("info")`, + `L2C3 delimiter("")`, // /info opens on `title`, not on the colon a line below + `L2C3 key("title")`, + `L2C10 string("Petstore")`, + `L3C3 key("version")`, + `L3C12 number("1")`, + `L3C12 delimiter("")`, // /info closes on its last token + `L3C12 delimiter("")`, // root closes there too + }, render(got)) + }) + + t.Run("block sequence", func(t *testing.T) { + got := lexPositions(t, "schemes:\n - http\n - https\n") + + assert.Equal(t, []string{ + `L1C1 delimiter("")`, + `L1C1 key("schemes")`, + `L2C5 delimiter("")`, // opens on `http`, not on the "-" before it + `L2C5 string("http")`, + `L3C5 string("https")`, + `L3C5 delimiter("")`, + `L3C5 delimiter("")`, + }, render(got)) + }) +} + +// TestFlowContainerPositionsUnchanged pins the other side of the fix: a flow collection HAS real +// delimiter characters, and those positions must survive untouched. +func TestFlowContainerPositionsUnchanged(t *testing.T) { + got := lexPositions(t, "tags: [a, b]\n") + + assert.Equal(t, []string{ + `L1C1 delimiter("")`, + `L1C1 key("tags")`, + `L1C7 delimiter("")`, // the real "[" + `L1C8 string("a")`, + `L1C11 string("b")`, + `L1C12 delimiter("")`, // the real "]" + `L1C12 delimiter("")`, + }, render(got)) + + t.Run("flow mapping", func(t *testing.T) { + got := lexPositions(t, "m: {a: 1}\n") + assert.Equal(t, `L1C4 delimiter("")`, render(got)[2], "the real \"{\"") + assert.Equal(t, `L1C9 delimiter("")`, render(got)[5], "the real \"}\"") + }) +} + +// TestNoTokenHasAZeroPosition pins that every emitted token carries a usable position. Line and +// column are 1-based, so 0 is not a position: a consumer that sees one can only discard the token. +func TestNoTokenHasAZeroPosition(t *testing.T) { + for _, src := range []string{ + "info:\n title: Petstore\n", + "schemes:\n - http\n - https\n", + "tags: [a, b]\n", + "m: {a: 1}\n", + "a:\n b:\n c:\n - 1\n - x: 2\n", + "- 1\n- 2\n", + "root: null\n", + "empty: {}\n", + "emptyseq: []\n", + } { + t.Run(src, func(t *testing.T) { + for _, p := range lexPositions(t, src) { + assert.Positivef(t, p.line, "%s: line must be 1-based", p) + assert.Positivef(t, p.col, "%s: column must be 1-based", p) + } + }) + } +} + +// TestPositionsAreNonDecreasing is the property the TUI actually needs: laying tokens out by +// position must not require sorting them first. +// +// Non-decreasing rather than strictly increasing: in block style a container delimiter has no +// character of its own, so it shares its neighbour's position. +func TestPositionsAreNonDecreasing(t *testing.T) { + for _, src := range []string{ + "info:\n title: Petstore\n version: 1\n", + "schemes:\n - http\n - https\n", + "tags: [a, b]\n", + "openapi: 3.0.0\ninfo:\n title: t\npaths:\n /p:\n get:\n responses:\n '200':\n description: ok\n", + "- a\n- b: 1\n c: 2\n- - nested\n - seq\n", + } { + t.Run(src, func(t *testing.T) { + assertNonDecreasing(t, lexPositions(t, src)) + }) + } +} + +// TestAliasPositionsStillPointAtTheAnchor documents the one case that stays out of order, so the +// behaviour is a recorded decision rather than an accident. +// +// Alias-expanded tokens deliberately report the ANCHOR DEFINITION site (see walkAlias), which is +// earlier in the document than the alias. A consumer highlighting an alias-bearing document +// therefore still cannot assume monotonicity. +func TestAliasPositionsStillPointAtTheAnchor(t *testing.T) { + got := lexPositions(t, "defs: &d\n p: 1\nuse: *d\n") + + var backwards bool + prev := posTok{} + for _, p := range got { + if p.line < prev.line || (p.line == prev.line && p.col < prev.col) { + backwards = true + } + prev = p + } + + assert.True(t, backwards, + "expansion still reports the anchor site: if this ever stops being true, the walkAlias "+ + "doc and the TUI's ordering assumptions both need revisiting") +} + +// TestOffsetTracksPosition pins that Offset moved with Line/Column — it comes from the same +// source position, so a container delimiter used to report offset 0 as well. +func TestOffsetTracksPosition(t *testing.T) { + src := "info:\n title: Petstore\n" + got := lexPositions(t, src) + + for _, p := range got { + assert.LessOrEqualf(t, p.off, uint64(len(src)), "%s: offset must be inside the input", p) + } + // the closing delimiters sit on the last token, not at offset 0 + last := got[len(got)-1] + assert.Positivef(t, last.off, "%s: a closing delimiter must carry a real offset", last) +} + +func render(toks []posTok) []string { + out := make([]string, 0, len(toks)) + for _, p := range toks { + out = append(out, p.String()) + } + + return out +} + +func assertNonDecreasing(t *testing.T, toks []posTok) { + t.Helper() + + prev := posTok{line: 1, col: 1} + for i, p := range toks { + ok := p.line > prev.line || (p.line == prev.line && p.col >= prev.col) + assert.Truef(t, ok, "token %d goes backwards: %s after %s\nstream: %s", + i, p, prev, strings.Join(render(toks), " ")) + prev = p + } +} + +// TestPositionsOverTheWholeSuite runs the position invariants over every document the vendored +// YAML Test Suite says is valid — 400-odd real-world shapes rather than the handful above. +// +// Alias-bearing documents are skipped for the ordering check only: expansion reports the anchor +// site by design (see TestAliasPositionsStillPointAtTheAnchor). Everything else must carry a +// 1-based position and never go backwards. +func TestPositionsOverTheWholeSuite(t *testing.T) { + var checked, skipped int + + for _, c := range loadSuite(t) { + if c.fail { + continue + } + + l := yamllexer.NewWithBytes([]byte(c.yaml), + yamllexer.WithMaxContainerStack(512), yamllexer.WithMaxTokens(1<<20)) + + var toks []posTok + for tok := range l.Tokens() { + if tok.Kind() == token.EOF { + break + } + toks = append(toks, posTok{ + kind: tok.Kind(), val: string(tok.Value()), + line: l.Line(), col: l.Column(), off: l.Offset(), + }) + } + if l.Err() != nil || len(toks) == 0 { + continue // rejected or empty: nothing to say about its positions + } + + t.Run(c.label(), func(t *testing.T) { + for _, p := range toks { + require.Positivef(t, p.line, "%s: line must be 1-based\nsrc=%q", p, c.yaml) + require.Positivef(t, p.col, "%s: column must be 1-based\nsrc=%q", p, c.yaml) + } + + // an alias re-walks the anchored node, so its tokens report the definition site + if strings.ContainsAny(c.yaml, "*&") { + skipped++ + + return + } + checked++ + assertNonDecreasing(t, toks) + }) + } + + t.Logf("ordering checked on %d documents, %d skipped for aliases", checked, skipped) +} + +// TestBOMPositions pins that stripping a leading byte order mark does not shift the positions +// YL reports: the parser sees the input without the mark, so every position it hands back has to +// be put back on the caller's coordinates (walk.go:stripBOM). +func TestBOMPositions(t *testing.T) { + const bom = "\uFEFF" + + t.Run("offsets stay on the caller's bytes", func(t *testing.T) { + withBOM := lexPositions(t, bom+"{a: 1}\n") + without := lexPositions(t, "{a: 1}\n") + require.Len(t, withBOM, len(without)) + + for i := range withBOM { + assert.Equalf(t, without[i].off+uint64(len(bom)), withBOM[i].off, + "token %d: offset must account for the stripped mark", i) + } + }) + + t.Run("columns shift on line 1 only", func(t *testing.T) { + withBOM := lexPositions(t, bom+"a: 1\nb: 2\n") + without := lexPositions(t, "a: 1\nb: 2\n") + require.Len(t, withBOM, len(without)) + + for i := range withBOM { + assert.Equalf(t, without[i].line, withBOM[i].line, "token %d: line must not move", i) + + want := without[i].col + if without[i].line == 1 { + want++ // the mark is one character, and it is on line 1 + } + assert.Equalf(t, want, withBOM[i].col, "token %d: column", i) + } + }) + + t.Run("a mark that is not at the start is content", func(t *testing.T) { + // only a LEADING mark is a document prefix; anywhere else U+FEFF is an ordinary character + got := lexPositions(t, "a: "+bom+"x\n") + require.Len(t, got, 4) + assert.Equal(t, bom+"x", got[2].val) + }) +} diff --git a/json/lexers/yaml-lexer/resolved_key_test.go b/json/lexers/yaml-lexer/resolved_key_test.go new file mode 100644 index 0000000..17bcf8c --- /dev/null +++ b/json/lexers/yaml-lexer/resolved_key_test.go @@ -0,0 +1,141 @@ +package lexer_test + +import ( + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" + + "github.com/go-openapi/core/json/lexers/token" + yamllexer "github.com/go-openapi/core/json/lexers/yaml-lexer" +) + +// Mapping keys that RESOLVE to a scalar. +// +// A JSON object key is a string, so YL only accepts a key that denotes a scalar. YAML lets a key +// carry node properties and indirection that JSON has no place for but that do not stop it being +// a string — an explicit key ("? k"), a tag ("!!str k"), an anchor ("&a k"), an alias ("*a"), and +// combinations. Those were rejected as "complex" until walk.go:resolveKey peeled them; 22 of the +// YAML Test Suite's documents turn on it. +// +// A key that really is a sequence or a mapping stays rejected: that one JSON cannot express. + +func keysOf(t *testing.T, src string) []string { + t.Helper() + + l := yamllexer.NewWithBytes([]byte(src)) + var keys []string + for tok := range l.Tokens() { + if tok.Kind() == token.Key { + keys = append(keys, string(tok.Value())) + } + } + require.NoErrorf(t, l.Err(), "must lex: %q", src) + + return keys +} + +func TestResolvedScalarKeys(t *testing.T) { + cases := []struct { + name string + src string + want []string + }{ + {name: "plain", src: "k: v\n", want: []string{"k"}}, + {name: "explicit key", src: "? k\n: v\n", want: []string{"k"}}, + {name: "explicit key, flow value", src: "? k\n: [1, 2]\n", want: []string{"k"}}, + {name: "tagged key", src: "!!str k: v\n", want: []string{"k"}}, + {name: "tagged integer key", src: "!!int 3: v\n", want: []string{"3"}}, + {name: "anchored key", src: "&a k: v\n", want: []string{"k"}}, + {name: "alias key", src: "d: &a name\nm:\n *a : v\n", want: []string{"d", "m", "name"}}, + {name: "explicit + tag", src: "? !!str k\n: v\n", want: []string{"k"}}, + {name: "explicit + anchor", src: "? &a k\n: v\n", want: []string{"k"}}, + {name: "tag + anchor", src: "!!str &a k: v\n", want: []string{"k"}}, + { + name: "anchor defined on a key is reusable", + src: "&a k: v\nm:\n *a : w\n", + want: []string{"k", "m", "k"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, keysOf(t, tc.src)) + }) + } +} + +// TestComplexKeysStillRejected pins the boundary: a key that denotes a collection is outside the +// JSON data model however many wrappers are peeled off it. +// +// Which error you get depends on how the key is written, and that is worth recording. Written +// out directly, goccy refuses the document before we see it ("found an invalid key for this +// map"), so ErrComplexKey is unreachable for those. It is reached through INDIRECTION — an alias +// naming a collection — where goccy is happy and only our JSON-shaped model objects. +func TestComplexKeysStillRejected(t *testing.T) { + t.Run("rejected upstream, before our walk", func(t *testing.T) { + for _, tc := range []struct { + name string + src string + }{ + {name: "sequence key", src: "? [a, b]\n: v\n"}, + {name: "mapping key", src: "? {a: 1}\n: v\n"}, + {name: "anchored sequence key", src: "? &a [x]\n: v\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + l := yamllexer.NewWithBytes([]byte(tc.src)) + for range l.Tokens() { //nolint:revive // draining is the point + } + require.Error(t, l.Err(), "a collection key must be rejected") + assert.NotErrorIs(t, l.Err(), yamllexer.ErrComplexKey, + "goccy rejects these at parse time, so our own guard never runs") + }) + } + }) + + t.Run("rejected by our model", func(t *testing.T) { + // goccy parses this happily: the key is an alias, and only resolving it reveals a sequence + l := yamllexer.NewWithBytes([]byte("d: &a [x]\nm:\n *a : v\n")) + for range l.Tokens() { //nolint:revive // draining is the point + } + require.Error(t, l.Err()) + assert.ErrorIs(t, l.Err(), yamllexer.ErrComplexKey) + }) +} + +// TestResolvedKeyPosition pins where a resolved key reports itself: on the scalar it denotes, so +// the position always holds the text the token carries. +func TestResolvedKeyPosition(t *testing.T) { + // "&a k" -- the key token says "k", so it must point at k (column 4), not at the anchor + // one mapping: open, key, value, close + got := lexPositions(t, "&a k: v\n") + require.Len(t, got, 4) + assert.Equal(t, `L1C4 key("k")`, got[1].String(), + "the token says \"k\", so it must point at k -- not at the anchor that precedes it") + + t.Run("an alias key points at the anchor definition", func(t *testing.T) { + // consistent with alias-expanded VALUES (see walkAlias): the position holds the text + got := lexPositions(t, "d: &a name\nm:\n *a : v\n") + var key posTok + for _, p := range got { + if p.kind == token.Key && p.val == "name" { + key = p + } + } + assert.Equal( + t, + 1, + key.line, + "reports the anchor site (line 1), not the alias site (line 3)", + ) + }) +} + +// TestResolvedKeyCycleIsRejected pins that a key aliasing its own definition terminates rather +// than recursing: the guard is shared with the alias-cycle machinery. +func TestResolvedKeyCycleIsRejected(t *testing.T) { + l := yamllexer.NewWithBytes([]byte("&a *a : v\n")) + for range l.Tokens() { //nolint:revive // draining is the point + } + assert.Error(t, l.Err(), "a self-referential key must not recurse") +} diff --git a/json/lexers/yaml-lexer/testdata/conformance_recorded.golden b/json/lexers/yaml-lexer/testdata/conformance_recorded.golden index 09fa185..36244b7 100644 --- a/json/lexers/yaml-lexer/testdata/conformance_recorded.golden +++ b/json/lexers/yaml-lexer/testdata/conformance_recorded.golden @@ -6,7 +6,7 @@ 4MUZ/1 accept Flow mapping colon on line after key 5TYM reject Spec Example 6.21. Local Tag Prefix 6BFJ reject Mapping, key and flow sequence item anchors -6M2F reject Aliases in Explicit Block Mapping +6M2F accept Aliases in Explicit Block Mapping 6PBE reject Zero-indented sequences in explicit mapping keys 6WLZ reject Spec Example 6.18. Primary Tag Handle [1.3] 6XDY accept Two document start markers @@ -21,7 +21,7 @@ 9WXW reject Spec Example 6.18. Primary Tag Handle AVM7 accept Empty Stream CFD4 reject Empty implicit key in single pair flow sequences -DFF7 reject Spec Example 7.16. Flow Mapping Entries +DFF7 accept Spec Example 7.16. Flow Mapping Entries FH7J reject Tags on Empty Scalars FRK4 reject Spec Example 7.3. Completely Empty Flow Nodes HWV9 accept Document-end marker diff --git a/json/lexers/yaml-lexer/testdata/fuzz/FuzzYL/509dec0845c6303e b/json/lexers/yaml-lexer/testdata/fuzz/FuzzYL/509dec0845c6303e new file mode 100644 index 0000000..339e206 --- /dev/null +++ b/json/lexers/yaml-lexer/testdata/fuzz/FuzzYL/509dec0845c6303e @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\ufeff\t0") diff --git a/json/lexers/yaml-lexer/walk.go b/json/lexers/yaml-lexer/walk.go index 6f433d0..e5f1e19 100644 --- a/json/lexers/yaml-lexer/walk.go +++ b/json/lexers/yaml-lexer/walk.go @@ -1,7 +1,9 @@ package lexer import ( + "bytes" "strings" + "unicode/utf8" codes "github.com/go-openapi/core/json/lexers/error-codes" "github.com/go-openapi/core/json/lexers/token" @@ -20,13 +22,18 @@ func (l *YL) build() { return } - f, err := safeParse(l.data) + src := l.stripBOM() + + f, err := safeParse(src) if err != nil { l.err = err return } + l.indexLines(src) + defer func() { l.lineStarts, l.lineASCII = nil, nil }() + switch len(f.Docs) { case 0: // empty input: a YAML document with no content resolves to null. @@ -50,6 +57,102 @@ func (l *YL) build() { l.merging = nil } +// indexLines records where each line of src starts and whether it is pure ASCII, so a +// (line, column) pair can be turned back into a byte offset. Built once per build and dropped +// when it ends; the ASCII flag rides along for free, since this pass already reads every byte. +func (l *YL) indexLines(src []byte) { + lines := bytes.Count(src, newline) + 1 + l.lineStarts = make([]int, 1, lines) // line 1 starts at 0 + l.lineASCII = make([]bool, 0, lines) + + ascii := true + for i, b := range src { + switch { + case b == '\n': + l.lineASCII = append(l.lineASCII, ascii) + l.lineStarts = append(l.lineStarts, i+1) + ascii = true + case b >= utf8.RuneSelf: + ascii = false + } + } + l.lineASCII = append(l.lineASCII, ascii) // the last line, which need not be terminated +} + +// byteOffset converts a goccy (line, column) pair into a byte offset into the parsed source. +// +// We do NOT use goccy's own Position.Offset, for two independent reasons: +// +// - it is a 1-based RUNE index, not a byte offset (goccy's scanner holds the source as +// []rune), so it cannot address the bytes of any document containing non-ASCII text, and +// drifts further with every multi-byte character before the token; +// - it loses one more per comment line preceding the token (goccy #856). +// +// Those two errors have opposite signs, so on an ASCII document with exactly one comment they +// cancel and pos.Offset looks right. It is not: patching either one alone leaves the other. +// +// Line and Column are correct under both defects, so deriving from them is the only thing that +// can be made correct without a fix upstream. See PROPOSALS-go-openapi.md §1b in the goccy +// checkout for the reproducers and what we intend to ask for. +func (l *YL) byteOffset(line, col int) uint64 { + if line < 1 || line > len(l.lineStarts) || col < 1 { + return 0 // no position we can honestly claim; put() reports the start of the input + } + + src := l.data[l.bomBytes:] + start := l.lineStarts[line-1] + end := len(src) + if line < len(l.lineStarts) { + end = l.lineStarts[line] + } + + // A column counts characters, not bytes, so in general we have to walk the line. On a pure + // ASCII line -- nearly all of them, in nearly every document -- the two coincide and we can + // index straight to it. + i := start + col - 1 + if !l.lineASCII[line-1] { + i = start + for range col - 1 { + if i >= end { + break + } + i++ + for i < end && src[i]&0xC0 == 0x80 { // skip the rune's continuation bytes + i++ + } + } + } + + return uint64(min(i, end)) //nolint:gosec // an index into a []byte is never negative +} + +// newline is the line separator scanned for by indexLines. +var newline = []byte{'\n'} //nolint:gochecknoglobals // immutable 1-byte constant + +// bomUTF8 is the UTF-8 encoding of U+FEFF, the byte order mark. +var bomUTF8 = []byte{0xEF, 0xBB, 0xBF} //nolint:gochecknoglobals // immutable 3-byte constant + +// stripBOM returns the input with a leading UTF-8 byte order mark removed, recording its width +// so reported positions can be put back on the caller's coordinates. +// +// YAML 1.2 allows a document to be prefixed by a BOM and it is not part of the content, but +// goccy does not strip it: it becomes the first character of the first token, which does not +// merely dirty a value -- it changes the parse. A BOM followed by "{}" comes back as the +// SCALAR "{}" rather than an empty mapping, and a BOM followed by "a: 1" yields the key +// "a". So the mark has to go before the parser sees it, not be trimmed off afterwards. +// +// The JSON lexer L consumes a leading BOM the same way (see input.CheckBOM), which is what the +// FuzzYL JSON-subset differential compares against. +func (l *YL) stripBOM() []byte { + if !bytes.HasPrefix(l.data, bomUTF8) { + return l.data + } + + l.bomBytes = len(bomUTF8) + + return l.data[len(bomUTF8):] +} + // safeParse runs the goccy parser, converting a recoverable panic into an error so a // malformed input never crashes a YL consumer. (A stack overflow from pathological nesting // is fatal and cannot be recovered — bound it with WithMaxContainerStack instead.) @@ -100,11 +203,13 @@ func (l *YL) walkValue(node ast.Node, lvl int) { case *ast.MappingNode: l.walkMapping(n, lvl) case *ast.MappingValueNode: - // a single-entry mapping that goccy did not wrap in a MappingNode + // a single-entry mapping that goccy did not wrap in a MappingNode; only block style + // produces one, and its token is the pair's ":" (see patchBlockSpan) l.walkMappingEntries( []*ast.MappingValueNode{n}, posOf(n.GetToken()), posOf(n.GetToken()), + false, lvl, ) case *ast.SequenceNode: @@ -129,7 +234,7 @@ func (l *YL) walkValue(node ast.Node, lvl int) { // walkMapping emits an object: { key : value , … }. func (l *YL) walkMapping(n *ast.MappingNode, lvl int) { - l.walkMappingEntries(n.Values, posOf(n.Start), posOf(n.End), lvl) + l.walkMappingEntries(n.Values, posOf(n.Start), posOf(n.End), n.IsFlowStyle, lvl) } // walkMappingEntries emits the object delimiters around a list of key/value entries. @@ -137,6 +242,7 @@ func (l *YL) walkMapping(n *ast.MappingNode, lvl int) { func (l *YL) walkMappingEntries( values []*ast.MappingValueNode, start, end *yamltoken.Position, + flow bool, lvl int, ) { inner := lvl + 1 @@ -144,6 +250,7 @@ func (l *YL) walkMappingEntries( return } + openIdx := len(l.toks) l.emitDelim(token.OpeningBracket, start, inner) if hasMergeKey(values) { // D5: resolve "<<" merge keys with RFC precedence (explicit keys win, earlier merges @@ -167,6 +274,10 @@ func (l *YL) walkMappingEntries( // The closing delimiter reports the ENCLOSING level (the level it returns to), matching // the JSON lexer L, which pops the container before emitting the closer. l.emitDelim(token.ClosingBracket, end, lvl) + + if !flow { + l.patchBlockSpan(openIdx) + } } // entryKV is a resolved mapping member (after merge-key expansion). @@ -196,7 +307,7 @@ func (l *YL) resolveMapping(values []*ast.MappingValueNode) []entryKV { if _, isMerge := mv.Key.(*ast.MergeKeyNode); isMerge { continue } - if ks, ok := keyString(mv.Key); ok { + if ks, _, ok := l.scalarKeyResolved(mv.Key); ok { explicit[ks] = true } } @@ -231,7 +342,7 @@ func (l *YL) resolveMapping(values []*ast.MappingValueNode) []entryKV { continue } - ks, ok := keyString(mv.Key) + ks, _, ok := l.scalarKeyResolved(mv.Key) if !ok { l.err = ErrComplexKey @@ -295,20 +406,106 @@ func (l *YL) mergeEntries(src ast.Node) []*ast.MappingValueNode { // keyString returns the string form of a scalar mapping key (matching emitKey's token value), // used for merge de-duplication. ok is false for a non-scalar (complex) key. -func keyString(key ast.MapKeyNode) (string, bool) { +// maxKeyUnwrap bounds the resolveKey recursion. A key can legitimately stack node properties +// ("? !!str &a foo"), but only a handful deep; the bound is what stops an alias chain that +// resolves back into itself from recursing forever. +const maxKeyUnwrap = 16 + +// scalarKey reduces a mapping key to the scalar it denotes, returning its text and the token to +// report a position at. +// +// A JSON object key is a string, so YL only accepts a key that resolves to a scalar. "Resolves" +// is doing real work, because YAML lets a key carry node properties and indirection that the +// JSON data model has no place for but that do not stop it being a string: +// +// ? explicit key : v an explicit key (MappingKeyNode) wrapping any node +// !!str key : v a tagged key (TagNode) +// &anchor key : v an anchored key (AnchorNode) +// *alias : v an alias to a scalar defined elsewhere (AliasNode) +// +// and combinations of those ("? !!str foo"). Each wrapper is peeled until a scalar is reached; +// what comes out is an ordinary string key, which is exactly what the YAML Test Suite's own JSON +// equivalent shows for these documents. +// +// A key that resolves to a sequence or a mapping is genuinely outside the JSON data model and +// still fails with ErrComplexKey. +// +// Aliases are resolved against the anchor table, so this is a method: an alias key can only be +// resolved once the anchor it names has been walked, which the single forward pass guarantees +// (YAML requires an anchor to be defined before it is used). +func (l *YL) resolveKey(key ast.Node, depth int) (ast.Node, bool) { + if depth > maxKeyUnwrap { + return nil, false + } + switch k := key.(type) { + case *ast.MappingKeyNode: // "? key" + return l.resolveKey(k.Value, depth+1) + + case *ast.TagNode: // "!!str key" + return l.resolveKey(k.Value, depth+1) + + case *ast.AnchorNode: // "&a key" -- also registers the anchor, so a later "*a" resolves + if name := anchorName(k.Name); name != "" { + l.anchors[name] = k.Value + } + + return l.resolveKey(k.Value, depth+1) + + case *ast.AliasNode: // "*a" + name := anchorName(k.Value) + target, ok := l.anchors[name] + if !ok { + return nil, false + } + if l.expanding[name] { + return nil, false // a key aliasing its own definition + } + l.expanding[name] = true + node, resolved := l.resolveKey(target, depth+1) + delete(l.expanding, name) + + return node, resolved + + default: + return key, true + } +} + +// scalarKey is [YL.resolveKey] followed by the scalar test: it returns the key's text and the +// token whose position the Key token should report, or ok=false when the key is not a scalar. +// +// The position is the resolved scalar's own token, so the reported location always holds the +// text the token carries. For an alias that is the anchor DEFINITION site, consistent with how +// alias-expanded values are positioned (see walkAlias). +func (l *YL) scalarKeyResolved(key ast.MapKeyNode) (string, *yamltoken.Token, bool) { + node, ok := l.resolveKey(key, 0) + if !ok { + return "", nil, false + } + + return scalarKey(node) +} + +// scalarKey reads a scalar node's text and token. It does no unwrapping: callers that may see a +// wrapped key go through [YL.scalarKeyResolved]. +func scalarKey(node ast.Node) (string, *yamltoken.Token, bool) { + switch k := node.(type) { case *ast.StringNode: - return k.Value, true + return k.Value, k.Token, true case *ast.IntegerNode: - return k.Token.Value, true + return k.Token.Value, k.Token, true case *ast.FloatNode: - return k.Token.Value, true + return k.Token.Value, k.Token, true case *ast.BoolNode: - return k.Token.Value, true + return k.Token.Value, k.Token, true case *ast.NullNode: - return k.Token.Value, true + return k.Token.Value, k.Token, true + case *ast.LiteralNode: + // a block scalar used as an explicit key ("? |" ...): its text is the folded content + return k.Value.Value, k.Start, true default: - return "", false + return "", nil, false } } @@ -361,6 +558,7 @@ func (l *YL) walkSequence(n *ast.SequenceNode, lvl int) { return } + openIdx := len(l.toks) l.emitDelim(token.OpeningSquareBracket, posOf(n.Start), inner) for _, v := range n.Values { if l.err != nil { @@ -370,6 +568,10 @@ func (l *YL) walkSequence(n *ast.SequenceNode, lvl int) { } // The closing delimiter reports the ENCLOSING level, matching L (see walkMappingEntries). l.emitDelim(token.ClosingSquareBracket, posOf(n.End), lvl) + + if !n.IsFlowStyle { + l.patchBlockSpan(openIdx) + } } // walkAlias resolves an alias to its anchored value and emits its tokens inline, guarding @@ -403,23 +605,21 @@ func (l *YL) emitKey(key ast.MapKeyNode, lvl int) { return } - switch k := key.(type) { - case *ast.StringNode: - l.putValue(token.MakeWithValue(token.Key, []byte(k.Value)), posOf(k.Token), lvl) - case *ast.IntegerNode: - l.putValue(token.MakeWithValue(token.Key, []byte(k.Token.Value)), posOf(k.Token), lvl) - case *ast.FloatNode: - l.putValue(token.MakeWithValue(token.Key, []byte(k.Token.Value)), posOf(k.Token), lvl) - case *ast.BoolNode: - l.putValue(token.MakeWithValue(token.Key, []byte(k.Token.Value)), posOf(k.Token), lvl) - case *ast.NullNode: - l.putValue(token.MakeWithValue(token.Key, []byte(k.Token.Value)), posOf(k.Token), lvl) - case *ast.MergeKeyNode: - // D5 (deferred): the "<<" merge key is a later increment. + if _, isMerge := key.(*ast.MergeKeyNode); isMerge { + // a "<<" reaching here was not resolved away by resolveMapping l.err = codes.ErrInvalidToken - default: + + return + } + + text, tok, ok := l.scalarKeyResolved(key) + if !ok { l.err = ErrComplexKey + + return } + + l.putValue(token.MakeWithValue(token.Key, []byte(text)), posOf(tok), lvl) } // walkInteger emits an integer, unconverted when its spelling is already a JSON number and @@ -512,14 +712,82 @@ func (l *YL) put(tok token.T, pos *yamltoken.Position, lvl int) { } e := emit{tok: tok, lvl: lvl} - if pos != nil { - e.off = uint64(pos.Offset) //nolint:gosec // no overflow, no negative values + switch { + case pos != nil: + // derived from line/column rather than taken from pos.Offset -- see byteOffset + e.off = l.byteOffset(pos.Line, pos.Column) e.line = pos.Line e.col = pos.Column + if l.bomBytes > 0 { + // the parser saw the input without its byte order mark; put the reported position + // back on the caller's coordinates. The mark is 3 bytes and one character, all of + // it on line 1, so only that line's columns shift. + e.off += uint64(l.bomBytes) + if e.line == 1 { + e.col++ + } + } + + case len(l.toks) > 0: + // No source position: an implicit value the document does not spell out (a "key:" with + // nothing after it). It belongs where the construct that implies it left off, so it + // inherits the preceding token's position rather than reporting none. + prev := l.toks[len(l.toks)-1] + e.off, e.line, e.col = prev.off, prev.line, prev.col + + default: + // Nothing precedes it either: an empty, comment-only or header-only document, whose + // single implicit null value is the whole token stream. The start of the input is the + // only position it can honestly claim. + e.off, e.line, e.col = 0, 1, 1 } l.toks = append(l.toks, e) } +// patchBlockSpan gives a BLOCK collection's delimiters real positions, openIdx being the index +// at which its opening delimiter was appended. +// +// Block style has no "{" / "[" / "}" / "]" characters, so goccy has no token to point the +// delimiters at. It sets the node's Start to the first entry's SEPARATOR — the ":" of the first +// pair, the "-" of the first item — and its End to nil. Taken literally that is worse than +// imprecise: +// +// - the opening delimiter lands AFTER the key it precedes ("info:" reports the ":" of the +// nested "title:" a line below), so the token stream contradicts its own order and a +// consumer laying tokens out by position has to sort them back; +// - the closing delimiter gets no position at all and surfaces as line 0, column 0, which is +// not a position — lines are 1-based — so a consumer can only discard it. +// +// Instead the delimiters take the SPAN of what they enclose: the opening one reports the first +// token inside the container, the closing one the last. Both are real, in-range positions, and +// an alias-free document then reads monotonically. +// +// They are EQUAL to their neighbour's position rather than strictly before/after it, because in +// block style there is no character of their own to point at: a consumer must order by +// non-decreasing position, not strictly increasing. (An alias-BEARING document can still go +// backwards, by design — expanded tokens report the anchor definition site, see walkAlias.) +// +// The span is read back off the emitted stream rather than computed from the AST so that it +// composes with everything the walk may have done to the children — merge-key resolution, alias +// expansion, tags, nested containers — none of which the node's own tokens know about. +// +// It is a no-op for a flow collection (the caller checks), for an empty one (nothing to span, +// so goccy's own tokens stand), and when a circuit breaker cut the walk short and the indices +// no longer line up. +func (l *YL) patchBlockSpan(openIdx int) { + closeIdx := len(l.toks) - 1 + if l.err != nil || openIdx < 0 || closeIdx <= openIdx || closeIdx >= len(l.toks) { + return + } + if closeIdx == openIdx+1 { + return // empty container: no children to take a span from + } + + first, last := l.toks[openIdx+1], l.toks[closeIdx-1] + l.toks[openIdx].off, l.toks[openIdx].line, l.toks[openIdx].col = first.off, first.line, first.col + l.toks[closeIdx].off, l.toks[closeIdx].line, l.toks[closeIdx].col = last.off, last.line, last.col +} + // overContainerStack reports whether opening a container at depth would exceed the // WithMaxContainerStack circuit breaker, setting the error if so. func (l *YL) overContainerStack(depth int) bool {