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

ContextValidate panics if the field is nil for discriminator types #2911

Closed
romainbou opened this issue Mar 14, 2023 · 2 comments
Closed

ContextValidate panics if the field is nil for discriminator types #2911

romainbou opened this issue Mar 14, 2023 · 2 comments
Labels
discriminator Related to polymorphic types model Related to swagger generate model command validator Related to codegen generation of validations

Comments

@romainbou
Copy link
Contributor

romainbou commented Mar 14, 2023

Problem statement

The context validation expects a field to be set and can panic if it's nil. I expect it to skip the validation if it's nil.

Swagger specification

pets.yml

definitions:
  animal:
    type: 'object'
    properties:
      kind:
        $ref: '#/definitions/pet'
  pet:
    type: object
    required: [type]
    discriminator: type
    properties:
      type:
        $ref: '#/definitions/petType'

  petType:
    type: string
    enum:
      - dog
      - cat

  dog:    
    allOf:
      - $ref: '#/definitions/pet'
        type: object
  cat:
    allOf:
      - $ref: '#/definitions/pet'
        type: object

The generated model for animal.go as the following validator since go-swagger v0.27.0

func (m *Animal) contextValidateKind(ctx context.Context, formats strfmt.Registry) error {

	if err := m.Kind().ContextValidate(ctx, formats); err != nil {
// ...

Which panics if m.Kind() == nil

While the check is done in the field's validation

func (m *Animal) validateKind(formats strfmt.Registry) error {
	if swag.IsZero(m.Kind()) { // not required
		return nil
	}
// ...

Steps to reproduce

swagger -q generate model \
	 	--accept-definitions-only \
		--model-package=pkg/models_test \
		--target=./ \
		--spec=pets.yml

Go test:

package models_test

import (
	"context"
	"testing"

	"github.com/go-openapi/strfmt"
	"gopkg.in/go-playground/assert.v1"
)

func TestAnimal(t *testing.T) {
	animal := Animal{
		Name: "Fido",
	}
	formats := strfmt.NewFormats()
	err := animal.validateKind(formats)
	assert.Equal(t, nil, err)
	err = animal.contextValidateKind(context.Background(), formats)
	assert.Equal(t, nil, err)
}

The first assert pass, the second panics:

--- FAIL: TestAnimal (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x86ba1d]

goroutine 6 [running]:
testing.tRunner.func1.2({0x8c83e0, 0xcd2e70})
	/usr/lib/go/src/testing/testing.go:1526 +0x24e
testing.tRunner.func1()
	/usr/lib/go/src/testing/testing.go:1529 +0x39f
panic({0x8c83e0, 0xcd2e70})
	/usr/lib/go/src/runtime/panic.go:884 +0x213
models_test.(*Animal).contextValidateKind(0xc0000d5d40?, {0x9fc718?, 0xc00002a130?}, {0x9fdcf0?, 0xc0002ae8a0?})
	models_test/animal.go:150 +0x7d
models_test.TestAnimal(0x0?)
	models_test/animal_test.go:23 +0x1cf
testing.tRunner(0xc0000d5d40, 0x97dc68)
	/usr/lib/go/src/testing/testing.go:1576 +0x10b
created by testing.(*T).Run
	/usr/lib/go/src/testing/testing.go:1629 +0x3ea
FAIL	models_test	0.006s

Environment

swagger version: 0.30.4
go version: 1.20
OS: Manjaro Linux 6.1.12

Potential fix

romainbou@76553bf

If this makes sense and doesn't have unforeseen consequences, I can make a PR.

Alternatively, would there be an option to disable contextValidate?
Thanks!

@youyuanwu
Copy link
Member

Looks like a valid bug. PR welcome.

romainbou added a commit to romainbou/go-swagger that referenced this issue Apr 23, 2023
romainbou added a commit to romainbou/go-swagger that referenced this issue Apr 23, 2023
…generated code

Signed-off-by: Romain Bouyé <romain@bouye.fr>
casualjim added a commit that referenced this issue Apr 26, 2023
Skip `ContextValidate` when the field is `nil` avoiding a panic fixing #2911
@fredbi fredbi added validator Related to codegen generation of validations model Related to swagger generate model command discriminator Related to polymorphic types labels Dec 21, 2023
@fredbi
Copy link
Contributor

fredbi commented Dec 29, 2023

fixed by #2911 merged. Closing.

@fredbi fredbi closed this as completed Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discriminator Related to polymorphic types model Related to swagger generate model command validator Related to codegen generation of validations
Projects
None yet
Development

No branches or pull requests

3 participants