pb33f / libopenapi-validator

OpenAPI validation extension for libopenapi, validate http requests and responses as well as schemas
https://pb33f.io/libopenapi/validation/
Other
57 stars 20 forks source link

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

Open vanntile opened 6 days ago

vanntile commented 6 days ago

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

https://github.com/pb33f/libopenapi-validator/blob/7b4e021e253dae5c215da2c29b5d2862190df7b0/responses/validate_response.go#L131-L132

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 commented 3 days ago

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

daveshanley commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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.