openapi3: preserve origin for a $ref to a schema under an arbitrary top-level key

openapi3/issue513_test.go

@@ -1,11 +1,14 @@
-package openapi3
+package openapi3_test
 
 import (
 	"encoding/json"
+	"net/url"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
+	"github.com/getkin/kin-openapi/openapi3"
 )
 
 func TestExtraSiblingsInRemoteRef(t *testing.T) {
@@ -34,45 +37,52 @@ paths:
                 $ref: http://schemas.sentex.io/store/categories.json
 `
 
-	// When that site fails to respond:
-	// see https://github.com/getkin/kin-openapi/issues/495
+	resolver := func(loader *openapi3.Loader, url *url.URL) (data []byte, err error) {
+		switch url.String() {
+		case "http://schemas.sentex.io/store/categories.json":
+			data = []byte(`
+			{
+			  "$id": "http://schemas.sentex.io/store/categories.json",
+			  "$schema": "http://json-schema.org/draft-07/schema#",
+			  "description": "array of category strings",
+			  "type": "array",
+			  "items": {
+			    "allOf": [
+			      {
+			        "$ref": "http://schemas.sentex.io/store/category.json"
+			      }
+			    ]
+			  }
+			}`)
 
-	// http://schemas.sentex.io/store/categories.json
-	// {
-	//   "$id": "http://schemas.sentex.io/store/categories.json",
-	//   "$schema": "http://json-schema.org/draft-07/schema#",
-	//   "description": "array of category strings",
-	//   "type": "array",
-	//   "items": {
-	//     "allOf": [
-	//       {
-	//         "$ref": "http://schemas.sentex.io/store/category.json"
-	//       }
-	//     ]
-	//   }
-	// }
+		case "http://schemas.sentex.io/store/category.json":
+			data = []byte(`
+			{
+			  "$id": "http://schemas.sentex.io/store/category.json",
+			  "$schema": "http://json-schema.org/draft-07/schema#",
+			  "description": "category name for products",
+			  "type": "string",
+			  "pattern": "^[A-Za-z0-9\\-]+$",
+			  "minimum": 1,
+			  "maximum": 30
+			}`)
 
-	// http://schemas.sentex.io/store/category.json
-	// {
-	//   "$id": "http://schemas.sentex.io/store/category.json",
-	//   "$schema": "http://json-schema.org/draft-07/schema#",
-	//   "description": "category name for products",
-	//   "type": "string",
-	//   "pattern": "^[A-Za-z0-9\\-]+$",
-	//   "minimum": 1,
-	//   "maximum": 30
-	// }
+		default:
+			panic(url)
+		}
+		return
+	}
 
-	for _, majmin := range []string{"'3.0'", "'3.1'"} {
+	for _, majmin := range []string{"'3.0'", "'3.1'", "'3.2'"} {
 		t.Run(majmin, func(t *testing.T) {
 			t.Parallel()
-			sl := NewLoader()
-			sl.IsExternalRefsAllowed = true
+			sl := openapi3.NewLoader()
+			sl.ReadFromURIFunc = resolver
 
 			doc, err := sl.LoadFromData([]byte(strings.ReplaceAll(spec, "3.0.1", majmin)))
 			require.NoError(t, err)
 
-			err = doc.Validate(sl.Context, AllowExtraSiblingFields("$id", "$schema"))
+			err = doc.Validate(sl.Context, openapi3.AllowExtraSiblingFields("$id", "$schema"))
 			require.NoError(t, err)
 		})
 	}
@@ -114,7 +124,7 @@ components:
 	for _, majmin := range []string{"3.0", "3.1"} {
 		t.Run(majmin, func(t *testing.T) {
 			t.Parallel()
-			sl := NewLoader()
+			sl := openapi3.NewLoader()
 			doc, err := sl.LoadFromData([]byte(strings.ReplaceAll(spec, "3.0.3", majmin)))
 			require.NoError(t, err)
 			err = doc.Validate(sl.Context)
@@ -165,7 +175,7 @@ components:
 	for _, majmin := range []string{"3.0", "3.1"} {
 		t.Run(majmin, func(t *testing.T) {
 			t.Parallel()
-			sl := NewLoader()
+			sl := openapi3.NewLoader()
 			doc, err := sl.LoadFromData([]byte(strings.ReplaceAll(spec, "3.0.3", majmin)))
 			require.NoError(t, err)
 			require.Contains(t, doc.Paths.Value("/v1/operation").Delete.Responses.Default().Value.Extensions, `x-my-extension`)
@@ -212,7 +222,7 @@ components:
 	for _, majmin := range []string{"3.0", "3.1"} {
 		t.Run(majmin, func(t *testing.T) {
 			t.Parallel()
-			sl := NewLoader()
+			sl := openapi3.NewLoader()
 			doc, err := sl.LoadFromData([]byte(strings.ReplaceAll(spec, "3.0.3", majmin)))
 			require.NoError(t, err)
 			err = doc.Validate(sl.Context)
@@ -258,10 +268,10 @@ components:
 	for _, majmin := range []string{"3.0", "3.1"} {
 		t.Run(majmin, func(t *testing.T) {
 			t.Parallel()
-			sl := NewLoader()
+			sl := openapi3.NewLoader()
 			doc, err := sl.LoadFromData([]byte(strings.ReplaceAll(spec, "3.0.3", majmin)))
 			require.NoError(t, err)
-			err = doc.Validate(sl.Context, AllowExtraSiblingFields("description"))
+			err = doc.Validate(sl.Context, openapi3.AllowExtraSiblingFields("description"))
 			require.NoError(t, err)
 		})
 	}

openapi3/loader.go

@@ -516,13 +516,47 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv
 		if err := codec(cursor, resolved); err != nil {
 			return nil, nil, fmt.Errorf("bad data in %q (expecting %s)", ref, readableType(resolved))
 		}
+		// The value came from a generic map in T.Extensions (a $ref to an
+		// arbitrary top-level key), so the json round-trip above stripped its
+		// origins. Re-attach them from the file, best-effort.
+		loader.attachOriginToResolved(resolved, componentPath, fragment)
 		return componentDoc, componentPath, nil
 
 	default:
 		return nil, nil, fmt.Errorf("bad data in %q (expecting %s)", ref, readableType(resolved))
 	}
 }
 
+// attachOriginToResolved re-attaches source origins to a component resolved
+// through the generic-map path: a $ref to a schema under an arbitrary top-level
+// key lands in T.Extensions, and the json round-trip in resolveComponent strips
+// its origin. It re-derives the component file's origin tree, walks to the ref
+// fragment, and applies that subtree, so the object carries the same origins a
+// typed resolution would, with the original file's line numbers. Best-effort:
+// any failure leaves the object without origins, exactly as before.
+func (loader *Loader) attachOriginToResolved(resolved any, componentPath *url.URL, fragment string) {
+	if !loader.IncludeOrigin || componentPath == nil {
+		return
+	}
+	data, err := loader.readURL(componentPath)
+	if err != nil {
+		return
+	}
+	tree := originTree(data, componentPath)
+	if tree == nil {
+		return
+	}
+	for _, part := range strings.Split(strings.Trim(fragment, "/"), "/") {
+		if part == "" {
+			continue
+		}
+		if tree = tree.Fields[unescapeRefString(part)]; tree == nil {
+			return
+		}
+	}
+	applyOrigins(resolved, tree)
+}
+
 func readableType(x any) string {
 	switch x.(type) {
 	case *Callback:

openapi3/marsh.go

@@ -43,3 +43,24 @@ func unmarshal(data []byte, v any, includeOrigin bool, location *url.URL) error
 	// If both unmarshaling attempts fail, return a new error that includes both errors
 	return fmt.Errorf("failed to unmarshal data: json error: %v, yaml error: %v", jsonErr, yamlErr)
 }
+
+// originTree parses data solely for its origin tree, used to re-attach source
+// origins to a component resolved through the generic-map path: a $ref to a
+// schema under an arbitrary top-level key lands in T.Extensions and loses its
+// origin in the json round-trip (see resolveComponent). Returns nil when parsing
+// fails; callers degrade to no origin.
+func originTree(data []byte, location *url.URL) *yaml.OriginTree {
+	var file string
+	if location != nil {
+		file = location.String()
+	}
+	var v any
+	tree, err := yaml.Unmarshal(data, &v, yaml.DecodeOpts{
+		Origin:            yaml.OriginOpt{Enabled: true, File: file},
+		DisableTimestamps: true,
+	})
+	if err != nil {
+		return nil
+	}
+	return tree
+}

openapi3/origin_external_ref_test.go

@@ -0,0 +1,31 @@
+package openapi3
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// A $ref to a schema stored under an arbitrary top-level key in another file
+// (e.g. "./schemas.yaml#/User", the Swagger-2-era "definitions bag") must carry
+// the origin of the file it lives in, like a $ref into /components/schemas does.
+// The key is not a typed field of T, so the loader reaches it through T.Extensions
+// (a generic map); the origin must survive that path.
+func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) {
+	loader := NewLoader()
+	loader.IncludeOrigin = true
+	loader.IsExternalRefsAllowed = true
+
+	doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml")
+	require.NoError(t, err)
+
+	user := doc.Paths.Value("/users").Get.Responses.Value("200").Value.Content["application/json"].Schema.Value
+	require.NotNil(t, user, "the User schema resolves")
+
+	require.NotNil(t, user.Origin, "a schema $ref'd under an arbitrary top-level key must carry an origin")
+	require.NotNil(t, user.Origin.Key, "the origin has a key location")
+	require.Equal(t, "arbitrary_key_schemas.yaml", filepath.Base(user.Origin.Key.File),
+		"origin points at the file the schema lives in, not the referencing document")
+	require.NotZero(t, user.Origin.Key.Line, "the origin has a source line")
+}

openapi3/testdata/origin/arbitrary_key.yaml

@@ -0,0 +1,14 @@
+openapi: 3.0.0
+info:
+  title: Arbitrary top-level key ref
+  version: 1.0.0
+paths:
+  /users:
+    get:
+      responses:
+        "200":
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "./arbitrary_key_schemas.yaml#/User"

openapi3/testdata/origin/arbitrary_key_schemas.yaml

@@ -0,0 +1,7 @@
+User:
+  type: object
+  properties:
+    id:
+      type: string
+    name:
+      type: string

openapi3filter/req_resp_decoder.go

@@ -932,6 +932,13 @@ func makeObject(props map[string]string, schema *openapi3.SchemaRef) (map[string
 	return result, nil
 }
 
+// maxSliceMapToSliceGap bounds how many synthesized nil holes sliceMapToSlice
+// will fill in for a sparse array before rejecting the input. Without this,
+// an attacker-supplied index (e.g. from a deepObject query parameter) drives
+// an allocation proportional to the index itself, regardless of how many
+// elements were actually provided.
+const maxSliceMapToSliceGap = 10000
+
 // example: map[0:map[key:true] 1:map[key:false]] -> [map[key:true] map[key:false]]
 func sliceMapToSlice(m map[string]any) ([]any, error) {
 	var result []any
@@ -942,6 +949,9 @@ func sliceMapToSlice(m map[string]any) ([]any, error) {
 		if err != nil {
 			return nil, fmt.Errorf("array indexes must be integers: %w", err)
 		}
+		if key < 0 {
+			return nil, fmt.Errorf("array indexes must not be negative: %d", key)
+		}
 		keys = append(keys, key)
 	}
 	max := -1
@@ -950,6 +960,12 @@ func sliceMapToSlice(m map[string]any) ([]any, error) {
 			max = k
 		}
 	}
+	// max+1 is the size of the slice this loop is about to build; bound the
+	// gap between what was actually supplied (len(m)) and that size so a
+	// single huge index can't force an outsized allocation.
+	if gap := max + 1 - len(m); gap > maxSliceMapToSliceGap {
+		return nil, fmt.Errorf("array index %d is too sparse relative to the %d supplied items", max, len(m))
+	}
 	for i := 0; i <= max; i++ {
 		val, ok := m[strconv.Itoa(i)]
 		if !ok {