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

Enforce Validation on any request type #12

Open
baderkha opened this issue Oct 11, 2020 · 1 comment
Open

Enforce Validation on any request type #12

baderkha opened this issue Oct 11, 2020 · 1 comment

Comments

@baderkha
Copy link

baderkha commented Oct 11, 2020

Hey there ,
Thank you for providing this middleware , it's a blessing since i don't have to have to manually do it now. I forked it and modified abit for a project i'm working on .

I would only recommend that for the validation to not rely on the front end headers for the content type. as alot of times those aren't provided and as a result your validation logic will not work.

in this code block , theoretically we should just enforce validation on all 3 types of request types and headers types.

if ctLen > 1 && ctHdr == "application/json" {
			err := mw.HandleJson(c)
			if err != nil {
				return err
			}
		} else if ctHdr == "application/x-www-form-urlencoded" {
			err := mw.HandleXFormEncoded(c)
			if err != nil {
				return err
			}
		} else if strings.Contains(ctHdr, "multipart/form-data") {
			err := mw.HandleMultiPartFormData(c, ctHdr)
			if err != nil {
				return err
			}
		}

or an alternative solution if you think that makes it not elegant , is to enforce it route level , where we can specify which request typeValidator to use ie would look like this kind of.

var xssMdlwr middleware.XssMw
router.POST("",xssMdlwr.HeaderType('application/json'),controller.SomePostFunc)

this way we don't have to rely on the client passed header type.

@dvwright
Copy link
Owner

Hi @baderkha thanks for the suggestion. I haven't worked on this code for some time, so will have to get my head around it again when I get a chance, go with what seems right to you. Feel free to submit a PR and we can get this merged in, try to include a test if possible. Thanks (Sorry for the late reply.)

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