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

Validation seems to fail for boolean 'false' #713

Closed
elfner opened this issue Jan 4, 2021 · 12 comments
Closed

Validation seems to fail for boolean 'false' #713

elfner opened this issue Jan 4, 2021 · 12 comments

Comments

@elfner
Copy link

elfner commented Jan 4, 2021

Package version:

v10

Issue, Question or Enhancement:

Seems that validation fails on boolean types when value is passed as 'false', but works for 'true'. Appreciate any insight - thanks!

$ curl -s -X POST -H 'Content-Type: application/json' http://localhost:8080/doFoo -d '{"fieldx":true}'
{
    "fieldx": true
}
$ curl -s -X POST -H 'Content-Type: application/json' http://localhost:8080/doFoo -d '{"fieldx":false}'
{
    "message": "Key: 'something.Fieldx' Error:Field validation for 'Fieldx' failed on the 'required' tag"
}

Postman submission gives the same result.

Code sample, to showcase or reproduce:

package main

import (
	"net/http"
	"encoding/json"

	"github.com/go-playground/validator/v10"
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
)

type (
	CustomValidator struct {
		validator *validator.Validate
	}

	something struct {
                Fieldx  bool `json:"fieldx" validate:"required"`
	}
)

func (cv *CustomValidator) Validate(i interface{}) error {
	return cv.validator.Struct(i)
}

func doFoo (c echo.Context) (err error) {
	req := new(something)
	if err := c.Bind(req); err != nil {
		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
	}
	if err := c.Validate(req); err != nil {
		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
	}
	dataString, err := json.Marshal(req)
        if err != nil {
                return c.String(http.StatusBadRequest, "failed to marshal JSON")
        }
	return c.String(http.StatusOK, string(dataString))
}

func main() {
	e := echo.New()
	e.Validator = &CustomValidator{validator: validator.New()}

	// Middleware
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())

	// Routes
	e.POST("/doFoo", doFoo)

	// Start server
	e.Logger.Fatal(e.Start(":8080"))
}

@4n70w4
Copy link

4n70w4 commented Jun 3, 2021

the same issue

@joebartels
Copy link

joebartels commented Jun 4, 2021

by default required will fail when the value is zero value. so with that in mind, it makes sense to fail on false

however, if true and false are valid values then you don't really need a validate:"required" tag at all

see https://pkg.go.dev/gopkg.in/go-playground/validator.v10#hdr-Required

vargax added a commit to vargax/midas-echo that referenced this issue Jun 27, 2021
By default catalogo will be private, unless EsPublico explicitly set to True
go-playground/validator#713
@pintobikez
Copy link

Same issue here. validate:"boolean" fails whichever value (true/false) is passed in a json request

@9r1nc3w1ll
Copy link

Same issue here. validate:"boolean" fails whichever value (true/false) is passed in a json request

I am having the same issue.

@MuShaf-NMS
Copy link

Same issue here. validate:"boolean" fails whichever value (true/false) is passed in a json request

I am having the same issue.

Me too. Anyone can help me?

@deankarn
Copy link
Contributor

deankarn commented Jun 9, 2022

Forgive the short reply, answering from phone.

Please read the documentation for required and it should make sense why false fails the required check AND this was explained above by @joebartels

Please let me know if I’m missing something and something else is wrong.

@zemzale
Copy link
Member

zemzale commented Sep 5, 2022

If you want to validate that the value is present you will have to use a pointer there.

So for the example given, the change should be something like this.

	something struct {
-               Fieldx  bool `json:"fieldx" validate:"required"`
+               Fieldx  *bool `json:"fieldx" validate:"required"`
	}

I will close this as answered.

@zemzale zemzale closed this as completed Sep 5, 2022
@vansenic
Copy link

vansenic commented Jan 8, 2023

You can use this way:

Name  *bool `json:"name" validate:"required"`

Use a pointer

thanks @zemzale

@SharkFourSix
Copy link

by default required will fail when the value is zero value. so with that in mind, it makes sense to fail on false

however, if true and false are valid values then you don't really need a validate:"required" tag at all

see https://pkg.go.dev/gopkg.in/go-playground/validator.v10#hdr-Required

This is extremely dangerous because if a caller forgets/does not to specify a field when it is required by contract then it would lead to unintended consequences.

This is very bad design.

@deankarn
Copy link
Contributor

@SharkFourSix not sure how long you’ve been using Go but there is no concept of an uninitialized variable and so ever variable will get a default value including when unmarshalling data into a struct.

and so to that end there are two options:

  1. Design so the default types work in your favour eg. If not present the default makes sense.(which is good design in and language)
  2. Provide the ability to detect the difference between presence and default values. This can be done in several ways including using a pointer and using a Nullable type like sql.NullBool or equivalent to keep this on the stack when possible in concert with Registering the custom type with validator.

I think you may be confusing bad design with the facts and semantics of the Go language. I would be glad to be wrong but as far as I know there is no other way to detect presence vs default.

@SharkFourSix
Copy link

@deankarn

Well, at the same time, it doesn't make sense to treat false as bad input because it's just as important as any other value.

That right there is where the bad design is. Without even looking at semantics or language behavior.

Assignment of default values to uninitialized variables is not exclusive to go. So this is not a problem with the underlying language.

You want to leave that behavior to the language and let it do what it does (throw a null pointer exception or panic or whatever or assign whatever initialization value the compiler may decide) because then and only then will the programmer realize that they did not provide a value to that value, instead of the library hijacking that behavior because it decided to interpret certain valid values as if the memory/variable containing that said value doesn't exist, which again, is bad design.

Also, it's not like the library is allocating the memory for me to worry about initialization. That's my business, not the library's.

radnov added a commit to dhis2-sre/im-manager that referenced this issue Jul 19, 2023
If the Deployable bool field is `false`, the validation from the "required" tag will fail,
as it's considered the zero-value for bools. In our case both `true` and `false`
are valid, so there's no need to use the "required" tag.

go-playground/validator#713
radnov added a commit to dhis2-sre/im-manager that referenced this issue Jul 20, 2023
* fix: remove "required" binding from Deployable bool field

If the Deployable bool field is `false`, the validation from the "required" tag will fail,
as it's considered the zero-value for bools. In our case both `true` and `false`
are valid, so there's no need to use the "required" tag.

go-playground/validator#713

* fix: handle duplicated key error for group creation
@npropadovic
Copy link

@deankarn @SharkFourSix
I absolutely agree with SharkFourSix. While for strings this might make sense, it doesn't even make sense for integers, much less for booleans. I'm not arguing the design of Golang here, I'm arguing that to my brain the tag required implies just that the field needs to be in the json I'm verifying, not that it has to have a certain value. And requiring one to use a pointer when one does not need a pointer just in order to make sense of a tag is just puting the cart before the horse.

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