From 2f16bf63946086b69346bd1c4cbfb6542cf353dd Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 4 Aug 2026 14:26:47 +0300 Subject: [PATCH 1/2] fix(compilers/openapi): keep raw timestamp and binary spelling The channel whose documented promise is verbatim preservation rewrote the two YAML scalars JSON has no type for, so a source could not be recovered from what reached the IR. A timestamp was rendered as the RFC 3339 instant it resolves to, giving `2021-1-1` a padding, a time and a zone the source never wrote. A !!binary was rendered as its decoded bytes in a JSON string, which lost the base64 spelling on every value and lost the data itself on any byte that is not valid UTF-8: `/w==` is 0xFF, and encoding/json substitutes U+FFFD for it, leaving nothing to tell it from a source that wrote U+FFFD. Both now keep the text the source wrote. The resolved form is derivable from the spelling and the spelling is not derivable from the resolved form, which is the same reasoning the ir.Value channel already applies to a timestamp; the two channels agreed on nothing here before. The decode stays as the tag check it has always been, so a scalar that does not satisfy the type it declares is still refused and what a source gets back for one is unchanged. Whether such a scalar should instead survive as text is a question about what the walk accepts rather than what it preserves, and is left open as #245. Both conformance fixtures that reach these sites were blind to this: no golden held a normalized timestamp and no !!binary reached the raw channel at all, so the corpus could not have caught the rewrite and cannot show the fix without new cases. Reverting the change reddens both. Closes #242 --- compilers/openapi/conformance_test.go | 67 +++++++- .../openapi/internal/annotation/rawjson.go | 74 ++++++--- .../annotation/rawjson_internal_test.go | 154 +++++++++++++----- ir/unmodeled.go | 11 +- .../openapi/extensions-x.golden.json | 26 ++- .../conformance/openapi/extensions-x.yaml | 10 ++ .../yaml-timestamp-scalars.golden.json | 62 ++++++- .../openapi/yaml-timestamp-scalars.yaml | 12 ++ 8 files changed, 345 insertions(+), 71 deletions(-) diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 062eb44..05ab1da 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -854,10 +854,22 @@ func assertNonScalarDefaults(t *testing.T, m *ir.Model) { // assertYAMLTimestampScalars covers a YAML 1.1 quirk: an unquoted date like // 2021-01-01 resolves to tag !!timestamp, not !!str. It must survive as the // literal string everywhere OpenAPI's JSON data model can carry one — enum, -// const, a property default, a schema-level example, and a media-type -// example — with nothing dropped or degraded to null. +// const, a property default, a schema-level example, and a media-type example — +// with nothing dropped or degraded to null. +// +// "Everywhere" spans both channels a date can land in. It used to mean only the +// ir.Value one, and the raw-JSON one went on rewriting a date to RFC 3339 with +// this fixture green (GitHub #242); assertRawPreservedDates is the other half. func assertYAMLTimestampScalars(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { - assert.Empty(t, diags, "every unquoted date converts cleanly; nothing is dropped or degraded") + // R's `not` announces the §4.7 carve-out, and that notice is the only thing + // any site here is allowed to say: a date that was dropped or degraded + // reports itself as a warning, which is what this fixture exists to catch. + for _, d := range diags { + assert.Equal(t, ir.SeverityInfo, d.Severity, + "every unquoted date converts cleanly; nothing is dropped or degraded: %+v", d) + assert.Equal(t, "openapi/validation-only-keyword", d.Code, + "the §4.7 carve-out is the only notice this fixture expects: %+v", d) + } d, ok := doc.Types[namedID("D")].(*ir.Enum) require.True(t, ok, "D stays a closed Enum of the real dates, not a union of null literals") @@ -888,6 +900,31 @@ func assertYAMLTimestampScalars(t *testing.T, doc *ir.Document, diags []ir.Diagn require.Len(t, mediaExamples, 1, "the media-type example is preserved") require.NotNil(t, mediaExamples[0].Value) assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, *mediaExamples[0].Value) + + assertRawPreservedDates(t, doc) +} + +// assertRawPreservedDates pins the source spelling of a date kept as raw JSON, +// at both sites that channel has: a vendor extension and the §4.7 +// validation-only carve-out. Each wants the text the source wrote — resolving +// the tag and re-rendering it hands the construct a padding, a time and a zone +// nobody asked for, and the resolved form cannot be read back to the spelling. +func assertRawPreservedDates(t *testing.T, doc *ir.Document) { + t.Helper() + r, ok := doc.Types[namedID("R")].(*ir.Model) + require.True(t, ok) + kept := r.Unmodeled + + for _, tc := range []struct{ key, want string }{ + {"openapi:x-effective", `"2021-1-1"`}, + {"openapi:x-window", `{"from":"2021-1-1","to":"2022-2-2"}`}, + {"openapi:not", `{"const":"2021-1-1"}`}, + } { + entry, found := kept[tc.key] + require.True(t, found, "%s is preserved; got %v", tc.key, kept) + assert.Equal(t, tc.want, string(entry.Value), + "%s keeps the date as written, not as RFC 3339 spells it", tc.key) + } } func assertConstraints(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { @@ -1672,6 +1709,30 @@ func assertExtensionsX(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { require.True(t, ok, "an operation extension lands on the operation; got %v", op.Unmodeled) assert.Equal(t, ir.ReasonVendorExtension, entry.Reason) assert.JSONEq(t, `true`, string(entry.Value)) + + assertRawPreservedBinary(t, m) +} + +// assertRawPreservedBinary pins what a !!binary extension keeps: the base64 the +// source wrote. Decoding it and storing the bytes in a JSON string lost the +// spelling on every value and lost the data itself on any byte that is not +// valid UTF-8, since encoding/json rewrites those to U+FFFD (GitHub #242). +// +// The block-form row carries the line breaks with it, because they are part of +// what the source wrote; base64.StdEncoding skips them, so a consumer resolving +// the tag reads the same bytes the decode used to store. +func assertRawPreservedBinary(t *testing.T, m *ir.Model) { + t.Helper() + for _, tc := range []struct{ key, want string }{ + {"openapi:x-blob", `"aGVsbG8="`}, + {"openapi:x-raw", `"/w=="`}, + {"openapi:x-wrapped", `"aGVs\nbG8=\n"`}, + } { + entry, found := m.Unmodeled[tc.key] + require.True(t, found, "%s is preserved; got %v", tc.key, m.Unmodeled) + assert.Equal(t, tc.want, string(entry.Value), + "%s keeps its base64 text, not the bytes it names", tc.key) + } } // assertInlineAnnotations covers the positions with no ir.Property or diff --git a/compilers/openapi/internal/annotation/rawjson.go b/compilers/openapi/internal/annotation/rawjson.go index 56c564a..9b72d92 100644 --- a/compilers/openapi/internal/annotation/rawjson.go +++ b/compilers/openapi/internal/annotation/rawjson.go @@ -110,15 +110,10 @@ func (c *rawConv) node(n *yaml.Node, depth int) (json.RawMessage, error) { // parser never hands one over — it resolves every scalar it emits, and writes // the short form even for a spelled-out `!` — but a // caller assembling nodes can, and reading n.Tag would spell such a scalar as a -// string. A tag yaml.v3 assigns no type to keeps its text rather than being -// refused, which is also what that Decode did. +// string. // -// What changed is the numeric arms, which now read the source text (GitHub #32). -// Every other arm still renders the way that Decode would have, so a timestamp -// still normalizes to RFC 3339 and a !!binary still carries its decoded bytes -// rather than its base64 spelling. Those two are lossy against the source and -// deliberately left that way: they are a different mechanism from the float64 -// rounding, and moving them belongs with its own reasoning (GitHub #242). +// A tag yaml.v3 assigns no type to keeps its text rather than being refused, +// which is also what that Decode did. func (c *rawConv) scalar(n *yaml.Node) (json.RawMessage, error) { switch n.ShortTag() { case "!!null": @@ -153,31 +148,62 @@ func (c *rawConv) scalar(n *yaml.Node) (json.RawMessage, error) { return json.RawMessage(num), nil case "!!str": return jsonString(n.Value), nil + case "!!timestamp", "!!binary": + // The two tags YAML gives a type and JSON does not. Both keep the text + // the source wrote, because the resolved form is derivable from the + // spelling and the spelling is not derivable from the resolved form + // (GitHub #242). + // + // Rendering the resolved form instead lost data both ways: a timestamp + // came back RFC 3339, so `2021-1-1` acquired a padding, a time and a + // zone the source never wrote, and a !!binary came back as its decoded + // bytes, so `/w==` — the byte 0xFF — reached the IR as the U+FFFD + // encoding/json substitutes for it, indistinguishable from a source + // that wrote U+FFFD itself. + // + // The decode stays, and stays the tag check it has always been: a + // scalar that does not satisfy the type it declares is refused exactly + // as before, so only the kept spelling moved. Whether such a scalar + // should instead survive as text — JSON can name it, and refusing costs + // the caller the whole construct — is a question about what this walk + // accepts rather than what it preserves, and is open as GitHub #245 + // rather than settled as a side effect here. + return c.verbatimTagged(n) + default: + // A tag yaml.v3 cannot resolve carries no type it could decode to, so + // its scalar comes back as the text it was written with — the behaviour + // this walk replaced, and the lossless one: refusing would drop an + // `!acme/thing` extension value the source did write. + return jsonString(n.Value), nil + } +} + +// verbatimTagged renders a scalar whose tag YAML resolves and JSON cannot name. +// Each arm only checks the tag, by the decode that used to supply the output, +// and the one return states the rule they share: what is kept is the source +// text. The two decode to different Go types and report differently — a +// timestamp names the offending text, a binary payload deliberately does not. +func (c *rawConv) verbatimTagged(n *yaml.Node) (json.RawMessage, error) { + switch tag := n.ShortTag(); tag { case "!!timestamp": - var t time.Time - if err := n.Decode(&t); err != nil { + var when time.Time + if err := n.Decode(&when); err != nil { return nil, fmt.Errorf("timestamp literal %q: %w", n.Value, err) } - // The spelling time.Time's own MarshalJSON produces. It is reproduced - // rather than called because that method reports an error for a year - // outside [0,9999], which YAML's timestamp resolution cannot produce — - // an unreachable branch is worse than an explicit format. - return jsonString(t.Format(time.RFC3339Nano)), nil case "!!binary": // yaml.v3 base64-decodes a !!binary node into a string (it rejects a - // []byte target), so decode to string and carry the bytes from there. - var raw string - if err := n.Decode(&raw); err != nil { + // []byte target), so the check decodes to string. + var decoded string + if err := n.Decode(&decoded); err != nil { return nil, fmt.Errorf("binary literal: %w", err) } - return jsonString(raw), nil default: - // A tag yaml.v3 cannot resolve carries no type it could decode to, so - // its scalar comes back as the text it was written with — the behaviour - // this walk replaced, and the lossless one: refusing would drop an - // `!acme/thing` extension value the source did write. - return jsonString(n.Value), nil + // Unreachable through scalar, which routes only the two tags above + // here. It stays so a third tag added to that arm is answered rather + // than silently read as base64. + return nil, fmt.Errorf("scalar tag %q is not kept verbatim", tag) } + return jsonString(n.Value), nil } // sequence renders a YAML sequence as a JSON array, in source order. diff --git a/compilers/openapi/internal/annotation/rawjson_internal_test.go b/compilers/openapi/internal/annotation/rawjson_internal_test.go index 80f50ed..844934e 100644 --- a/compilers/openapi/internal/annotation/rawjson_internal_test.go +++ b/compilers/openapi/internal/annotation/rawjson_internal_test.go @@ -13,9 +13,9 @@ import ( // decodeAndMarshal is the conversion RawFromNode used before GitHub #32: decode // the node into Go's JSON model, then re-marshal it. It is kept here as the -// differential oracle for TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers, -// which is the only claim about it worth making — that everything except number -// spelling came through it unchanged. +// differential oracle for TestRawFromNode_DiffersFromTheOldDecodeOnlyWhereRecorded, +// which is the only claim about it worth making — that everything came through +// it unchanged except number spelling and the rows rawDivergences names. func decodeAndMarshal(node *yaml.Node) (json.RawMessage, error) { var v any if err := node.Decode(&v); err != nil { @@ -25,8 +25,9 @@ func decodeAndMarshal(node *yaml.Node) (json.RawMessage, error) { } // throughFloat64 re-encodes raw JSON through Go's JSON model, which rounds every -// number to float64 — the one transformation the old conversion applied that the -// new walk does not. +// number to float64 — the only transformation the old conversion applied that +// the new walk does not, on a row rawDivergences does not name. A divergence is +// compared unnormalized instead, since that spelling is what it pins. // // Both sides of the comparison go through it, not just the new output: the trip // also canonicalizes how an escape is spelled (a "\ufffd" escape comes back as @@ -114,10 +115,21 @@ func TestRawFromNode_RendersEveryScalarTag(t *testing.T) { {"quoted number stays a string", `"123"`, `"123"`}, {"empty string", `""`, `""`}, {"HTML is escaped, as encoding/json does it", `"a&c"`, `"a\u003cb\u003e\u0026c"`}, - {"date normalizes to RFC 3339", "2021-1-1", `"2021-01-01T00:00:00Z"`}, - {"datetime keeps its nanoseconds", "2021-01-01T10:20:30.5Z", `"2021-01-01T10:20:30.5Z"`}, - {"binary carries decoded bytes", `!!binary aGVsbG8=`, `"hello"`}, {"out-of-float64-range plain scalar stays a string", "1e400", `"1e400"`}, + // A tag YAML has a type for and JSON does not keeps the source text + // (GitHub #242). The resolved form is derivable from the spelling; the + // spelling is not derivable from the resolved form. + {"date keeps its source spelling", "2021-1-1", `"2021-1-1"`}, + {"date keeps its padding", "2021-01-01", `"2021-01-01"`}, + {"datetime keeps its space separator", "2021-01-01 10:20:30", `"2021-01-01 10:20:30"`}, + {"datetime keeps its zone as written", "2021-01-01T10:20:30+05:00", `"2021-01-01T10:20:30+05:00"`}, + {"explicitly tagged date keeps its spelling", "!!timestamp 2021-1-1", `"2021-1-1"`}, + {"binary keeps its base64 text", `!!binary aGVsbG8=`, `"aGVsbG8="`}, + {"binary keeps a byte no UTF-8 can name", `!!binary /w==`, `"/w=="`}, + // base64.StdEncoding skips newlines, so YAML's own block spelling of a + // !!binary decodes; what it kept was the decoded form, and what it + // keeps now is the wrapped text the source wrote. + {"block-form binary keeps its line breaks", "!!binary |\n aGVs\n bG8=\n", `"aGVs\nbG8=\n"`}, // yaml.v3 resolves no type for these, so the scalar keeps the text it // was written with rather than being dropped for want of a tag. {"unresolvable double-bang tag", "!!python/object x", `"x"`}, @@ -189,6 +201,9 @@ func TestRawFromNode_RefusesWhatJSONCannotName(t *testing.T) { {"merge from an alias to a scalar", "a: &n 1\nb: {<<: *n}", "map merge requires"}, {"non-string key inside a merged mapping", "{<<: [{1: v}]}", "not a string"}, {"boolean tag on a non-boolean", "!!bool notabool", "bool literal"}, + // The tag check outlives the decode that used to supply the output: a + // scalar that does not satisfy the type it declares is still refused, + // so preserving the source spelling widened nothing (GitHub #242). {"timestamp tag on a non-date", "!!timestamp notadate", "timestamp literal"}, {"binary tag on non-base64", `!!binary "###"`, "binary literal"}, {"float tag on a binary-exponent literal", "!!float 1p4", "which is not JSON"}, @@ -303,40 +318,75 @@ func TestRawFromNode_WalksThroughADocumentNode(t *testing.T) { assert.JSONEq(t, `{"a":1}`, string(got)) } -// TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers is the equivalence +// rawDivergences is the closed list of inputs on which the walk deliberately +// disagrees with the decode it replaced, beyond how a number is spelled. Each +// row is a scalar YAML gives a type and JSON does not, kept as the text the +// source wrote rather than as the resolved form (GitHub #242). +// +// Both spellings are pinned. The new one is the preservation the change exists +// for; the old one is what keeps the row honest — without it a divergence that +// quietly stopped being one would leave this entry asserting nothing, which is +// the same hole as excluding the row outright. +var rawDivergences = map[string]struct{ old, want string }{ + "2021-1-1": {`"2021-01-01T00:00:00Z"`, `"2021-1-1"`}, + "2021-01-01 10:20:30": {`"2021-01-01T10:20:30Z"`, `"2021-01-01 10:20:30"`}, + `!!binary aGVsbG8=`: {`"hello"`, `"aGVsbG8="`}, + // The old spelling is the escape encoding/json writes for a byte no UTF-8 + // can name, which is the loss itself: 0xFF and a source that really wrote + // U+FFFD both reached the IR as this, with nothing to tell them apart. + `!!binary /w==`: {"\"\\ufffd\"", `"/w=="`}, + // A block !!binary decodes to the same bytes as the flow one above, so the + // old spelling is identical while the source text is not. + "!!binary |\n aGVs\n bG8=\n": {`"hello"`, `"aGVs\nbG8=\n"`}, +} + +// rawEquivalenceCorpus is the input set both differential tests below read. +var rawEquivalenceCorpus = []string{ + "1", "1.5", "-2", "0", "0.0", "1e10", "1.10", "0o17", "0x1f", "1_000", + "12345678901234567890123", "1.000000000000000000001", "-9223372036854775809", + "null", "true", "false", "hello", `"123"`, `""`, "1e400", `"a&c"`, + "2021-1-1", "2021-01-01T10:20:30.5Z", `!!binary aGVsbG8=`, `!!binary /w==`, + // Two timestamp spellings the old decode already reproduced exactly, so + // they are equivalence rows rather than divergences. They are here to keep + // rawDivergences tight: it must list what actually diverges, not every + // input that carries a tag JSON has no type for. + "2021-01-01T10:20:30+05:00", "2021-01-01 10:20:30", + "{}", "[]", "[1, 2, 3]", "{z: 1, a: 2, m: 3}", "{a: {b: {c: 1}}}", + "[[1], [2, [3]]]", "{a: [1, {b: 2}], c: null}", + "{<<: {p: 1}, q: 2}", "{<<: {p: 1}, p: 2}", "{<<: [{p: 1}, {p: 2}]}", + "a: &m {p: 1}\nb: {<<: *m, q: 2}", "a: &n 5\nb: *n", "a: &s [1, 2]\nb: *s", + "{k: [{n: 12345678901234567890123}, 1.10]}", + "{unicode: \"héllo→\"}", "{empty_map: {}, empty_seq: []}", + // Shapes where the walk had to reproduce a yaml.v3 rule rather than a + // JSON one. Each of these was a live regression until the rule was found. + "a: &k mykey\nb: {*k: v}", "!!python/object x", "!foo bar", "!!set x", + "! 5", `!!int "5"`, `!!str 123`, + "{<<: {}}", "{<<: [{}]}", "a: &m {p: 1}\nb: {<<: [*m, {q: 2}]}", + "{a: yes, b: no, c: on, d: off}", "{a: 1:30, b: 12:34:56}", + "|\n line1\n line2", ">\n folded text", "a: &s {x: 1}\nb: *s\nc: *s", + `{"": 1}`, `{"a\nb": 1}`, "{a: }", "{a: null, b: ~}", + // Refused by the old conversion, preserved by the walk. They carry no + // equality claim, and are here so the asymmetry is visible in a -v run + // rather than asserted in a comment. + "!!int 12345678901234567890123", "!!float 1e400", + // YAML's block spelling of a !!binary: base64.StdEncoding skips the line + // breaks, so the old decode accepted it and this is a divergence in what + // is kept rather than in what is accepted. + "!!binary |\n aGVs\n bG8=\n", +} + +// TestRawFromNode_DiffersFromTheOldDecodeOnlyWhereRecorded is the equivalence // oracle for the rewrite. Reading the two implementations cannot show they // agree; rounding the new output through float64 and demanding the old output -// back can, because that is the only transformation the old one applied. +// back can, because rounding is the only transformation the old one applied to +// a row rawDivergences does not name. // // A row the old conversion refused carries no claim — the new walk is allowed -// to be strictly more capable, and on `!!int 12345678901234567890123` it is. -func TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers(t *testing.T) { +// to be strictly more capable, and on `!!int 12345678901234567890123` and on a +// block-form `!!binary` it is. +func TestRawFromNode_DiffersFromTheOldDecodeOnlyWhereRecorded(t *testing.T) { t.Parallel() - corpus := []string{ - "1", "1.5", "-2", "0", "0.0", "1e10", "1.10", "0o17", "0x1f", "1_000", - "12345678901234567890123", "1.000000000000000000001", "-9223372036854775809", - "null", "true", "false", "hello", `"123"`, `""`, "1e400", `"a&c"`, - "2021-1-1", "2021-01-01T10:20:30.5Z", `!!binary aGVsbG8=`, `!!binary /w==`, - "{}", "[]", "[1, 2, 3]", "{z: 1, a: 2, m: 3}", "{a: {b: {c: 1}}}", - "[[1], [2, [3]]]", "{a: [1, {b: 2}], c: null}", - "{<<: {p: 1}, q: 2}", "{<<: {p: 1}, p: 2}", "{<<: [{p: 1}, {p: 2}]}", - "a: &m {p: 1}\nb: {<<: *m, q: 2}", "a: &n 5\nb: *n", "a: &s [1, 2]\nb: *s", - "{k: [{n: 12345678901234567890123}, 1.10]}", - "{unicode: \"héllo→\"}", "{empty_map: {}, empty_seq: []}", - // Shapes where the walk had to reproduce a yaml.v3 rule rather than a - // JSON one. Each of these was a live regression until the rule was found. - "a: &k mykey\nb: {*k: v}", "!!python/object x", "!foo bar", "!!set x", - "! 5", `!!int "5"`, `!!str 123`, - "{<<: {}}", "{<<: [{}]}", "a: &m {p: 1}\nb: {<<: [*m, {q: 2}]}", - "{a: yes, b: no, c: on, d: off}", "{a: 1:30, b: 12:34:56}", - "|\n line1\n line2", ">\n folded text", "a: &s {x: 1}\nb: *s\nc: *s", - `{"": 1}`, `{"a\nb": 1}`, "{a: }", "{a: null, b: ~}", - // Refused by the old conversion, preserved by the walk. They carry no - // equality claim, and are here so the asymmetry is visible in a -v run - // rather than asserted in a comment. - "!!int 12345678901234567890123", "!!float 1e400", - } - for _, src := range corpus { + for _, src := range rawEquivalenceCorpus { t.Run(src, func(t *testing.T) { t.Parallel() node := yamlNode(t, src) @@ -344,16 +394,36 @@ func TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers(t *testing.T) { want, oldErr := decodeAndMarshal(node) got, newErr := RawFromNode(node) + if recorded, diverges := rawDivergences[src]; diverges { + require.NoError(t, oldErr, "a recorded divergence must be a row the old conversion accepted") + require.NoError(t, newErr) + assert.Equal(t, recorded.old, string(want), + "the old spelling, pinned so the row cannot stop being a divergence unnoticed") + assert.Equal(t, recorded.want, string(got), "the source text the walk preserves instead") + return + } if oldErr != nil { t.Skipf("the old conversion refused this input (%v); the new walk owes it nothing", oldErr) } require.NoError(t, newErr, "the old conversion accepted this input") assert.Equal(t, throughFloat64(t, want), throughFloat64(t, got), - "the walk must differ from the decode it replaced only in how a number is spelled") + "the walk must differ from the decode it replaced only in how a number is spelled, "+ + "or in a way rawDivergences records") }) } } +// TestRawDivergences_NamesOnlyCorpusRows keeps the two halves tied together. A +// divergence naming an input the corpus does not hold is never evaluated, so it +// would sit there reading as coverage while asserting nothing at all. +func TestRawDivergences_NamesOnlyCorpusRows(t *testing.T) { + t.Parallel() + for src := range rawDivergences { + assert.Contains(t, rawEquivalenceCorpus, src, + "a recorded divergence the corpus never feeds through is dead weight") + } +} + // aliasChain builds n alias nodes each naming the next, ending on a string. A // parser reaches this shape only by anchoring an alias, and never this deep. func aliasChain(n int) *yaml.Node { @@ -388,6 +458,14 @@ func TestRawConv_RefusesNodesNoCallerShouldPass(t *testing.T) { err = c.mappingInto(map[string]json.RawMessage{}, yamlNode(t, "[1]"), 0) require.Error(t, err, "filling a mapping from a sequence is a caller bug") assert.Contains(t, err.Error(), "expected a mapping") + + // scalar routes only !!timestamp and !!binary into verbatimTagged, so no + // input reaches this arm. It answers rather than falling through to the + // base64 check, which is what a third tag added to that case would hit. + got, err = c.verbatimTagged(yamlNode(t, "plain")) + require.Error(t, err, "a tag scalar does not route here is a caller bug") + assert.Nil(t, got) + assert.Contains(t, err.Error(), `scalar tag "!!str" is not kept verbatim`) } // TestRawFromNode_BoundsAMergeChainThatNeverRevisitsANode proves the bound on diff --git a/ir/unmodeled.go b/ir/unmodeled.go index a8813ad..63e3f58 100644 --- a/ir/unmodeled.go +++ b/ir/unmodeled.go @@ -67,10 +67,13 @@ type UnmodeledEntry struct { // where JSON and YAML disagree about how to write one — .5 becomes 0.5, // 0o17 becomes 15 — while every significant digit stays (GitHub #32). // - // Two scalars YAML gives a type and JSON does not are still rewritten: a - // timestamp normalizes to RFC 3339, and a !!binary carries its decoded bytes - // rather than its base64 text, which costs ill-formed UTF-8 its identity to - // U+FFFD (GitHub #242). + // A scalar the source format types and JSON does not is kept as the text + // the source wrote, as a JSON string: a YAML timestamp stays `2021-1-1` + // rather than becoming the RFC 3339 instant it resolves to, and a + // `!!binary` keeps its base64 spelling rather than the bytes it names + // (GitHub #242). Reading such a value means resolving the tag the way its + // source format would; what this field promises is that the text is still + // there to resolve. Value RawValue `json:"value"` // Provenance locates the construct itself, which the owning node's own // provenance cannot: a validation emitter reporting on a `not` must point at diff --git a/testdata/conformance/openapi/extensions-x.golden.json b/testdata/conformance/openapi/extensions-x.golden.json index 80ab4b2..76faaad 100644 --- a/testdata/conformance/openapi/extensions-x.golden.json +++ b/testdata/conformance/openapi/extensions-x.golden.json @@ -91,6 +91,14 @@ "docs": {}, "sensitive": false, "unmodeled": { + "openapi:x-blob": { + "reason": "vendor_extension", + "value": "aGVsbG8=", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/x-blob" + } + }, "openapi:x-rate-limit": { "reason": "vendor_extension", "value": 100, @@ -98,6 +106,22 @@ "source": 0, "pointer": "/components/schemas/S/x-rate-limit" } + }, + "openapi:x-raw": { + "reason": "vendor_extension", + "value": "/w==", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/x-raw" + } + }, + "openapi:x-wrapped": { + "reason": "vendor_extension", + "value": "aGVs\nbG8=\n", + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/x-wrapped" + } } }, "provenance": { @@ -172,7 +196,7 @@ { "format": "openapi@3.1", "path": "extensions-x.yaml", - "hash": "3ee41a0e3e10e8b30944e1582b149ac0ab178a4c9dcf78cacc911f48d43d886f" + "hash": "bdd4e78c821cd4ff1e418d0036d0f44c67120aa29ecd201a8d9a2cbc5a825ae9" } ] } diff --git a/testdata/conformance/openapi/extensions-x.yaml b/testdata/conformance/openapi/extensions-x.yaml index ed3f53e..dfbcee4 100644 --- a/testdata/conformance/openapi/extensions-x.yaml +++ b/testdata/conformance/openapi/extensions-x.yaml @@ -14,5 +14,15 @@ components: S: type: object x-rate-limit: 100 + # A !!binary keeps the base64 the source wrote, not the bytes it names: + # decoded bytes in a JSON string lose the spelling, and lose the data + # outright when they are not valid UTF-8 — "/w==" is the byte 0xFF, which + # encoding/json rewrites to U+FFFD. The block form is how YAML's own spec + # writes a !!binary, and its line breaks are part of the spelling. + x-blob: !!binary "aGVsbG8=" + x-raw: !!binary "/w==" + x-wrapped: !!binary | + aGVs + bG8= properties: a: {type: string} diff --git a/testdata/conformance/openapi/yaml-timestamp-scalars.golden.json b/testdata/conformance/openapi/yaml-timestamp-scalars.golden.json index 36a7db5..1fcb1ad 100644 --- a/testdata/conformance/openapi/yaml-timestamp-scalars.golden.json +++ b/testdata/conformance/openapi/yaml-timestamp-scalars.golden.json @@ -162,6 +162,55 @@ "object": null } }, + "t/openapi/components/schemas/R": { + "kind": "model", + "id": "t/openapi/components/schemas/R", + "name": { + "source": "R", + "canonical": "r" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:not": { + "reason": "validation_only", + "value": { + "const": "2021-1-1" + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/R/not" + } + }, + "openapi:x-effective": { + "reason": "vendor_extension", + "value": "2021-1-1", + "provenance": { + "source": 0, + "pointer": "/components/schemas/R/x-effective" + } + }, + "openapi:x-window": { + "reason": "vendor_extension", + "value": { + "from": "2021-1-1", + "to": "2022-2-2" + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/R/x-window" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/R" + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/S": { "kind": "model", "id": "t/openapi/components/schemas/S", @@ -248,11 +297,22 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/validation-only-keyword", + "message": "validation-only keyword \"not\" kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/R" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "yaml-timestamp-scalars.yaml", - "hash": "e649b990382693ae848f9f4187ad9fa5889606d9aeaea03a9e3713144e4d2d17" + "hash": "b68b751a3b3278260b857b08d920d07d9e9ba0fe7c832e0db92b50ddb3db9bcf" } ] } diff --git a/testdata/conformance/openapi/yaml-timestamp-scalars.yaml b/testdata/conformance/openapi/yaml-timestamp-scalars.yaml index e4a8fb9..cea1b1a 100644 --- a/testdata/conformance/openapi/yaml-timestamp-scalars.yaml +++ b/testdata/conformance/openapi/yaml-timestamp-scalars.yaml @@ -27,3 +27,15 @@ components: format: date default: 2021-01-01 example: 2021-01-01 + # Every site above rides the Value channel. A construct kept verbatim as raw + # JSON is a second channel with the same requirement, and it is the one that + # had no corpus case — which is how it went on rewriting a date to RFC 3339 + # while the fixture named for dates stayed green. The single-digit spelling + # is the point: 2021-1-1 must stay 2021-1-1 rather than acquire the padding, + # the time and the zone the source never wrote. + R: + type: object + x-effective: 2021-1-1 + x-window: {from: 2021-1-1, to: 2022-2-2} + not: + const: 2021-1-1 From 72d25ccc6369c4d3d45b2436eb96a0bd0190139e Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 4 Aug 2026 14:44:48 +0300 Subject: [PATCH 2/2] test(compilers/openapi): pin nesting and correct the divergence notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corrections to the raw-scalar work, found reviewing the branch as it will be merged. The corpus grouped 2021-01-01 10:20:30 under a comment calling it a spelling the old decode reproduced exactly, while rawDivergences two declarations above recorded it as a divergence — the old decode rewrote it to a T and a Z. The block-form !!binary sat under a header reserved for rows the old conversion refused, which it is not. Both rows now sit with the scalars they belong to, under a comment that says which kind is which and why the equivalence ones earn their place. Nesting had no unit row. Every raw site that holds a structure rather than a bare scalar reaches the walk that way, so a mapping whose members diverge is now recorded with both spellings; reverting the fix reddens it alongside the scalar rows. Also restores the fractional-seconds row dropped when that table was rewritten, and drops a stale count from a test doc that said "the two preconditions" after a third was added. --- .../annotation/rawjson_internal_test.go | 42 +++++++++++-------- ir/unmodeled.go | 14 +++---- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/compilers/openapi/internal/annotation/rawjson_internal_test.go b/compilers/openapi/internal/annotation/rawjson_internal_test.go index 844934e..77dd106 100644 --- a/compilers/openapi/internal/annotation/rawjson_internal_test.go +++ b/compilers/openapi/internal/annotation/rawjson_internal_test.go @@ -123,6 +123,7 @@ func TestRawFromNode_RendersEveryScalarTag(t *testing.T) { {"date keeps its padding", "2021-01-01", `"2021-01-01"`}, {"datetime keeps its space separator", "2021-01-01 10:20:30", `"2021-01-01 10:20:30"`}, {"datetime keeps its zone as written", "2021-01-01T10:20:30+05:00", `"2021-01-01T10:20:30+05:00"`}, + {"datetime keeps its fractional seconds", "2021-01-01T10:20:30.5Z", `"2021-01-01T10:20:30.5Z"`}, {"explicitly tagged date keeps its spelling", "!!timestamp 2021-1-1", `"2021-1-1"`}, {"binary keeps its base64 text", `!!binary aGVsbG8=`, `"aGVsbG8="`}, {"binary keeps a byte no UTF-8 can name", `!!binary /w==`, `"/w=="`}, @@ -320,8 +321,8 @@ func TestRawFromNode_WalksThroughADocumentNode(t *testing.T) { // rawDivergences is the closed list of inputs on which the walk deliberately // disagrees with the decode it replaced, beyond how a number is spelled. Each -// row is a scalar YAML gives a type and JSON does not, kept as the text the -// source wrote rather than as the resolved form (GitHub #242). +// row holds at least one scalar YAML gives a type and JSON does not, kept as +// the text the source wrote rather than as the resolved form (GitHub #242). // // Both spellings are pinned. The new one is the preservation the change exists // for; the old one is what keeps the row honest — without it a divergence that @@ -335,6 +336,13 @@ var rawDivergences = map[string]struct{ old, want string }{ // can name, which is the loss itself: 0xFF and a source that really wrote // U+FFFD both reached the IR as this, with nothing to tell them apart. `!!binary /w==`: {"\"\\ufffd\"", `"/w=="`}, + // Nesting is the same rule one level down: one divergent scalar makes the + // whole construct diverge, which is how every raw site holding a structure + // rather than a bare scalar reaches this. + "{when: 2021-1-1, blob: !!binary /w==}": { + "{\"blob\":\"\\ufffd\",\"when\":\"2021-01-01T00:00:00Z\"}", + `{"blob":"/w==","when":"2021-1-1"}`, + }, // A block !!binary decodes to the same bytes as the flow one above, so the // old spelling is identical while the source text is not. "!!binary |\n aGVs\n bG8=\n": {`"hello"`, `"aGVs\nbG8=\n"`}, @@ -345,12 +353,13 @@ var rawEquivalenceCorpus = []string{ "1", "1.5", "-2", "0", "0.0", "1e10", "1.10", "0o17", "0x1f", "1_000", "12345678901234567890123", "1.000000000000000000001", "-9223372036854775809", "null", "true", "false", "hello", `"123"`, `""`, "1e400", `"a&c"`, - "2021-1-1", "2021-01-01T10:20:30.5Z", `!!binary aGVsbG8=`, `!!binary /w==`, - // Two timestamp spellings the old decode already reproduced exactly, so - // they are equivalence rows rather than divergences. They are here to keep - // rawDivergences tight: it must list what actually diverges, not every - // input that carries a tag JSON has no type for. - "2021-01-01T10:20:30+05:00", "2021-01-01 10:20:30", + // The scalars YAML types and JSON does not, in both spellings that matter: + // ones the old decode rewrote, which rawDivergences records, and ones it + // already reproduced exactly. The second kind is what keeps that map tight — + // it must list what actually diverges, not every input carrying such a tag. + "2021-1-1", "2021-01-01 10:20:30", "2021-01-01T10:20:30.5Z", + "2021-01-01T10:20:30+05:00", `!!binary aGVsbG8=`, `!!binary /w==`, + "!!binary |\n aGVs\n bG8=\n", "{when: 2021-1-1, blob: !!binary /w==}", "{}", "[]", "[1, 2, 3]", "{z: 1, a: 2, m: 3}", "{a: {b: {c: 1}}}", "[[1], [2, [3]]]", "{a: [1, {b: 2}], c: null}", "{<<: {p: 1}, q: 2}", "{<<: {p: 1}, p: 2}", "{<<: [{p: 1}, {p: 2}]}", @@ -369,10 +378,6 @@ var rawEquivalenceCorpus = []string{ // equality claim, and are here so the asymmetry is visible in a -v run // rather than asserted in a comment. "!!int 12345678901234567890123", "!!float 1e400", - // YAML's block spelling of a !!binary: base64.StdEncoding skips the line - // breaks, so the old decode accepted it and this is a divergence in what - // is kept rather than in what is accepted. - "!!binary |\n aGVs\n bG8=\n", } // TestRawFromNode_DiffersFromTheOldDecodeOnlyWhereRecorded is the equivalence @@ -440,12 +445,13 @@ func mappingOf(key, val *yaml.Node) *yaml.Node { return &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map", Content: []*yaml.Node{key, val}} } -// TestRawConv_RefusesNodesNoCallerShouldPass covers the two preconditions the -// walk asserts on itself rather than on its input. Neither is reachable through -// RawFromNode — it rejects a nil node before walking, and every mapping handed -// to mappingInto has already been proven to be one — so they are exercised here -// directly. They stay because dropping them turns a caller's mistake into a -// silently empty object rather than an answer. +// TestRawConv_RefusesNodesNoCallerShouldPass covers the preconditions the walk +// asserts on itself rather than on its input. None is reachable through +// RawFromNode — it rejects a nil node before walking, every mapping handed to +// mappingInto has already been proven to be one, and scalar routes only two +// tags into verbatimTagged — so they are exercised here directly. They stay +// because dropping one turns a caller's mistake into a silently wrong answer +// rather than an answer at all. func TestRawConv_RefusesNodesNoCallerShouldPass(t *testing.T) { t.Parallel() diff --git a/ir/unmodeled.go b/ir/unmodeled.go index 63e3f58..cd2fe0c 100644 --- a/ir/unmodeled.go +++ b/ir/unmodeled.go @@ -67,13 +67,13 @@ type UnmodeledEntry struct { // where JSON and YAML disagree about how to write one — .5 becomes 0.5, // 0o17 becomes 15 — while every significant digit stays (GitHub #32). // - // A scalar the source format types and JSON does not is kept as the text - // the source wrote, as a JSON string: a YAML timestamp stays `2021-1-1` - // rather than becoming the RFC 3339 instant it resolves to, and a - // `!!binary` keeps its base64 spelling rather than the bytes it names - // (GitHub #242). Reading such a value means resolving the tag the way its - // source format would; what this field promises is that the text is still - // there to resolve. + // A scalar the source format gives a type to and JSON does not is kept as + // the text the source wrote, as a JSON string: a YAML timestamp stays + // `2021-1-1` rather than becoming the RFC 3339 instant it resolves to, and + // a `!!binary` keeps its base64 spelling rather than the bytes it names + // (GitHub #242). Reading one means resolving it the way its source format + // would; what this field promises is that the text is still there to + // resolve, which the resolved form would not have been. Value RawValue `json:"value"` // Provenance locates the construct itself, which the owning node's own // provenance cannot: a validation emitter reporting on a `not` must point at