From 88b69495c0d6a1547667b9ecd839eee8879772f5 Mon Sep 17 00:00:00 2001 From: tx3stn <14163530+tx3stn@users.noreply.github.com> Date: Mon, 17 Nov 2025 19:07:40 +0000 Subject: [PATCH] handle errors when a reference has sibling properties --- bundler/bundler.go | 7 +- bundler/bundler_composer.go | 18 ++- bundler/bundler_strict_validation_test.go | 136 ++++++++++++++++++++++ 3 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 bundler/bundler_strict_validation_test.go diff --git a/bundler/bundler.go b/bundler/bundler.go index f9b38bf7..13af2784 100644 --- a/bundler/bundler.go +++ b/bundler/bundler.go @@ -84,7 +84,8 @@ func BundleDocument(model *v3.Document) ([]byte, error) { // BundleCompositionConfig is used to configure the composition of OpenAPI documents when using BundleDocumentComposed. type BundleCompositionConfig struct { - Delimiter string // Delimiter is used to separate clashing names. Defaults to `__`. + Delimiter string // Delimiter is used to separate clashing names. Defaults to `__`. + StrictValidation bool // StrictValidation will cause bundling to fail on invalid OpenAPI specs (e.g. $ref with siblings) } // BundleDocumentComposed will take a v3.Document and return a composed bundled version of it. Composed means @@ -134,7 +135,9 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([] compositionConfig: compositionConfig, discriminatorMappings: discriminatorMappings, } - handleIndex(cf) + if err := handleIndex(cf); err != nil { + return nil, err + } processedNodes := orderedmap.New[string, *processRef]() var errs []error diff --git a/bundler/bundler_composer.go b/bundler/bundler_composer.go index a0fa0fa0..06ea6439 100644 --- a/bundler/bundler_composer.go +++ b/bundler/bundler_composer.go @@ -5,6 +5,7 @@ package bundler import ( "context" + "fmt" "strings" "sync" @@ -38,7 +39,7 @@ type handleIndexConfig struct { // handleIndex will recursively explore the indexes and their references, building a map of references // to be processed later. It will also check for circular references and avoid infinite loops. // everything is stored in the handleIndexConfig, which is passed around to avoid passing too many parameters. -func handleIndex(c *handleIndexConfig) { +func handleIndex(c *handleIndexConfig) error { mappedReferences := c.idx.GetMappedReferences() sequencedReferences := c.idx.GetRawReferencesSequenced() var indexesToExplore []*index.SpecIndex @@ -46,6 +47,16 @@ func handleIndex(c *handleIndexConfig) { for _, sequenced := range sequencedReferences { mappedReference := mappedReferences[sequenced.FullDefinition] + // Check for invalid sibling properties if strict validation is enabled + if c.compositionConfig.StrictValidation && sequenced.HasSiblingProperties { + siblingKeys := make([]string, 0, len(sequenced.SiblingProperties)) + for key := range sequenced.SiblingProperties { + siblingKeys = append(siblingKeys, key) + } + return fmt.Errorf("invalid OpenAPI specification: $ref cannot have sibling properties. Found $ref '%s' with siblings %v at line %d, column %d", + sequenced.FullDefinition, siblingKeys, sequenced.Node.Line, sequenced.Node.Column) + } + // if we're in the root document, don't bundle anything. refExp := strings.Split(sequenced.FullDefinition, "#/") var foundIndex *index.SpecIndex @@ -90,8 +101,11 @@ func handleIndex(c *handleIndexConfig) { for _, idx := range indexesToExplore { c.idx = idx - handleIndex(c) + if err := handleIndex(c); err != nil { + return err + } } + return nil } // processReference will extract a reference from the current index, and transform it into a first class diff --git a/bundler/bundler_strict_validation_test.go b/bundler/bundler_strict_validation_test.go new file mode 100644 index 00000000..d66ade42 --- /dev/null +++ b/bundler/bundler_strict_validation_test.go @@ -0,0 +1,136 @@ +package bundler + +import ( + "testing" + + "github.com/pb33f/libopenapi/datamodel" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStrictValidation_RefWithSiblings_ShouldError(t *testing.T) { + spec := `openapi: 3.0.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' + description: This is invalid - $ref cannot have siblings +components: + schemas: + TestSchema: + type: object + properties: + name: + type: string` + + config := &BundleCompositionConfig{ + StrictValidation: true, + } + + docConfig := &datamodel.DocumentConfiguration{ + AllowFileReferences: false, + } + + _, err := BundleBytesComposed([]byte(spec), docConfig, config) + + require.Error(t, err, "Strict validation must fail on invalid $ref siblings") + assert.Contains( + t, + err.Error(), + "invalid OpenAPI specification: $ref cannot have sibling properties", + ) + assert.Contains(t, err.Error(), "siblings [description]") + assert.Contains(t, err.Error(), "line 14") + assert.Contains(t, err.Error(), "column 17") +} + +func TestStrictValidation_RefWithoutSiblings_ShouldSucceed(t *testing.T) { + spec := `openapi: 3.0.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + TestSchema: + type: object + properties: + name: + type: string` + + config := &BundleCompositionConfig{ + StrictValidation: true, + } + + docConfig := &datamodel.DocumentConfiguration{ + AllowFileReferences: false, + } + + result, err := BundleBytesComposed([]byte(spec), docConfig, config) + + require.NoError(t, err, "Valid $ref without siblings should succeed") + assert.NotNil(t, result) + assert.True(t, len(result) > 0) +} + +func TestStrictValidation_Disabled_ShouldNotError(t *testing.T) { + spec := `openapi: 3.0.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' + description: This would be invalid with strict validation +components: + schemas: + TestSchema: + type: object + properties: + name: + type: string` + + config := &BundleCompositionConfig{ + StrictValidation: false, // Disabled - should not error + } + + docConfig := &datamodel.DocumentConfiguration{ + AllowFileReferences: false, + } + + result, err := BundleBytesComposed([]byte(spec), docConfig, config) + + require.NoError(t, err, "Disabled strict validation should allow invalid siblings") + assert.NotNil(t, result) +} + +func TestBundleCompositionConfig_DefaultValues(t *testing.T) { + config := &BundleCompositionConfig{} + assert.False(t, config.StrictValidation) + assert.Empty(t, config.Delimiter) +}