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

Don't skip parameter validation when parameter array is empty. #68

Merged
merged 4 commits into from May 11, 2021

Conversation

wparad
Copy link
Contributor

@wparad wparad commented Mar 2, 2021

Parameter validation is ignored in the case when no parameters are specified. This is due to a bug here:

if (localParameters.length > 0 || options.contentTypeValidation) {

if (localParameters.length > 0 || options.contentTypeValidation) {
        requestSchema.parameters = buildParametersValidation(localParameters,
            dereferenced.paths[currentPath][currentMethod].consumes || dereferenced.paths[currentPath].consumes || dereferenced.consumes, options);
    }

Which incorrectly skips parameter building in case when there are no parameters and content type validation is not true. However parameters have nothing to do with content type, so this should always be done. Since content type validation is again checked there's no reason to do this check in this place:

ajvParametersSchema.properties.headers.content = createContentTypeHeaders(options.contentTypeValidation, contentTypes);
:

    ajvParametersSchema.properties.headers.content = createContentTypeHeaders(options.contentTypeValidation, contentTypes);

module.exports = function createContentTypeHeaders(validate, contentTypes) {
    if (!validate || !contentTypes) return;

    return {
        types: contentTypes
    };
};

@wparad
Copy link
Contributor Author

wparad commented May 2, 2021

@kobik, can we get a look at this?

@kobik
Copy link
Collaborator

kobik commented May 3, 2021

Sure, will have a look

@kibertoad
Copy link
Collaborator

@wparad Could you please add test?

@wparad
Copy link
Contributor Author

wparad commented May 11, 2021

@kibertoad

@wparad Could you please add test?

I would, but I honestly can't figure out how in this repository to do that. If you could suggest a block of code I could copy and paste, I would be happy to.

@kibertoad
Copy link
Collaborator

@wparad
Copy link
Contributor Author

wparad commented May 11, 2021

@kibertoad 👍 added!

@kobik kobik merged commit da58523 into PayU:master May 11, 2021
@github-actions
Copy link

🎉 This PR is included in version 2.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants