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

0.22.2 fails to validate *bool false (!) #158

Closed
yktoo opened this issue Nov 21, 2023 · 8 comments
Closed

0.22.2 fails to validate *bool false (!) #158

yktoo opened this issue Nov 21, 2023 · 8 comments

Comments

@yktoo
Copy link

yktoo commented Nov 21, 2023

With the swagger spec:

    post:
      operationId: CommentModerate
      summary: Moderate the specified comment
      tags:
        - ApiGeneral
      parameters:
        - $ref: "#/parameters/pathUuid"
        - in: body
          name: body
          required: true
          schema:
            type: object
            required:
              - pending
              - approve
            properties:
              pending:
                description: Whether the comment is pending moderation
                type: boolean
              approve:
                description: Whether to approve the comment
                type: boolean
      responses:
        204:
          description: Comment has been updated

It apparently expects a true for the required fields (not quite sure). With 0.22.1 it would accept this request:

{
    "pending": false,
    "approve": false
}

With 0.22.2 the validation fails:

{
    "code": 602,
    "message": "body.approve in body is required"
}

I strongly suspect this change to values.go / Required() is the culprit: 348543c

@yktoo yktoo changed the title 0.22.2 generates invalid validation code (!) 0.22.2 fails to validate *bool false (!) Nov 21, 2023
@wawaka
Copy link

wawaka commented Nov 21, 2023

same error happens when number-typed fields have 0 value.
I get error <field> in body is required

@amadorcervera
Copy link

Same error for me with required int64.

@vgogolev1703
Copy link

vgogolev1703 commented Nov 22, 2023

#152
Looks like this PR from 0.22.2 destroys the whole concept of "required" parameters - for example, pointer to int64(0) is not a nil, because there is a value after the pointer. But that PR now does not allow using "0" anywhere at all, even as a pointer. Same applies to strings and bools

I assume, there should be a hotfix ASAP

@jiuker
Copy link
Contributor

jiuker commented Nov 28, 2023

You can use the old version for that.

        - in: body
          name: body
          required: true

you set require: true. That's required. @vgogolev1703 @wawaka @yktoo

@vgogolev1703
Copy link

vgogolev1703 commented Nov 28, 2023

You can use the old version for that.

Sorry, that the same if I told you, that you can fork this repo for your purposes)

This package is used in go-swagger package, which generates API components based on swagger spec and frequently used in microservices. In go-swagger, require: true generates pointers values to make sure, that the value is not nil, but contains some value (even null value of primitive, means "", 0, false etc)

Currently, this issue brokes "required" mechanism at all, cause pointer to 0 is now invalid. So your modification doesn't allow to use null values of primitive types as required parameters.

For example, we have OrderStatus int64 where 0 is 'New'. And it's required, because every order should have a status. But after your change, we can't use it, cause validator says, that is not valid.

The same applies to boolean pointers (which is totally strange), which means all required boolean values must be true??? Cause pointer to boolean with false value is not longer valid

@jiuker
Copy link
Contributor

jiuker commented Nov 28, 2023

You can use the old version for that.

Sorry, that the same if I told you, that you can fork this repo for your purposes)

This package is used in go-swagger package, which generates API components based on swagger spec and frequently used in microservices. In go-swagger, require: true generates pointers values to make sure, that the value is not nil, but contains some value (even null value of primitive, means "", 0, false etc)

Currently, this issue brokes "required" mechanism at all, cause pointer to 0 is now invalid. So your modification doesn't allow to use null values of primitive types as required parameters.

For example, we have OrderStatus int64 where 0 is 'New'. And it's required, because every order should have a status. But after your change, we can't use it, cause validator says, that is not valid.

The same applies to boolean pointers (which is totally strange), which means all required boolean values must be true??? Cause pointer to boolean with false value is not longer valid

Ok. I agree that. Let's revert.

@yktoo
Copy link
Author

yktoo commented Nov 28, 2023

You can use the old version for that.

        - in: body
          name: body
          required: true

you set require: true. That's required. @vgogolev1703 @wawaka @yktoo

Correct. But required means a value must be present and can be either true or false. At the moment all zero values are considered as failing the requirement.

I agree this better be reverted.

@fredbi
Copy link
Member

fredbi commented Dec 4, 2023

fixed by reverting this commit in v0.22.3

@fredbi fredbi closed this as completed Dec 4, 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

6 participants