From 8b50619a8dad9517ae0f3e98ab0e67818823990c Mon Sep 17 00:00:00 2001 From: Cowan Macady Date: Wed, 16 Sep 2026 19:50:48 +0200 Subject: [PATCH] fix: fix missing object in tags implement [ENG-9771] --- gen/constrain.go | 34 ++++++++++++ gen/emit.go | 22 +++++++- gen/gen_test.go | 9 ++++ gen/internal_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++- gen/schema.go | 37 ++++++++----- gen/yaml.go | 45 ++++++++++++++-- testdata/sample.go | 9 +++- 7 files changed, 255 insertions(+), 23 deletions(-) diff --git a/gen/constrain.go b/gen/constrain.go index def3906..9bddc1e 100644 --- a/gen/constrain.go +++ b/gen/constrain.go @@ -19,6 +19,7 @@ package gen // to JSON Schema keywords. import ( + "encoding/json" "strconv" "strings" ) @@ -43,6 +44,11 @@ func coerceScalar(v string, s *Schema) any { return b } case "array": + // JSON text is accepted first (`example:"[1,2]"`, or a list of + // objects on an array,object field), then swag's comma form. + if arr, ok := decodeJSON(v, '['); ok { + return arr + } item := s.Items if item == nil { item = &Schema{} @@ -53,10 +59,38 @@ func coerceScalar(v string, s *Schema) any { arr = append(arr, coerceScalar(strings.TrimSpace(p), item)) } return arr + case "object": + // An object example is JSON text: `example:"{\"k\":1}"` emits + // {"k": 1}, not the quoted string. (swag's key:value,key:value form + // is not supported; it falls through and stays a string.) + if obj, ok := decodeJSON(v, '{'); ok { + return obj + } } return v } +// decodeJSON parses v as a JSON value when it starts with the given opening +// delimiter and decodes as a whole. Numbers are kept as json.Number so +// integers beyond 2^53 survive verbatim into the emitted spec. JSON null and +// an empty document are reported as no match so callers keep the raw text. +func decodeJSON(v string, open byte) (any, bool) { + v = strings.TrimSpace(v) + // json.Valid checks the whole input is exactly one value: Decoder alone + // would stop after the first value and accept trailing text such as + // `{"a":1}]`. + if v == "" || v[0] != open || !json.Valid([]byte(v)) { + return nil, false + } + dec := json.NewDecoder(strings.NewReader(v)) + dec.UseNumber() + var out any + if err := dec.Decode(&out); err != nil || out == nil { + return nil, false + } + return out, true +} + // primaryType is the schema's first non-null type, or "". func primaryType(s *Schema) string { if s == nil { diff --git a/gen/emit.go b/gen/emit.go index 1dec6a0..4ba9194 100644 --- a/gen/emit.go +++ b/gen/emit.go @@ -17,6 +17,7 @@ package gen import ( + "bytes" "encoding/json" "maps" "net/http" @@ -90,9 +91,26 @@ func (api *API) Emit(opt EmitOptions) map[string]any { // Produces returns a default response content type. func (*API) Produces() string { return "application/json" } -// EmitJSON renders to indented JSON bytes. +// EmitJSON renders to indented JSON bytes. Like EmitYAML it leaves &, < and +// > unescaped so the two formats render a description identically (swag's +// json.MarshalIndent would write "a=1&b=2"). A consumer that inlines +// the document into an HTML page must escape it itself. func (api *API) EmitJSON(opt EmitOptions) ([]byte, error) { - return json.MarshalIndent(api.Emit(opt), "", " ") + return encodeJSON(api.Emit(opt), " ") +} + +// encodeJSON is json.Marshal with HTML-safe escaping off and no trailing +// newline; indent is the per-level indentation, "" for compact output. It +// is the single JSON text policy shared by EmitJSON and the YAML scalars. +func encodeJSON(v any, indent string) ([]byte, error) { + var b bytes.Buffer + enc := json.NewEncoder(&b) + enc.SetEscapeHTML(false) + enc.SetIndent("", indent) + if err := enc.Encode(v); err != nil { + return nil, err + } + return bytes.TrimSuffix(b.Bytes(), []byte("\n")), nil } func emitInfo(in *Info) map[string]any { diff --git a/gen/gen_test.go b/gen/gen_test.go index 927e75f..25a8763 100644 --- a/gen/gen_test.go +++ b/gen/gen_test.go @@ -15,6 +15,7 @@ package gen_test import ( + "encoding/json" "maps" "slices" "strings" @@ -871,6 +872,14 @@ func TestMapAndSwaggertypeFields(t *testing.T) { if !slices.Contains(raw.Type, "string") || raw.Format != "base64" { t.Errorf("swaggertype override should win over []byte: %+v", raw) } + policy := acct.Properties["policy"] + if !slices.Contains(policy.Type, "object") || policy.Format != "" { + t.Errorf("swaggertype:\"object\" on json.RawMessage should emit type object: %+v", policy) + } + // The example is JSON text and must decode to an object, not stay a string. + if ex, ok := policy.Example.(map[string]any); !ok || ex["tier"] != "gold" || ex["limit"] != json.Number("5") { + t.Errorf("object example should decode from JSON: %#v", policy.Example) + } } func TestEmitExamplesArrayAndOpenMap(t *testing.T) { diff --git a/gen/internal_test.go b/gen/internal_test.go index 33bb10d..dfa7d28 100644 --- a/gen/internal_test.go +++ b/gen/internal_test.go @@ -20,6 +20,7 @@ package gen // stdlib practice and the testpackage linter's default exemption. import ( + "encoding/json" "go/ast" "go/parser" "go/token" @@ -106,6 +107,28 @@ func TestCoerceScalarArrayTable(t *testing.T) { {intArr, []any{int64(1), int64(2)}, "1, 2"}, // items trimmed and coerced {intArr, []any{int64(1), "2x"}, "1,2x"}, // unparseable item stays string {&Schema{Type: []string{"array"}}, []any{"a", "b"}, "a,b"}, // nil Items: items stay strings + // object schemas take JSON text; anything else stays a string. + {&Schema{Type: []string{"object"}}, map[string]any{"k": json.Number("1"), "s": "v"}, `{"k":1,"s":"v"}`}, + {&Schema{Type: []string{"object"}}, "not json", "not json"}, + // a JSON array, JSON null and trailing garbage are not objects; a + // leading space is fine. + {&Schema{Type: []string{"object"}}, "[1,2]", "[1,2]"}, + {&Schema{Type: []string{"object"}}, "null", "null"}, + {&Schema{Type: []string{"object"}}, `{"a":1} x`, `{"a":1} x`}, + // a stray closing delimiter after a complete value is trailing garbage + // too; Decoder.More() alone would let these through. + {&Schema{Type: []string{"object"}}, `{"a":1}]`, `{"a":1}]`}, + {&Schema{Type: []string{"object"}}, `{"a":1}}`, `{"a":1}}`}, + {intArr, []any{"[1", "2]}"}, "[1,2]}"}, + {&Schema{Type: []string{"object"}}, map[string]any{"a": json.Number("1")}, ` {"a":1}`}, + // integers beyond 2^53 must survive verbatim, not round through float64. + {&Schema{Type: []string{"object"}}, map[string]any{"id": json.Number("9007199254740993")}, `{"id":9007199254740993}`}, + // arrays accept JSON text too, so array,object examples and [1,2] + // don't get split on the commas inside them. + {intArr, []any{json.Number("1"), json.Number("2")}, "[1,2]"}, + {&Schema{Type: []string{"array"}, Items: &Schema{Type: []string{"object"}}}, + []any{map[string]any{"id": json.Number("1"), "n": "x"}}, `[{"id":1,"n":"x"}]`}, + {strArr, []any{"[1", "2"}, "[1,2"}, // unterminated: back to the comma form } for _, c := range cases { if got := coerceScalar(c.in, c.schema); !reflect.DeepEqual(got, c.want) { @@ -537,6 +560,10 @@ func TestYAMLStringTable(t *testing.T) { "": `""`, "$ref": `"$ref"`, "line1\nline2": `"line1\nline2"`, + // &, < and > must not be HTML-escaped to \u0026 etc.: they are plain + // characters in a YAML double-quoted scalar. + "a=1&b=2": `"a=1&b=2"`, + " or >=3": `" or >=3"`, } for in, want := range cases { if got := yamlString(in); got != want { @@ -560,7 +587,7 @@ func TestYAMLScalarAndEmpties(t *testing.T) { func TestEmitJSONRoundTrip(t *testing.T) { api := &API{ Schemas: map[string]*Schema{}, - Info: Info{Title: "T", Version: "1"}, + Info: Info{Title: "T", Version: "1", Description: "a=1&b=2 "}, } data, err := api.EmitJSON(EmitOptions{}) if err != nil { @@ -569,6 +596,18 @@ func TestEmitJSONRoundTrip(t *testing.T) { if !strings.Contains(string(data), `"openapi": "3.2.0"`) { t.Errorf("default version missing: %s", data) } + // &, < and > stay literal, matching EmitYAML; no &-style escapes. + if !strings.Contains(string(data), `"a=1&b=2 "`) || strings.Contains(string(data), `\u00`) { + t.Errorf("JSON must not HTML-escape: %s", data) + } + // Same shape as json.MarshalIndent: no trailing newline. + if strings.HasSuffix(string(data), "\n") { + t.Errorf("unexpected trailing newline") + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("output is not valid JSON: %v", err) + } } func TestPrimitiveTypeTable(t *testing.T) { @@ -624,6 +663,47 @@ func TestSchemaFromSwaggertypeTable(t *testing.T) { if !reflect.DeepEqual(unknown.Type, []string{"string"}) { t.Errorf("unknown swaggertype should degrade to string: %+v", unknown) } + // swag accepts "object" for json.RawMessage-style fields; it must not + // degrade to string. + obj := schemaFromSwaggertype("object") + if !reflect.DeepEqual(obj.Type, []string{"object"}) || obj.Format != "" { + t.Errorf("object: %+v", obj) + } + arrObj := schemaFromSwaggertype("array,object") + if arrObj.Items == nil || !reflect.DeepEqual(arrObj.Items.Type, []string{"object"}) { + t.Errorf("array,object should have object items: %+v", arrObj.Items) + } + // An unknown array element name must not emit an empty type. + arrUnknown := schemaFromSwaggertype("array,mystery") + if arrUnknown.Items == nil || !reflect.DeepEqual(arrUnknown.Items.Type, []string{"string"}) { + t.Errorf("array,unknown should degrade to string items: %+v", arrUnknown.Items) + } + // swag's map form: object, is a map with typed values. + objMap := schemaFromSwaggertype("object,string") + if !reflect.DeepEqual(objMap.Type, []string{"object"}) || objMap.AdditionalProperties == nil || + !reflect.DeepEqual(objMap.AdditionalProperties.Type, []string{"string"}) { + t.Errorf("object,string should be a map of strings: %+v values %+v", objMap, objMap.AdditionalProperties) + } + if obj.AdditionalProperties != nil { + t.Errorf("bare object must stay free-form: %+v", obj.AdditionalProperties) + } + // The tail is parsed recursively, so nested arrays keep every dimension. + nested := schemaFromSwaggertype("array,array,integer") + if nested.Items == nil || !reflect.DeepEqual(nested.Items.Type, []string{"array"}) || + nested.Items.Items == nil || !reflect.DeepEqual(nested.Items.Items.Type, []string{"integer"}) { + t.Errorf("array,array,integer should nest: %+v", nested) + } + arrMap := schemaFromSwaggertype("array,object,int64") + if arrMap.Items == nil || arrMap.Items.AdditionalProperties == nil || + arrMap.Items.AdditionalProperties.Format != "int64" { + t.Errorf("array,object,int64 should be an array of int64 maps: %+v", arrMap.Items) + } + // Whitespace around the separator is tolerated (swag rejects it, but a + // silent downgrade to string items would be worse than leniency). + spaced := schemaFromSwaggertype("array, int64") + if spaced.Items == nil || spaced.Items.Format != "int64" { + t.Errorf("array, int64 with a space should keep the item type: %+v", spaced.Items) + } } func TestSanitizeComponentKey(t *testing.T) { @@ -644,3 +724,43 @@ func TestParseAttributesAndFields(t *testing.T) { t.Errorf("attrs: %v", attrs) } } + +func TestYAMLControlCharactersEscaped(t *testing.T) { + // DEL and the C1 range are forbidden raw in a YAML double-quoted scalar; + // encoding/json leaves them alone, so the YAML path must escape them. + // Bytes are spelled in octal so no editor or tool decodes them. + const bs = "\134" // backslash + esc := func(hex string) string { return bs + "u" + hex } + cases := map[string]string{ + "a\177b": `"a` + esc("007F") + `b"`, + "x\302\200y": `"x` + esc("0080") + `y"`, + "nel\302\205": `"nel` + esc("0085") + `"`, + "end\302\237": `"end` + esc("009F") + `"`, + "tab\tok": `"tab` + bs + `tok"`, // C0: json already escapes + "caf\303\251": "\"caf\303\251\"", // U+00A0 and above stay raw + "nbsp\302\240": "\"nbsp\302\240\"", // first code point past the C1 range + // the non-characters U+FFFE/U+FFFF are also rejected raw by yaml.v3 + "nc\357\277\276": `"nc` + esc("FFFE") + `"`, + "nc\357\277\277": `"nc` + esc("FFFF") + `"`, + "bom\357\273\277": "\"bom\357\273\277\"", // U+FEFF is accepted, left raw + } + for in, want := range cases { + if got := yamlString(in); got != want { + t.Errorf("yamlString(%q) = %s, want %s", in, got, want) + } + } + if got := escapeYAMLControls("plain"); got != "plain" { + t.Errorf("no-op path changed the input: %q", got) + } + + // End to end: the emitted document must carry the escape, not the byte. + api := &API{Schemas: map[string]*Schema{}, Info: Info{Title: "T", Version: "1", Description: "bad\177\302\222"}} + data, err := api.EmitYAML(EmitOptions{}) + if err != nil { + t.Fatal(err) + } + out := string(data) + if strings.ContainsAny(out, "\177\302\222") || !strings.Contains(out, `"bad`+esc("007F")+esc("0092")+`"`) { + t.Errorf("control characters leaked into YAML: %q", data) + } +} diff --git a/gen/schema.go b/gen/schema.go index 893b1c8..b80ce04 100644 --- a/gen/schema.go +++ b/gen/schema.go @@ -802,26 +802,35 @@ func applyExtensionsTag(s *Schema, spec string) { } } +// schemaFromSwaggertype builds a schema from a swaggertype tag. The grammar +// is swag's, applied recursively on the comma-separated tail: `array,` +// wraps the rest in items, `object` alone is a free-form object (the idiom +// for json.RawMessage fields) and `object,` a map whose values are the +// rest, `primitive,` is just the rest, and a bare name is a JSON or Go +// primitive. Anything unknown degrades to string rather than erroring. func schemaFromSwaggertype(spec string) *Schema { - parts := strings.Split(spec, ",") - switch parts[0] { + head, rest, _ := strings.Cut(spec, ",") + head, rest = strings.TrimSpace(head), strings.TrimSpace(rest) + switch head { case "array": - inner := "string" - if len(parts) > 1 { - inner = parts[1] + if rest == "" { + rest = "string" } - t, f, _ := primitiveType(inner) - return &Schema{Type: []string{"array"}, Items: &Schema{Type: []string{t}, Format: f}} - case "primitive": - if len(parts) > 1 { - t, f, _ := primitiveType(parts[1]) - return &Schema{Type: []string{t}, Format: f} + return &Schema{Type: []string{"array"}, Items: schemaFromSwaggertype(rest)} + case "object": + s := &Schema{Type: []string{"object"}} + if rest != "" { + s.AdditionalProperties = schemaFromSwaggertype(rest) } - default: - if t, f, ok := primitiveType(parts[0]); ok { - return &Schema{Type: []string{t}, Format: f} + return s + case "primitive": + if rest != "" { + return schemaFromSwaggertype(rest) } } + if t, f, ok := primitiveType(head); ok { + return &Schema{Type: []string{t}, Format: f} + } return &Schema{Type: []string{"string"}} } diff --git a/gen/yaml.go b/gen/yaml.go index f0872f3..79913d6 100644 --- a/gen/yaml.go +++ b/gen/yaml.go @@ -15,7 +15,7 @@ package gen import ( - "encoding/json" + "fmt" "maps" "regexp" "slices" @@ -109,14 +109,49 @@ func yamlScalar(v any) string { default: // bools, numbers, and any leftover structured value: JSON is valid // YAML flow style, and encoding/json handles every scalar correctly. - data, err := json.Marshal(t) + data, err := marshalNoHTMLEscape(t) if err != nil { return "null" } - return string(data) + return data } } +// marshalNoHTMLEscape renders v as compact JSON text via encodeJSON, then +// escapes what YAML additionally forbids. &, < and > stay literal: they are +// ordinary characters in a YAML double-quoted scalar, +// and a description reading "a=1\u0026b=2" is correct but unreadable. +func marshalNoHTMLEscape(v any) (string, error) { + data, err := encodeJSON(v, "") + if err != nil { + return "", err + } + return escapeYAMLControls(string(data)), nil +} + +// escapeYAMLControls rewrites the characters encoding/json leaves raw but +// YAML 1.2 forbids in a double-quoted scalar: DEL (U+007F), the C1 range +// (U+0080..U+009F) and the non-characters U+FFFE and U+FFFF. They only ever +// occur inside a string, so the pass is safe on a whole JSON fragment. +// Strict loaders such as yaml.v3 reject a document that contains them raw. +func escapeYAMLControls(s string) string { + if !strings.ContainsFunc(s, isYAMLControl) { + return s + } + var b strings.Builder + b.Grow(len(s) + 8) + for _, r := range s { + if isYAMLControl(r) { + fmt.Fprintf(&b, `\u%04X`, r) + continue + } + b.WriteRune(r) + } + return b.String() +} + +func isYAMLControl(r rune) bool { return (r >= 0x7F && r <= 0x9F) || r == 0xFFFE || r == 0xFFFF } + // yamlPlain matches strings that are unambiguous unquoted: they can't be // mistaken for numbers, and contain no YAML indicator characters. var yamlPlain = regexp.MustCompile(`^[A-Za-z_/][A-Za-z0-9_./-]*$`) @@ -131,9 +166,9 @@ func yamlString(s string) string { if yamlPlain.MatchString(s) && !yamlReserved[strings.ToLower(s)] { return s } - data, err := json.Marshal(s) + data, err := marshalNoHTMLEscape(s) if err != nil { return `""` } - return string(data) + return data } diff --git a/testdata/sample.go b/testdata/sample.go index a011ffa..a6efec0 100644 --- a/testdata/sample.go +++ b/testdata/sample.go @@ -1,6 +1,10 @@ package testdata -import "github.com/indykite/openapi-parser/testdata/model" +import ( + "encoding/json" + + "github.com/indykite/openapi-parser/testdata/model" +) // @title Example API // @summary Accounts and things, for parser exercise. @@ -65,6 +69,9 @@ type Account struct { Meta map[string]string `json:"meta"` Attrs map[string]interface{} `json:"attrs"` Raw []byte `json:"raw" swaggertype:"string" format:"base64"` + // Policy is stored JSON inlined as an object; swaggertype:"object" is the + // swag idiom for json.RawMessage fields and must not degrade to string. + Policy json.RawMessage `json:"policy" swaggertype:"object" example:"{\"tier\":\"gold\",\"limit\":5}"` // Config has an escaped-JSON example; tags after it must still parse. Config string `json:"config" example:"{\"key\":\"value\"}" binding:"required,min=1,max=100"` // Code is required even though json omits it when empty (swag semantics).