Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response validation panic when body uses array circular reference field #103

Open
vanntile opened this issue Nov 11, 2024 · 6 comments
Open

Comments

@vanntile
Copy link

vanntile commented Nov 11, 2024

I consistently get a panic in a production use case. Here is a minimal reproduction:

package main

import (
	"fmt"
	"io"
	"net/http"
	"net/url"
	"strings"

	"github.com/pb33f/libopenapi"
	validator "github.com/pb33f/libopenapi-validator"
)

func main() {
	spec := `openapi: 3.1.0
info:
  title: Panic at response validation
  version: 1.0.0
paths:
  /operations:
    delete:
      description: Delete operations
      responses:
        default:
          description: Any response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Error'

components:
  schemas:
    Error:
      type: object
      properties:
        code:
          type: string
        details:
          type: array
          items:
            $ref: '#/components/schemas/Error'
`

	document, err := libopenapi.NewDocument([]byte(spec))
	if err != nil {
		panic(fmt.Sprintf("failed to create new document: %v\n", err))
	}

	_, errs := document.BuildV3Model()
	if len(errs) > 0 {
		for i := range errs {
			fmt.Printf("model error: %e\n", errs[i])
		}
		panic(fmt.Sprintf("failed to create v3 model from document: %d errors reported", len(errs)))
	}

	fmt.Println("Successfully parsed OpenAPI spec")

	oapiValidator, errs := validator.NewValidator(document)
	if errs != nil {
		panic(fmt.Sprintf("failed to create validator: %v", errs))
	}
	if ok, errs := oapiValidator.ValidateDocument(); !ok {
		panic(fmt.Sprintf("document validation errors: %v", errs))
	}

	req := &http.Request{
		Method: http.MethodDelete,
		URL: &url.URL{
			Path: "/operations",
		},
	}
	res := &http.Response{
		StatusCode: http.StatusOK,
		Header: map[string][]string{
			"Content-Type": {"application/json"},
		},
		Body: io.NopCloser(strings.NewReader(`{"code":"abc","details":[{"code":"def"}]}`)),
	}
	if ok, errs := oapiValidator.ValidateHttpResponse(req, res); !ok {
		panic(fmt.Sprintf("response validation erros: %v", errs))
	}
}

This results in:

Successfully parsed OpenAPI spec
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x8937f1]

goroutine 1 [running]:
github.com/santhosh-tekuri/jsonschema/v6.unevalFrom({0x90be20, 0xc0004dd500}, 0x0, 0x0)
        /home/vanntile/.go/pkg/mod/github.com/santhosh-tekuri/jsonschema/v6@v6.0.1/validator.go:922 +0x131
github.com/santhosh-tekuri/jsonschema/v6.(*Schema).validate(0x0, {0x90be20, 0xc0004dd500}, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/vanntile/.go/pkg/mod/github.com/santhosh-tekuri/jsonschema/v6@v6.0.1/validator.go:25 +0xa9
github.com/santhosh-tekuri/jsonschema/v6.(*Schema).Validate(...)
        /home/vanntile/.go/pkg/mod/github.com/santhosh-tekuri/jsonschema/v6@v6.0.1/validator.go:16
github.com/pb33f/libopenapi-validator/responses.ValidateResponseSchema(0xc0004312c0, 0xc0004d8240, 0xc00038e288, {0xc000394300, 0x97, 0x100}, {0xc0005665a0, 0x82, 0xc000123c20?})
        /home/vanntile/.go/pkg/mod/github.com/pb33f/libopenapi-validator@v0.2.2/responses/validate_response.go:135 +0xebd
github.com/pb33f/libopenapi-validator/responses.(*responseBodyValidator).checkResponseSchema(0xc0004086c0, 0xc0004312c0, 0xc0004d8240, {0xa1cfed?, 0x1?}, 0xc00040b680)
        /home/vanntile/.go/pkg/mod/github.com/pb33f/libopenapi-validator@v0.2.2/responses/validate_body.go:160 +0x2a7
github.com/pb33f/libopenapi-validator/responses.(*responseBodyValidator).ValidateResponseBodyWithPathItem(0xc0004086c0, 0xc0004312c0, 0xc0004d8240, 0x90?, {0xc0002d7b45, 0xb})
        /home/vanntile/.go/pkg/mod/github.com/pb33f/libopenapi-validator@v0.2.2/responses/validate_body.go:92 +0x6ab
github.com/pb33f/libopenapi-validator.(*validator).ValidateHttpResponse(0xc000402910, 0xc0004312c0, 0xc0004d8240)
        /home/vanntile/.go/pkg/mod/github.com/pb33f/libopenapi-validator@v0.2.2/validator.go:124 +0x59
main.main()
        /home/vanntile/src/github.com/vanntile/libopenapi-validator-test/main.go:80 +0x34f
exit status 2

Considering that this library does not consider (non-required) circular references errors, and both building a model and validating it pass, I am wondering if this is just a bug or I am missing something.

Tracking down the error, I got to

_ = compiler.AddResource(fName, decodedSchema)
jsch, _ := compiler.Compile(fName)

compilation error: json-pointer in "file:///home/vanntile/src/github.com/vanntile/libopenapi-validator-test/response.json#/components/schemas/Error" not found

Some error validation would be useful, don't know at the moment how to solve

@daveshanley
Copy link
Member

Hmm... this is a bug, the schema should bundled inline so there is no ref.

@daveshanley
Copy link
Member

daveshanley commented Nov 14, 2024

Actually. This is not a bug. Well kinda. but not..

#/components/schemas/Error is a circular reference.

It cannot be rendered, so when it's passed to the validator, it remains as is..

{
  "properties" : {
    "code" : {
      "type" : "string"
    },
    "details" : {
      "items" : {
        "$ref" : "#/components/schemas/Error"
      },
      "type" : "array"
    }
  },
  "type" : "object"
}

This is what the validator gets. which is why it's failing.

The fix here is to either fail gracefully, warning of the circular reference.. or, determine it's a circular reference and then extract those circular references and add them to the schema validator one by one as resources, but this is a huge amount of work.

@vanntile
Copy link
Author

vanntile commented Nov 14, 2024

How come? It's at least missing proper validation. As any non-stdlib library, I don't think it should panic, at least

Sorry, pressed return too quickly.

@vanntile
Copy link
Author

The fix here is to either fail gracefully, warning of the circular reference.. or, determine it's a circular reference and then extract those circular references and add them to the schema validator one by one as resources, but this is a huge amount of work.

If you consider it non-critical, I could try to do that work myself. The original problem has been sidestepped in production, so I could just try to improve this for its own sake. PR allowed?

@daveshanley
Copy link
Member

When you parse the document, it will warn you there are circular references

model, errs := document.BuildV3Model()
circ := model.Index.GetCircularReferences()

It will show you something like this:

Screenshot 2024-11-14 at 3 37 38 PM

@daveshanley
Copy link
Member

If you consider it non-critical, I could try to do that work myself. The original problem has been sidestepped in production, so I could just try to improve this for its own sake. PR allowed?

All PRs are most welcome. I only have some many hours in the week to work on new stuff or upgrade existing stuff - so your contribution would be most welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants