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

Empty object coercion #178

Merged
merged 4 commits into from
May 6, 2020
Merged

Conversation

tpburch
Copy link
Contributor

@tpburch tpburch commented May 5, 2020

Addresses #166

Previously, two things were working together to create this result:

  1. The coercion function would return undefined for an empty object or array value for body parameters (parameter.schema exists).
  2. The result of running the coercion function ('undefined' in this case) was sent to schema.validate. Because this function is awaited, the actual value is returned, and is subsequently inspected for an error property. Since the value is undefined, this throws an unexpected error.

The fix applied is:

  1. Don't coerce empty objects or arrays into undefined. Just return the empty object/array.
  2. Don't await calls to validate. This is not as async function, so await is not needed. In this case, awaiting validate caused undefined to be returned rather than an object with value: undefined, which led to the unexpected error.
  3. Add a check to catch cases where the result of validate is falsy for some reason, although this should not happen due to the other changes.

After applying this fix, cases where an in: body parameter is specified but not provided return a 400 with an error message resolving correctly to the parameter name.

Copy link
Contributor

@tlivings tlivings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test to validate the issue has been fixed?

Copy link
Contributor

@tlivings tlivings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -97,11 +96,11 @@ const create = function (options = {}) {
request[p][parameter.name] = coerce && request[p][parameter.name] && coerce(request[p][parameter.name]);
return h.continue;
},
validate: async function (value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i imagine this was a left over from earlier version of async joi validate calls.

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

Successfully merging this pull request may close these issues.

2 participants