diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 1cc268d..062eb44 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -999,6 +999,31 @@ func assertNumericPrecision(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.Equal(t, ir.BigVal("0.5"), sampled.Examples[0].Value.Num) assertYAMLIntegerBases(t, doc, m) + assertRawPreservedNumbers(t, m) +} + +// assertRawPreservedNumbers pins the second numeric channel: the constructs kept +// verbatim as raw JSON rather than lowered into a Value. It rounded every one of +// these through float64 until GitHub #32 — a 30-digit extension came back as +// 1.2345678901234568e+29 — which no case in this corpus could see, because none +// of them carried a number a float64 cannot hold. +func assertRawPreservedNumbers(t *testing.T, m *ir.Model) { + t.Helper() + preserved, ok := propByWire(m, "preserved") + require.True(t, ok) + kept := preserved.Unmodeled + + for _, tc := range []struct{ key, want string }{ + {"openapi:x-precise-limit", "123456789012345678901234567890"}, + {"openapi:x-precise-step", "1.000000000000000000001"}, + {"openapi:x-precise-scale", "1.10"}, + {"openapi:not", `{"const":123456789012345678901234567890}`}, + } { + 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 is preserved as written, not as a float64 can spell it", tc.key) + } } // assertYAMLIntegerBases pins the value every numeric site stores for an integer diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index e911542..5eccaa8 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -34,22 +34,26 @@ import ( // happened (GitHub #144): an absent node yields (nil, nil) — there was no // construct here — while a node that cannot be represented yields an error. // -// Legal YAML reaches that second failure by two routes: a mapping with a -// non-string key does not decode into Go's JSON model at all, and .nan/.inf -// decode but do not marshal. +// A node fails to convert when it names something JSON cannot: a mapping key +// that is not a string, a key written twice, .nan or .inf, or a tag with no JSON +// counterpart. The walk's own bounds refuse two shapes more — an alias that +// cycles, and one that expands past its node budget. +// +// The conversion walks the node tree rather than decoding it into `any` and +// re-marshalling, because that decode rounds every numeric literal through +// float64: it silently rewrote a 23-digit extension value and flattened +// 1.000000000000000000001 to 1, in the one channel whose whole promise is +// verbatim preservation (GitHub #32). func RawFromNode(node *yaml.Node) (ir.RawValue, error) { if node == nil { return nil, nil } - var v any - if err := node.Decode(&v); err != nil { - return nil, fmt.Errorf("decode yaml node: %w", err) - } - data, err := json.Marshal(v) + var conv rawConv + data, err := conv.node(node, 0) if err != nil { - return nil, fmt.Errorf("encode as json: %w", err) + return nil, fmt.Errorf("render yaml node as json: %w", err) } - return ir.RawValue(data), nil + return data, nil } // EffectiveDeprecated reports the deprecated flag, use-site over referent. diff --git a/compilers/openapi/internal/annotation/rawjson.go b/compilers/openapi/internal/annotation/rawjson.go new file mode 100644 index 0000000..78a520a --- /dev/null +++ b/compilers/openapi/internal/annotation/rawjson.go @@ -0,0 +1,316 @@ +package annotation + +import ( + "encoding/json" + "fmt" + "slices" + "strings" + "time" + + yaml "gopkg.in/yaml.v3" + + "github.com/dexpace/morphic/compilers/openapi/internal/value" +) + +// Bounds on one conversion. Both exist because a YAML node is a graph, not a +// tree: an alias may point at an ancestor, and a chain of aliases each naming +// the one above it expands multiplicatively. +// +// Neither is this compiler's real defence against either shape — scan's cycle +// detector refuses both long before a node reaches here, on budgets calibrated +// against a 1,693-spec corpus, and its allowances are far tighter than these. +// They are here so the walk is bounded by its own terms rather than by what +// happens to run before it, which is the whole point of a backstop: nothing +// about this file's correctness should depend on the caller. +// +// maxRawDepth is yaml.v3's own parse-time nesting cap, so it binds only on +// input the parser could not have produced by nesting alone: an alias cycle. +// Nothing a spec can legally nest reaches it. +const ( + maxRawDepth = 10000 + maxRawNodes = 1 << 20 +) + +// errMergeWantsMap reports a `<<` whose value is neither a mapping nor a +// sequence of mappings, matching yaml.v3's own refusal to guess. +var errMergeWantsMap = fmt.Errorf("map merge requires map or sequence of maps as the value") + +// rawConv renders one YAML node as canonical JSON. It carries the node budget +// shared across the whole walk; depth is a parameter because it is a property +// of the path rather than of the conversion. +// +// JSON is assembled here rather than by marshalling a Go tree, for the same +// reason jsonObject does it: the members are already encoded, so handing them +// back to encoding/json only reopens the question of how a number is spelled — +// which is the bug this file exists to close (GitHub #32). +type rawConv struct { + nodes int +} + +// node renders any YAML node as canonical JSON. +func (c *rawConv) node(n *yaml.Node, depth int) (json.RawMessage, error) { + if n == nil { + return nil, fmt.Errorf("nil yaml node") + } + if depth > maxRawDepth { + return nil, fmt.Errorf("nesting exceeds %d", maxRawDepth) + } + c.nodes++ + if c.nodes > maxRawNodes { + return nil, fmt.Errorf("expands past %d nodes", maxRawNodes) + } + + switch n.Kind { + case yaml.DocumentNode: + if len(n.Content) != 1 { + return nil, fmt.Errorf("document node holds %d children", len(n.Content)) + } + return c.node(n.Content[0], depth+1) + case yaml.AliasNode: + if n.Alias == nil { + return nil, fmt.Errorf("alias %q resolves to nothing", n.Value) + } + return c.node(n.Alias, depth+1) + case yaml.ScalarNode: + return c.scalar(n) + case yaml.SequenceNode: + return c.sequence(n, depth) + case yaml.MappingNode: + return c.mapping(n, depth) + default: + return nil, fmt.Errorf("unsupported yaml node kind %d", n.Kind) + } +} + +// scalar renders a scalar node by its resolved tag. +// +// Only the numeric tags read the source text; every other tag is rendered the +// way a whole-tree 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 here: they are a different mechanism from the float64 rounding +// this change fixes, and moving them belongs with its own reasoning (#242). +func (c *rawConv) scalar(n *yaml.Node) (json.RawMessage, error) { + switch n.Tag { + case "!!null": + return json.RawMessage("null"), nil + case "!!bool": + var b bool + if err := n.Decode(&b); err != nil { + return nil, fmt.Errorf("bool literal %q: %w", n.Value, err) + } + if b { + return json.RawMessage("true"), nil + } + return json.RawMessage("false"), nil + case "!!int", "!!float": + // The whole point of this file: the literal's exact decimal, never a + // float64 rounding of it (GitHub #32). NumericLiteral resolves YAML's + // own bases too, so 0o17 still reads 15 rather than 17. + num, err := value.NumericLiteral(n) + if err != nil { + return nil, fmt.Errorf("numeric literal %q: %w", n.Value, err) + } + // BigVal's contract is that its text is a JSON-valid number, and this is + // the one caller that splices it straight into a document rather than + // into an ir.Value field. NewBigVal does not yet hold that contract for + // a binary exponent — `!!float 1p4` is stored verbatim (GitHub #45) — so + // the splice checks rather than trusts. Refusing here keeps a construct + // no JSON can name out of the IR, which is what the decode this replaced + // did with the same input, and leaves #45 to be settled on its own terms. + if !json.Valid([]byte(num)) { + return nil, fmt.Errorf("numeric literal %q renders as %q, which is not JSON", n.Value, num) + } + return json.RawMessage(num), nil + case "!!str": + return jsonString(n.Value), nil + case "!!timestamp": + var t time.Time + if err := n.Decode(&t); 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 { + return nil, fmt.Errorf("binary literal: %w", err) + } + return jsonString(raw), nil + default: + return nil, fmt.Errorf("unsupported scalar tag %q", n.Tag) + } +} + +// sequence renders a YAML sequence as a JSON array, in source order. +func (c *rawConv) sequence(n *yaml.Node, depth int) (json.RawMessage, error) { + var b strings.Builder + b.WriteByte('[') + for i, child := range n.Content { + if i > 0 { + b.WriteByte(',') + } + item, err := c.node(child, depth+1) + if err != nil { + return nil, err + } + b.Write(item) + } + b.WriteByte(']') + return json.RawMessage(b.String()), nil +} + +// mapping renders a YAML mapping as a JSON object, members in sorted key order. +// +// Sorted rather than source order on purpose: it is what the decode this +// replaced produced, since encoding/json sorts a Go map, and it is what the +// IR's determinism invariant asks of every map it serializes. +func (c *rawConv) mapping(n *yaml.Node, depth int) (json.RawMessage, error) { + members := map[string]json.RawMessage{} + if err := c.mappingInto(members, n, depth); err != nil { + return nil, err + } + + keys := make([]string, 0, len(members)) + for k := range members { + keys = append(keys, k) + } + slices.Sort(keys) + + var b strings.Builder + b.WriteByte('{') + for i, k := range keys { + if i > 0 { + b.WriteByte(',') + } + b.Write(jsonString(k)) + b.WriteByte(':') + b.Write(members[k]) + } + b.WriteByte('}') + return json.RawMessage(b.String()), nil +} + +// mappingInto fills dst from n, leaving keys dst already holds untouched. That +// one rule is the whole of YAML's merge precedence: a mapping's own keys are +// written before its `<<` is read, and a sequence of merge sources is read in +// order, so the nearer declaration always wins. +func (c *rawConv) mappingInto(dst map[string]json.RawMessage, n *yaml.Node, depth int) error { + if n.Kind != yaml.MappingNode { + return fmt.Errorf("expected a mapping, got yaml node kind %d", n.Kind) + } + if err := checkUniqueKeys(n); err != nil { + return err + } + + var merge *yaml.Node + for i := 0; i+1 < len(n.Content); i += 2 { + key, val := n.Content[i], n.Content[i+1] + if isMergeKey(key) { + merge = val + continue + } + name, err := mapKey(key) + if err != nil { + return err + } + if _, taken := dst[name]; taken { + continue + } + enc, err := c.node(val, depth+1) + if err != nil { + return err + } + dst[name] = enc + } + + if merge == nil { + return nil + } + return c.merge(dst, merge, depth) +} + +// merge folds a `<<` value into dst: a mapping, an alias to one, or a sequence +// of either. +func (c *rawConv) merge(dst map[string]json.RawMessage, merge *yaml.Node, depth int) error { + if depth > maxRawDepth { + return fmt.Errorf("nesting exceeds %d", maxRawDepth) + } + if merge.Kind == yaml.SequenceNode { + for _, item := range merge.Content { + src, err := mergeSource(item) + if err != nil { + return err + } + if err := c.mappingInto(dst, src, depth+1); err != nil { + return err + } + } + return nil + } + + src, err := mergeSource(merge) + if err != nil { + return err + } + return c.mappingInto(dst, src, depth+1) +} + +// mergeSource resolves one merge source to the mapping it names. +func mergeSource(n *yaml.Node) (*yaml.Node, error) { + if n.Kind == yaml.AliasNode { + if n.Alias == nil { + return nil, fmt.Errorf("alias %q resolves to nothing", n.Value) + } + n = n.Alias + } + if n.Kind != yaml.MappingNode { + return nil, errMergeWantsMap + } + return n, nil +} + +// checkUniqueKeys rejects a mapping that names one key twice, which yaml.v3 +// rejects by default and this compiler has therefore always refused. +func checkUniqueKeys(n *yaml.Node) error { + for i := 0; i < len(n.Content); i += 2 { + for j := i + 2; j < len(n.Content); j += 2 { + a, b := n.Content[i], n.Content[j] + if a.Kind == b.Kind && a.Value == b.Value { + return fmt.Errorf("mapping key %q already defined at line %d", b.Value, a.Line) + } + } + } + return nil +} + +// mapKey gives a mapping key its JSON name, rejecting every key JSON has no +// name for. The rule matches yaml.v3's own: a mapping decodes into Go's JSON +// model only when every key is a string, and a non-string key is what makes the +// whole construct unrepresentable rather than merely awkward (GitHub #144). +func mapKey(n *yaml.Node) (string, error) { + if n.Kind != yaml.ScalarNode || n.ShortTag() != "!!str" { + return "", fmt.Errorf("mapping key %q is not a string", n.Value) + } + return n.Value, nil +} + +// isMergeKey reports whether a key node is YAML's `<<` merge key, by the same +// test yaml.v3 applies. +func isMergeKey(n *yaml.Node) bool { + return n.Kind == yaml.ScalarNode && n.Value == "<<" && + (n.Tag == "" || n.Tag == "!" || n.ShortTag() == "!!merge") +} + +// jsonString encodes s as a JSON string. encoding/json cannot fail on a string +// — ill-formed UTF-8 is rewritten to U+FFFD rather than refused — so the error +// it declares is discarded here exactly as jsonObject discards it for a key. +func jsonString(s string) json.RawMessage { + encoded, _ := json.Marshal(s) + return encoded +} diff --git a/compilers/openapi/internal/annotation/rawjson_internal_test.go b/compilers/openapi/internal/annotation/rawjson_internal_test.go new file mode 100644 index 0000000..6aa8594 --- /dev/null +++ b/compilers/openapi/internal/annotation/rawjson_internal_test.go @@ -0,0 +1,371 @@ +package annotation + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + yaml "gopkg.in/yaml.v3" +) + +// 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. +func decodeAndMarshal(node *yaml.Node) (json.RawMessage, error) { + var v any + if err := node.Decode(&v); err != nil { + return nil, err + } + return json.Marshal(v) +} + +// 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. +// +// 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 +// the literal rune), and normalizing one side alone would report that as a +// difference. Rounding an already-rounded number changes nothing, so applying it +// to the old output costs the comparison none of its force. +func throughFloat64(t *testing.T, raw json.RawMessage) string { + t.Helper() + var v any + require.NoError(t, json.Unmarshal(raw, &v)) + out, err := json.Marshal(v) + require.NoError(t, err) + return string(out) +} + +// TestRawFromNode_KeepsNumericLiteralsExact is the regression this change +// exists for. Every row is a value the old decode/re-marshal rewrote, in the +// one channel whose documented promise is verbatim preservation (GitHub #32). +func TestRawFromNode_KeepsNumericLiteralsExact(t *testing.T) { + t.Parallel() + tests := []struct { + name string + yaml string + want string + }{ + {"integer past int64", "12345678901234567890123", "12345678901234567890123"}, + {"decimal past float64 precision", "1.000000000000000000001", "1.000000000000000000001"}, + {"negative past int64", "-9223372036854775809", "-9223372036854775809"}, + {"trailing zero is significant", "1.10", "1.10"}, + {"exponent case is preserved", "1E+10", "1E+10"}, + {"int64 boundary", "9223372036854775807", "9223372036854775807"}, + {"uint64 boundary", "18446744073709551615", "18446744073709551615"}, + {"tiny magnitude", "1.5e-300", "1.5e-300"}, + {"explicitly tagged huge int", `!!int 12345678901234567890123`, "12345678901234567890123"}, + {"nested in a mapping", "{a: 12345678901234567890123}", `{"a":12345678901234567890123}`}, + {"nested in a sequence", "[1.000000000000000000001]", "[1.000000000000000000001]"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(yamlNode(t, tc.yaml)) + require.NoError(t, err) + assert.Equal(t, tc.want, string(got)) + }) + } +} + +// TestRawFromNode_ResolvesYAMLIntegerBases pins the half of numeric handling +// that must not become verbatim: YAML takes an integer's base from its prefix, +// so the source spelling is not what the value means in base 10. Preserving +// these as written would read 0o17 as seventeen. +func TestRawFromNode_ResolvesYAMLIntegerBases(t *testing.T) { + t.Parallel() + tests := []struct{ name, yaml, want string }{ + {"octal", "0o17", "15"}, + {"hex", "0x1f", "31"}, + {"binary", "0b101", "5"}, + {"bare leading zero is octal", "0644", "420"}, + {"digit separators are dropped", "1_000", "1000"}, + {"leading plus is dropped", "+5", "5"}, + {"trailing dot is JSON-invalid", "5.", "5"}, + {"leading dot is JSON-invalid", ".5", "0.5"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(yamlNode(t, tc.yaml)) + require.NoError(t, err) + assert.Equal(t, tc.want, string(got)) + assert.True(t, json.Valid(got), "every rendered number is JSON-valid") + }) + } +} + +// TestRawFromNode_RendersEveryScalarTag covers the tags a walk over the node +// tree must render itself, now that no decode into `any` renders them for it. +func TestRawFromNode_RendersEveryScalarTag(t *testing.T) { + t.Parallel() + tests := []struct{ name, yaml, want string }{ + {"null", "null", "null"}, + {"tilde is null", "~", "null"}, + {"true", "true", "true"}, + {"false", "false", "false"}, + {"string", "hello", `"hello"`}, + {"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"`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(yamlNode(t, tc.yaml)) + require.NoError(t, err) + assert.Equal(t, tc.want, string(got)) + }) + } +} + +// TestRawFromNode_PreservesMergeAndOrderingSemantics pins the mapping rules the +// walk had to reimplement when it stopped delegating to yaml.v3's decoder. +// Losing any of them would be a silent regression: the construct still converts, +// just to something else. +func TestRawFromNode_PreservesMergeAndOrderingSemantics(t *testing.T) { + t.Parallel() + tests := []struct{ name, yaml, want string }{ + {"keys sort", "{z: 1, a: 2, m: 3}", `{"a":2,"m":3,"z":1}`}, + {"sequence keeps source order", "[3, 1, 2]", `[3,1,2]`}, + {"merge key expands", "{<<: {p: 1}, q: 2}", `{"p":1,"q":2}`}, + {"own key beats merged key", "{<<: {p: 1}, p: 2}", `{"p":2}`}, + {"earlier merge source wins", "{<<: [{p: 1}, {p: 2}]}", `{"p":1}`}, + {"merge through an alias", "a: &m {p: 1}\nb: {<<: *m, q: 2}", `{"a":{"p":1},"b":{"p":1,"q":2}}`}, + {"nested merge inside a merged map", "a: &m {<<: {deep: 1}, p: 2}\nb: {<<: *m}", `{"a":{"deep":1,"p":2},"b":{"deep":1,"p":2}}`}, + {"alias to a scalar", "a: &n 5\nb: *n", `{"a":5,"b":5}`}, + {"alias to a sequence", "a: &s [1, 2]\nb: *s", `{"a":[1,2],"b":[1,2]}`}, + {"empty mapping", "{}", `{}`}, + {"empty sequence", "[]", `[]`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(yamlNode(t, tc.yaml)) + require.NoError(t, err) + assert.Equal(t, tc.want, string(got)) + }) + } +} + +// TestRawFromNode_RefusesWhatJSONCannotName pins the failures. Each one is a +// construct that reaches the IR in no form at all rather than in a weaker one, +// which is why the caller turns it into a diagnostic (GitHub #144). +func TestRawFromNode_RefusesWhatJSONCannotName(t *testing.T) { + t.Parallel() + tests := []struct{ name, yaml, wantErr string }{ + {"integer key", "{1: v}", "not a string"}, + {"null key", "{null: v}", "not a string"}, + {"bool key", "{true: v}", "not a string"}, + {"sequence key", "{? [1, 2]\n: v}", "not a string"}, + {"nested non-string key", "{outer: {1: v}}", "not a string"}, + {"duplicate key", "{k: 1, k: 2}", "already defined"}, + {"duplicate key after a merge", "{<<: {p: 1}, k: 1, k: 2}", "already defined"}, + {"not a number", ".nan", "numeric literal"}, + {"positive infinity", ".inf", "numeric literal"}, + {"negative infinity", "-.inf", "numeric literal"}, + {"unknown scalar tag", "!!python/object x", `unsupported scalar tag`}, + {"merge from a scalar", "{<<: 1}", "map merge requires"}, + {"merge from a sequence of scalars", "{<<: [1]}", "map merge requires"}, + {"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"}, + {"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"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(yamlNode(t, tc.yaml)) + require.Error(t, err) + assert.Nil(t, got, "a refusal writes nothing") + assert.Contains(t, err.Error(), tc.wantErr) + }) + } +} + +// TestRawFromNode_IsBoundedOnACyclicAlias proves the walk terminates on the one +// input shape a node tree can hold that a document tree cannot: an alias naming +// an ancestor. Without the depth bound this recurses until the stack dies. +func TestRawFromNode_IsBoundedOnACyclicAlias(t *testing.T) { + t.Parallel() + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("a: &x [*x]\n"), &doc)) + + got, err := RawFromNode(doc.Content[0]) + + require.Error(t, err, "a cyclic alias is refused rather than followed forever") + assert.Nil(t, got) + assert.Contains(t, err.Error(), "nesting exceeds") +} + +// TestRawFromNode_IsBoundedOnAliasAmplification proves the node budget stops a +// wide alias chain. scan refuses this shape long before it reaches here; the +// bound exists so the walk does not depend on that having run. +func TestRawFromNode_IsBoundedOnAliasAmplification(t *testing.T) { + t.Parallel() + var b strings.Builder + b.WriteString("a0: &a0 [x, x, x, x, x, x, x, x, x]\n") + for level := 1; level <= 8; level++ { + fmt.Fprintf(&b, "a%d: &a%d [", level, level) + for i := 0; i < 9; i++ { + if i > 0 { + b.WriteString(", ") + } + fmt.Fprintf(&b, "*a%d", level-1) + } + b.WriteString("]\n") + } + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte(b.String()), &doc)) + + got, err := RawFromNode(doc.Content[0]) + + require.Error(t, err, "a multiplicative alias chain is refused") + assert.Nil(t, got) + assert.Contains(t, err.Error(), "expands past") +} + +// TestRawFromNode_RejectsMalformedNodes covers the node shapes no parser +// produces but a caller could hand-build, so the walk answers rather than +// panicking on them. +func TestRawFromNode_RejectsMalformedNodes(t *testing.T) { + t.Parallel() + tests := []struct { + name string + node *yaml.Node + wantErr string + }{ + {"zero node", &yaml.Node{}, "unsupported yaml node kind"}, + {"unresolved alias", &yaml.Node{Kind: yaml.AliasNode, Value: "x"}, "resolves to nothing"}, + {"childless document", &yaml.Node{Kind: yaml.DocumentNode}, "document node holds 0 children"}, + { + "merge from an unresolved alias", + mappingOf(&yaml.Node{Kind: yaml.ScalarNode, Tag: "!!merge", Value: "<<"}, + &yaml.Node{Kind: yaml.AliasNode, Value: "x"}), + "resolves to nothing", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := RawFromNode(tc.node) + require.Error(t, err) + assert.Nil(t, got) + assert.Contains(t, err.Error(), tc.wantErr) + }) + } +} + +// TestRawFromNode_WalksThroughADocumentNode covers the entry a caller reaches +// when it hands over a whole parsed document rather than a node inside one. +func TestRawFromNode_WalksThroughADocumentNode(t *testing.T) { + t.Parallel() + var doc yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("{a: 1}\n"), &doc)) + require.Equal(t, yaml.DocumentNode, doc.Kind) + + got, err := RawFromNode(&doc) + + require.NoError(t, err) + assert.JSONEq(t, `{"a":1}`, string(got)) +} + +// TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers 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. +// +// 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) { + 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: []}", + } + for _, src := range corpus { + t.Run(src, func(t *testing.T) { + t.Parallel() + node := yamlNode(t, src) + + want, oldErr := decodeAndMarshal(node) + got, newErr := RawFromNode(node) + + 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") + }) + } +} + +// mappingOf builds a one-pair mapping node, for the malformed shapes no parser +// emits. +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. +func TestRawConv_RefusesNodesNoCallerShouldPass(t *testing.T) { + t.Parallel() + + var c rawConv + got, err := c.node(nil, 0) + require.Error(t, err, "a nil node is a caller bug, not an absent construct") + assert.Nil(t, got) + assert.Contains(t, err.Error(), "nil yaml node") + + 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") +} + +// TestRawFromNode_BoundsAMergeChainThatNeverRevisitsANode proves the bound on +// the one recursion that does not pass through node(): a `<<` whose source is +// itself nothing but a `<<`. Each level adds depth while adding no value to +// walk, so only merge's own check stops it. +// +// The chain is built rather than parsed because yaml.v3 caps parse nesting at +// the same figure, so no document could express one this deep inline; aliases +// are how a real source would reach it. +func TestRawFromNode_BoundsAMergeChainThatNeverRevisitsANode(t *testing.T) { + t.Parallel() + innermost := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + chain := innermost + for i := 0; i < maxRawDepth+10; i++ { + chain = mappingOf(&yaml.Node{Kind: yaml.ScalarNode, Tag: "!!merge", Value: "<<"}, chain) + } + + got, err := RawFromNode(chain) + + require.Error(t, err, "a merge chain past the depth bound is refused") + assert.Nil(t, got) + assert.Contains(t, err.Error(), "nesting exceeds") +} diff --git a/ir/unmodeled.go b/ir/unmodeled.go index accd7df..a8813ad 100644 --- a/ir/unmodeled.go +++ b/ir/unmodeled.go @@ -58,12 +58,19 @@ type UnmodeledEntry struct { // entries and not vendor noise; a linter wants the degradations. Reason UnmodeledReason `json:"reason"` // Value is the source construct, preserved whole rather than byte-for-byte: - // nothing here discards or reshapes it, but two re-encodings sit between the + // nothing here discards or reshapes it, but re-encodings sit between the // source bytes and this field. json.Marshal compacts a RawValue and escapes // <, >, & as \uXXXX, and a compiler that rebuilds the value from its parsed - // tree (the OpenAPI path does, via a decode/re-marshal) also normalizes - // object key order and number spelling and rewrites ill-formed UTF-8 to - // U+FFFD. What survives is the construct's meaning, not its spelling. + // tree (the OpenAPI path does) also sorts object keys. + // + // A number's value survives exactly. Its spelling is canonicalized only + // 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). 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/numeric-precision.golden.json b/testdata/conformance/openapi/numeric-precision.golden.json index 5589426..fa8f781 100644 --- a/testdata/conformance/openapi/numeric-precision.golden.json +++ b/testdata/conformance/openapi/numeric-precision.golden.json @@ -490,6 +490,69 @@ "source": 0, "pointer": "/components/schemas/S/properties/loose" } + }, + { + "id": "p/openapi/components/schemas/S/properties/preserved", + "name": { + "source": "preserved", + "canonical": "preserved" + }, + "wireName": "preserved", + "type": { + "target": "t/prim/number", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "unmodeled": { + "openapi:not": { + "reason": "validation_only", + "value": { + "const": 123456789012345678901234567890 + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/properties/preserved/not" + } + }, + "openapi:x-precise-limit": { + "reason": "vendor_extension", + "value": 123456789012345678901234567890, + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/properties/preserved/x-precise-limit" + } + }, + "openapi:x-precise-scale": { + "reason": "vendor_extension", + "value": 1.10, + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/properties/preserved/x-precise-scale" + } + }, + "openapi:x-precise-step": { + "reason": "vendor_extension", + "value": 1.000000000000000000001, + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/properties/preserved/x-precise-step" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/S/properties/preserved" + } } ], "abstract": false, @@ -529,11 +592,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/S/properties/preserved" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "numeric-precision.yaml", - "hash": "6788c0244395b3f75e699917d5af52f9937ea3b97e4f006505a7909078bcd074" + "hash": "837bcd7be0a2bb0e52999a13dd7f444a8e16b12ad10b69ffd455f295372073ac" } ] } diff --git a/testdata/conformance/openapi/numeric-precision.yaml b/testdata/conformance/openapi/numeric-precision.yaml index b5bc0f4..c72b500 100644 --- a/testdata/conformance/openapi/numeric-precision.yaml +++ b/testdata/conformance/openapi/numeric-precision.yaml @@ -58,3 +58,16 @@ components: loose: type: number default: 09 + # Everything above rides the Value/BigVal channel. The constructs kept + # verbatim as raw JSON are a second channel with the same requirement, + # and it is the one that had no corpus case: a vendor extension, and a + # validation-only keyword held under the §4.7 carve-out. A trailing zero + # is significant here in a way it is not to a float — 1.10 and 1.1 are + # the same number and different source text. + preserved: + type: number + x-precise-limit: 123456789012345678901234567890 + x-precise-step: 1.000000000000000000001 + x-precise-scale: 1.10 + not: + const: 123456789012345678901234567890