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

invalid components error when response schema contains additionalProperties #772

Closed
anjaliSeven opened this issue Feb 22, 2023 · 13 comments
Closed

Comments

@anjaliSeven
Copy link

anjaliSeven commented Feb 22, 2023

Given a specification with additional properties in the response schema,
returns error "invalid components: schema "schema1": extra sibling fields: [has schema]"

example:

openapi: 3.0.0
info:
  title: "Service"
  version: 0.1.9
paths:
  /service:
    get:
      responses:
        "200":
          $ref: "#/components/responses/response1"

components:
  responses:
    response1:
      description: response data
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/schema1"

  schemas:
    schema1:
      properties:
        "":
          type: object
          additionalProperties:
            type: string
      type: object
      
    
@fenollp
Copy link
Collaborator

fenollp commented Feb 22, 2023

# issue772.yml
openapi: 3.0.0
info:
  title: "Service"
  version: 0.1.9
paths:
  /service:
    get:
      responses:
        "200":
          $ref: "#/components/responses/response1"

components:
  responses:
    response1:
      description: response data
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/schema1"

  schemas:
    schema1:
      properties:
        "":
          type: object
          additionalProperties:
            type: string
      type: object
go run github.com/getkin/kin-openapi/cmd/validate@latest issue772.yml
# go: downloading github.com/getkin/kin-openapi v0.114.0
# ...

No issue.

@fenollp fenollp closed this as completed Feb 22, 2023
@anjaliSeven
Copy link
Author

anjaliSeven commented Feb 23, 2023

Hi @fenollp as in v0.113.0 AdditionalProperties has been added with the additional field so the validation is failing for me after the marshalling. It would be very helpful if u can help in marshalling it properly.

func Test(t *testing.T) {
	before, err := os.ReadFile("issue772.yml")
	assert.NoError(t, err)

	loader := openapi3.NewLoader()
	loader.IsExternalRefsAllowed = true
	spec, err := loader.LoadFromData(before)
	assert.NoError(t, err)

	after, err := yaml.Marshal(spec)
	assert.NoError(t, err)
	f, err := os.Create("after.yml")
	assert.NoError(t, err)
	f.WriteString(string(after))

	doc, err := loader.LoadFromFile("after.yml")
	assert.NoError(t, err)
	err = doc.Validate(context.Background())
        t.Log("error from validation", err)
	assert.NoError(t, err)
}

--- FAIL: Test (0.00s)
error from validation invalid components: schema "schema1": extra sibling fields: [has schema]
 Error:      	Received unexpected error:
        	            	invalid components: schema "schema1": extra sibling fields: [has schema]

@fenollp
Copy link
Collaborator

fenollp commented Feb 23, 2023

  1. I have no idea what yaml package you're using
  2. Print the problematic YAML file (the after), we should find your issue much faster.

@fenollp fenollp reopened this Feb 23, 2023
@anjaliSeven
Copy link
Author

anjaliSeven commented Feb 24, 2023

package used:

	"gopkg.in/yaml.v3"

	"github.com/getkin/kin-openapi/openapi3"
	"github.com/stretchr/testify/assert"
#after.yml
openapi: 3.0.0
components:
    schemas:
        schema1:
            type: object
            properties:
                "":
                    type: object
                    additionalProperties:
                        has: null
                        schema:
                            type: string
    responses:
        response1:
            description: response data
            content:
                application/json:
                    schema:
                        $ref: '#/components/schemas/schema1'
info:
    title: Service
    version: 0.1.9
paths:
    /service:
        get:
            responses:
                "200":
                    $ref: '#/components/responses/response1'

@anjaliSeven
Copy link
Author

anjaliSeven commented Feb 24, 2023

additionalProperties:
            type: string

when converted to struct and then marshalled is changed to

additionalProperties:
                       has: null
                       schema:
                           type: string
                         

@praneetloke
Copy link
Contributor

praneetloke commented Feb 27, 2023

I am seeing some weirdness too since v0.113.0 that I think is similar to the issue reported here. Specifically, the changes in #728 seems to be triggering the error. For example, this path:

yaml
paths:
  /owners/{id}:
    get:
      tags:
        - Owners
      summary: Retrieve user or team
      description: >-
        [https://api-docs.render.com/reference/get-owner](https://api-docs.render.com/reference/get-owner)
      parameters:
        - name: Accept
          in: header
          schema:
            type: string
          example: application/json
        - name: id
          in: path
          schema:
            type: string
          required: true
          description: (Required) The ID of the user or team
          example: Lorem amet sin
      responses:
        "200":
          description: OK
          headers:
            Content-Type:
              schema:
                type: string
                example: application/json
          content:
            application/json:
              schema:
                type: object
                $ref: "#/components/schemas/owner"
              example:
                id: nisi aute veniam elit
                name: labore
                email: id adipisicing Excepteur
                type: user
components:
  schemas:
    owner:
      title: "Owner"
      description: "The owner object represents an authorized user or team. The `type` property indicates if the owner is a user or team."
      type: object
      properties:
        id:
          description: The owner ID.
          readOnly: true
          type: string
        name:
          description: The name of the owner.
          type: string
        email:
          description: The email of the owner.
          type: string
        type:
          description: The type of the authorized user.
          type: string
          enum: ["user", "team"]

...fails validation with the error:

invalid paths: invalid path /owners/{id}: invalid operation GET: extra sibling fields: [type]

And when that doc is parsed, the SchemaRef for #/components/schemas/owner looks like this:

image

I am not sure how the extra slice is being populated on doc parse since I haven't done a deeper investigation into the parsing routine yet. I am also trying to understand the changes in #728.

@fenollp
Copy link
Collaborator

fenollp commented Feb 27, 2023

Hi, sorry I haven't got much time to look into this right now.
I believe you are right, the issue comes from YAML serialization. The issue should be around this code:

type AdditionalProperties struct {
Has *bool
Schema *SchemaRef
}
// MarshalJSON returns the JSON encoding of AdditionalProperties.
func (addProps AdditionalProperties) MarshalJSON() ([]byte, error) {
if x := addProps.Has; x != nil {
if *x {
return []byte("true"), nil
}
return []byte("false"), nil
}
if x := addProps.Schema; x != nil {
return json.Marshal(x)
}
return nil, nil
}

A couple leads: json+yaml tags may need to be added to the struct. But I believe a better choice is to implement a MarshalYAML func.

@anjaliSeven You should be able to resolve your issue by replacing the YAML package you're using with github.com/invopop/yaml

@praneetloke Take a look around

x.extra = append(x.extra, key)

@praneetloke
Copy link
Contributor

@fenollp thanks for the tip! It turned out that my API spec was actually incorrect. The type property is unnecessary when a ref is provided. There were other issues as well such as the example property appearing as a child of schema rather than a sibling. So it looks like "disallow unknown properties" is actually working as intended in my case at least.

@anjaliSeven
Copy link
Author

@fenollp thanks for the suggestion , will try it

@anjaliSeven
Copy link
Author

using github.com/invopop/yaml for marshalling doesn't provide same result as gopkg.in/yaml.v3 for my usecase.

@fenollp
Copy link
Collaborator

fenollp commented Mar 1, 2023

doesn't provide same result

Yes that's the point. Show us the diff and be helpful.

@anjaliSeven
Copy link
Author

@fenollp when marshalled with Invopop, one of the fields returns null. I haven't investigated the reason yet, so I haven't shared any examples. I will share them once I complete my investigation.

@fenollp
Copy link
Collaborator

fenollp commented Nov 26, 2023

@anjaliSeven Heya, okay. Please reopen this once you do :)

@fenollp fenollp closed this as completed Nov 26, 2023
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

3 participants