Add content type validation header

PayU / openapi-validator-middleware

Input validation using Swagger (Open API) and ajv

Apache License 2.0

144 stars 50 forks source link

Add content type validation header #27

Closed idanto closed 6 years ago

idanto commented 6 years ago

Added header for header validation. The only case that isn't covered is in case the body can be empty, in this case, the validation won't be smart and will fail if the header is missing.

Resolve #17

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 107

4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 99.355%

Totals
Change from base Build 103:	0.02%
Covered Lines:	188
Relevant Lines:	188

Pull Request Test Coverage Report for Build 107

4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 99.355%

Totals
Change from base Build 103:	0.02%
Covered Lines:	188
Relevant Lines:	188

Pull Request Test Coverage Report for Build 119

62 of 62 (100.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.08%) to 99.413%

Totals
Change from base Build 103:	0.08%
Covered Lines:	208
Relevant Lines:	208

💛 - Coveralls

idanto commented 6 years ago

@igor-savin-ht you can review, If you have any good idea to handle the last open issue feel free to share.

idanto commented 6 years ago

Basically, I think everything is ready for review, I solved all the issues by not setting the content-type header as required and using a regex for validation. @igor-savin-ht @kibertoad can you please review?

kibertoad commented 6 years ago

@idanto Would it be possible to skip validation in case body is empty? That sounds to be a more valid approach.

idanto commented 6 years ago

@kibertoad It's much harder to implement, cause the whole idea behind ajv is the pre-compiled schema and currently the body and all other parameters are two different schemas. I can check if there is a conditional check in Ajv and think how to indicate that there is something in the body but it will take me some time to solve.

I'm with you, If you have time to check it for me I will implement.

kibertoad commented 6 years ago

@idanto Probably the easiest solution at this point is to make this check optional and disable by default, so that no existing requests start failing, and whoever needs this check, could opt-in.

idanto commented 6 years ago

Please do not merge this one, I found an issue with the code - it is application/json specific and doesn't fully support the swagger spec. I would like also to make this optional as @kibertoad suggested

kibertoad commented 6 years ago

@idanto Can we move the validation outside of ajv call and check the header manually?

idanto commented 6 years ago

@kibertoad Please review again:

Added content-type validation as an options
Added validation only if content-length is higher than 0
Added Support in multiple content-types (Supported by Swagger Spec)
A small refactor to the code
Added more tests to more edge cases

kibertoad commented 6 years ago

@idanto Can I merge it?