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

OpenAPI validation fails on Multiple methods defined on the same path #882

Closed
h6ah4i opened this issue Apr 16, 2024 · 9 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@h6ah4i
Copy link
Contributor

h6ah4i commented Apr 16, 2024

In OpenAPI, we can declare multiple methods under an endpoint path like this.

/users:
    get:
        ...
    post:
        ...

However, runn gives error like the following;

Failure/Error: http request failed on "***: openapi3 validation error: Error: POST operation request content type 'POST' does not exist, Reason: The path was found, but there was no 'POST' method found in the spec, Line: ***, Column: ***

ref.) https://swagger.io/docs/specification/describing-responses/

(I have not confirmed yet, but it seems pb33f/libopenapi does not support the syntax.)

@k1LoW
Copy link
Owner

k1LoW commented Apr 16, 2024

@h6ah4i Thank you for yor report!

Are you talking about the same structure as this one? ( get and put methods )

/users/{id}:
get:
parameters:
- description: ID
explode: false
in: path
name: id
required: true
schema:
type: string
responses:
'200':
description: OK
content:
application/json:
schema:
properties:
data:
type: object
properties:
username:
type: string
required:
- username
- email
required:
- data
put:

@h6ah4i
Copy link
Contributor Author

h6ah4i commented Apr 16, 2024

Are you talking about the same structure as this one? ( get and put methods )

It might be something different from the current test code. I'll create a sample code to reproduce the issue.

@k1LoW
Copy link
Owner

k1LoW commented Apr 18, 2024

I'll create a sample code to reproduce the issue.

@h6ah4i 🙏 Thank you!!

h6ah4i added a commit to h6ah4i/runn that referenced this issue Apr 20, 2024
@h6ah4i
Copy link
Contributor Author

h6ah4i commented Apr 20, 2024

@k1LoW
I have managed to create a minimal test code to reproduce this issue. It seems reusing the same OpenAPI validator object and nullable are related to the issue, and declaring multiple methods on the same path is not required. Could you take a look at the following code? Thank you.

c0c7714

@k1LoW
Copy link
Owner

k1LoW commented Apr 21, 2024

@h6ah4i Thank you!!

@k1LoW
Copy link
Owner

k1LoW commented Apr 22, 2024

A workaround has been added to this issue.
We will continue to investigate libopenapi-validator to fix the root cause.

@k1LoW k1LoW self-assigned this Apr 22, 2024
@k1LoW k1LoW added the patched label Apr 24, 2024
@h6ah4i
Copy link
Contributor Author

h6ah4i commented Apr 29, 2024

@k1LoW In v0.105.1, still I get so many OpenAPI validation errors like the followings in my setup.

  • openapi3 validation error: Error: POST operation request content type 'POST' does not exist, Reason: The path was found, but there was no 'POST' method found in the spec, Line: ***, Column: ***
  • openapi3 validation error: Error: GET operation request content type 'GET' does not exist, Reason: The path was found, but there was no 'GET' method found in the spec, Line: ***, Column: ***
  • openapi3 validation error: Error: Query parameter '***' is missing, Reason: The query parameter '***' is defined as being required, however it's missing from the requests, Line: ***, Column: ***
  • openapi3 validation error: Error: POST request body for '***' failed to validate schema, Reason: The request body is defined as an object. However, it does not meet the schema requirements of the specification, Validation Errors: [Reason: missing properties: '***', '***', '***', '***', '***', Location: /required], Line: ***, Column: ***

However, I noticed a very important signal that triggers this issue today. These errors only occurs when I use the --concurrent on option.

It seems that the validator instances of libopenapi-validator are not thread-safe (maybe they are not immutable object, says it has internal states). Basically, they should not be shared among threads. Otherwise you need to implement appropriate synchronization mechanism by using mutex or something else.

I suppose the fix of the issue #889 was not perfect. It can avoid panics, but it is not sufficient to protect from concurrency access to the validator instances.

@k1LoW
Copy link
Owner

k1LoW commented Apr 30, 2024

@h6ah4i Thank you for your report!

Fixed to have a validator for each HTTP runner (v0.105.2) and added the following tests.

runn/http_validator_test.go

Lines 762 to 801 in 0f97e31

for i, tt := range tests {
for range 10 {
for j, v := range []*openAPI3Validator{v, v2, v3} {
fn := func(tt testset, i, j int) func() error {
return func() error {
t.Run(fmt.Sprintf("%d-%d", i, j), func(t *testing.T) {
req := tt.req.Clone(ctx)
req.Body = io.NopCloser(bytes.NewReader(reqbs[i]))
res := &http.Response{
StatusCode: tt.res.StatusCode,
Header: tt.res.Header.Clone(),
Body: io.NopCloser(bytes.NewReader(resbs[i])),
}
if err := v.ValidateRequest(ctx, req); err != nil {
if !tt.wantErr {
t.Errorf("got error: %v", err)
}
return
}
if err := v.ValidateResponse(ctx, req, res); err != nil {
if !tt.wantErr {
t.Errorf("got error: %v", err)
}
return
}
if tt.wantErr {
t.Error("want error")
}
})
return nil
}
}(tt, i, j)
eg.Go(fn)
}
}
}
if err := eg.Wait(); err != nil {
t.Error(err)
}
}

@h6ah4i
Copy link
Contributor Author

h6ah4i commented May 1, 2024

@k1LoW Thank you so much! I have confirmed that v0.105.2 resolved all OpenAPI validation errors in my setup 🎉

@k1LoW k1LoW added the patched label May 1, 2024
@k1LoW k1LoW removed the patched label Jun 20, 2024
@k1LoW k1LoW closed this as completed Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants