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

Response body validation does not work #54

Closed
openreply-dleinhaeuser opened this issue Nov 21, 2018 · 8 comments
Closed

Response body validation does not work #54

openreply-dleinhaeuser opened this issue Nov 21, 2018 · 8 comments

Comments

@openreply-dleinhaeuser
Copy link

openreply-dleinhaeuser commented Nov 21, 2018

exegesis-express does not validate the body of a JSON response even if response validation is enabled with the onResponseValidationError option.

Steps to reproduce

  • Install node v10.12.0 (current LTS) or newer.

  • Extract the minimal example: exegesis-example.zip

  • Run npm i && node .

  • Send a GET request against http://localhost:8080/

Expected result

The server responds with status code 500 and message Response validation failed

Actual result

The server responds with status code 200 and message { "message": {} }.

Further information

Stepping through Response.validateResponse reveals, that response validation is skipped silently (in itself arguably not correct if a response specification exists) if the body is a string, which it always seems to be, even if the controller returns an object or calls res.json.

@jwalton
Copy link
Contributor

jwalton commented Nov 21, 2018

I suspect this is an unexpected side effect of #53 @erykwarren. :(

@jwalton
Copy link
Contributor

jwalton commented Nov 21, 2018

(Also, thanks for the detailed error report!)

@jwalton
Copy link
Contributor

jwalton commented Nov 27, 2018

🎉 This issue has been resolved in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@erykwarren
Copy link
Contributor

@jwalton Sorry for having introduced this bug :( . For what it is worth, swagger-tool did also parse the JSON to do the validation.

@jwalton
Copy link
Contributor

jwalton commented Nov 28, 2018 via email

@openreply-dleinhaeuser
Copy link
Author

If the body is an object you could store it somewhere else before stringifying it and then validate that as an optimization.

@jwalton
Copy link
Contributor

jwalton commented Dec 4, 2018

It's not quite that simple unfortunately. The thing that makes it complicated is that you can write a toJSON function on an object that will control how it is serialized:

const foo = {toJSON() {return "bar";}};
JSON.stringify({foo}); // returns '{"foo":"bar"}'

We're using ajv to do schema validation, and unfortunately it doesn't call into toJSON for us. So if you try:

const Ajv = require('ajv');
const ajv = new Ajv();
const validate = ajv.compile({
    type: 'object',
    properties: {
        myString: {type: "string"}
    }
});
validate({myString: 'bar'}); // passes
validate({myString: foo}); // fails

And, at first glance, you might think "Why would anyone care about this weird esoteric feature of JSON stringifiers?" But, if you use MongoDB (as a lot of node.js folks do), ObjectId is an object, with a toJSON() that converts it into a string.

So if there's an object with a toJSON(), we actually need to convert your response to JSON (or at least that object) and then parse it back before we can validate it. So we need to know ahead of time that you're not using any objects with toJSON(), or we need to traverse the response tree and check to see if you have any objects with toJSON(), or we need to give up and just do this for every response. :P

@openreply-dleinhaeuser
Copy link
Author

You could do something like this (using lodash for brevity):

_.cloneDeepWith(body, (value, key) => {
  if ('toJSON' in value && _.isFunction(value.toJSON)) {
    return value.toJSON(key);
  }
  return undefined;
}

Question is of course, if this will be faster than serializing/deserializing and up to what size.
Maybe just not worth the hassle.

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

3 participants