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

[QUESTION] Read* like ReadQuery ignores "required" tags and don't kick out an error #1727

Open
Dexus opened this issue Feb 14, 2021 · 11 comments

Comments

@Dexus
Copy link

Dexus commented Feb 14, 2021

Describe the bug
Schema supportet Reader dont respect "required" tag

To Reproduce
https://github.com/kataras/iris/blob/master/_examples/request-body/read-query/main.go < don't send a query with name in it -> no error

Expected behavior
Requests without the required fileds, should return an error.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Ubuntu 18.04

iris.Version

  • master

Additional context
Maybe the https://github.com/iris-contrib/schema need an update and/or fixes?

@Dexus Dexus changed the title [BUG] [BUG] Read* like ReadQuery ignores "required" tags and don't kick out an error Feb 14, 2021
@Dexus
Copy link
Author

Dexus commented Feb 14, 2021

Okay i found out that if you don't provide any querystrings you don't get a error. But if you only provide only one you get the errors.

I think this needs to be fixed.

type MyQuery struct {
	Session  string `url:"s,required"`
	Page     string `url:"p,required"`
	SubPage  string `url:"sp"`
	Action   string `url:"a"`
}

https://mydomain.org/pagewithqueryneed -> no error
https://mydomain.org/pagewithqueryneed?notneed -> error => s is empty (and 1 other error)
https://mydomain.org/pagewithqueryneed?p -> error = s is empty

@kataras
Copy link
Owner

kataras commented Feb 14, 2021

To Reproduce
https://github.com/kataras/iris/blob/master/_examples/request-body/read-query/main.go < don't send a query with name in it -> no error

@Dexus this is working on me.

image

image

And your example also works:

(p is missing):
image

(s is missing):
image

(both are missing):

image

Maybe you have the iris.IsErrPath(err) ignored?

@kataras kataras removed their assignment Feb 14, 2021
@kataras kataras changed the title [BUG] Read* like ReadQuery ignores "required" tags and don't kick out an error [QUESTION] Read* like ReadQuery ignores "required" tags and don't kick out an error Feb 14, 2021
@Dexus
Copy link
Author

Dexus commented Feb 14, 2021 via email

@kataras
Copy link
Owner

kataras commented Feb 14, 2021

Of course there is no error without query string. You have to first check if the URL Query is empty. This is intentional:

func (ctx *Context) ReadQuery(ptr interface{}) error {
	values := ctx.getQuery()
	if len(values) == 0 {
		return nil // HERE
	}
	
	// ...

But if you want we can respect the FireEmptyFormError configuration for queries as we do on ReadForm:

func (ctx *Context) ReadForm(formObject interface{}) error {
	values := ctx.FormValues()
	if len(values) == 0 {
		if ctx.app.ConfigurationReadOnly().GetFireEmptyFormError() {
			return ErrEmptyForm
		}
		return nil
	}
	
	// ....

@Dexus
Copy link
Author

Dexus commented Feb 14, 2021

That would be a greate idea!

@kataras
Copy link
Owner

kataras commented Feb 14, 2021

OK I am on it! Can you also please reply to you other issues labeld as "BUG" too? (e.g. #1723) thanks!

@kataras
Copy link
Owner

kataras commented Feb 14, 2021

OK It's done, upgrade on latest @master. The example was also updated to have comments about this feature.

@Dexus
Copy link
Author

Dexus commented Feb 14, 2021

@kataras did update the other issue

@Dexus
Copy link
Author

Dexus commented Feb 14, 2021

@kataras

it would be cool if we can alias schema.MultiError to catch the error of this type.

Because schema will collect the errors into this type.

@kataras
Copy link
Owner

kataras commented Feb 14, 2021

@Dexus Hmm what do you mean? If the data are empty, there is no other "error" that can happen (so MultiError has no reason to contain that error), decoder won't be fired at all to check the fields (the source is empty). Can you give me a usecase example so I provide a better option for you?

@Dexus
Copy link
Author

Dexus commented Feb 15, 2021

Hi @kataras

if I have only one required field filled, i get an error of MultiError (e#10003):

var (
	errMultiError = &schema.MultiError{}
	errEmptyField = &schema.EmptyFieldError{}
)

// CheckErr ...
func CheckErr(ctx iris.Context, err error) bool {

	if err != nil && err == iris.ErrEmptyForm {
		ctx.StopWithError(iris.StatusBadRequest, errors.New("error (e#10001)"))
		ctx.StopExecution()
		return true
	} else if err != nil && iris.IsErrPath(err) {
		ctx.StopWithError(iris.StatusBadRequest, errors.New("error (e#10002)"))
		ctx.StopExecution()
		return true
	} else if err != nil && errors.Is(err, iris.ErrEmptyFormField) {
		ctx.StopWithError(iris.StatusBadRequest, errors.New("error (e#10005)"))
		ctx.StopExecution()
		return true
	} else if err != nil && errors.As(err, errMultiError) {
		ctx.StopWithError(iris.StatusBadRequest, errors.New("error (e#10003)"))
		ctx.StopExecution()
		return true
	} else if err != nil && errors.As(err, errEmptyField) {
		ctx.StopWithError(iris.StatusBadRequest, errors.New("error (e#10004)"))
		ctx.StopExecution()
		return true
	} else if err != nil {
		ctx.StopWithError(iris.StatusInternalServerError, err)
		ctx.StopExecution()
		return true
	}
	return false
}

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

2 participants