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

Using named schema in items field breaks schema validation #1491

Closed
SkeLLLa opened this issue Feb 28, 2019 · 6 comments · Fixed by #1496
Closed

Using named schema in items field breaks schema validation #1491

SkeLLLa opened this issue Feb 28, 2019 · 6 comments · Fixed by #1496

Comments

@SkeLLLa
Copy link
Contributor

SkeLLLa commented Feb 28, 2019

🐛 Bug Report

Using some named schema as items in other schema cause exception when using first schema

To Reproduce

Steps to reproduce the behavior:

Add schema with simple item. Then add schema for items array and use first schema $id in items.
Then use the first schema in some route.

Paste your code here:

const fastify = require('fastify')();

(async () => {
  const name = 'item';
  const item = {type: 'object', properties: {foo: {type: 'string'}}};
  fastify.addSchema({
    $id: `${name}`,
    ...item,
  });
  fastify.addSchema({
    $id: `${name}List`,
    type: 'array',
    items: `${name}#`,
  });
  fastify.get(
    '/',
    {
      schema: {
        response: {
          200: `${name}#`,
        },
      },
    },
    async (req, res) => {
      return {};
    }
  );
  await fastify.ready();
  await fastify.listen(3000);
  console.log('app listens');
})();
(node:21665) UnhandledPromiseRejectionWarning: Error: schema is invalid: data.items should be object,boolean, data.items should be array, data.items should match some schema in anyOf
at Ajv.validateSchema (/tmp/xxx/node_modules/ajv/lib/ajv.js:177:16)
at Ajv._addSchema (/tmp/xxx/node_modules/ajv/lib/ajv.js:306:10)
at Ajv.addSchema (/tmp/xxx/node_modules/ajv/lib/ajv.js:136:29)
at Object.keys.forEach.key (/tmp/xxx/node_modules/fast-json-stringify/index.js:937:11)
at Array.forEach (<anonymous>)
at isValidSchema (/tmp/xxx/node_modules/fast-json-stringify/index.js:936:33)
at build (/tmp/xxx/node_modules/fast-json-stringify/index.js:39:3)
at getValidatorForStatusCodeSchema (/tmp/xxx/node_modules/fastify/lib/validation.js:13:10)
at /tmp/xxx/node_modules/fastify/lib/validation.js:19:21
at Array.reduce (<anonymous>)

Expected behavior

No exception thrown.

Your Environment

  • node version: 11
  • fastify version: 2.0.0
  • os: Mac, Linux
@mcollina
Copy link
Member

@SkeLLLa thanks for reporting! Would you like to send a PR to fix it?

Also @Eomm might now more.

@SkeLLLa
Copy link
Contributor Author

SkeLLLa commented Feb 28, 2019

@mcollina I don't know exactly where exactly this should be. Probably even in fast-json-stringify, so may be it will be better if someone who is more familiar with both fastify and fast-json-stringify libs.

PS: Important update. Only fastify v2 affected. On 1.X.X it works fine.

@Eomm
Copy link
Member

Eomm commented Feb 28, 2019

I will check this evening 👍
The problem seems in the cross-shared-schema references

@Eomm
Copy link
Member

Eomm commented Feb 28, 2019

This bug occurs only if you don't use a shared schema.

If you add the shared schema in a body, for example, all works.

// workaround
  fastify.get('/', {
      schema: {
        body: `${name}List#`,
        response: {
          200: `${name}#`
        }
      }
    },

The "replace-way" shared schemas are objects that are modified: the key: <id># part is replaced with a JSON resulting key: {<JSON>}.
This update of the reference is triggered only when Fastify start the loading.

But, if you have a shared schema that references a not used shared schema, it will be not evaluated but it will be pass to serializer here:

context[responseSchema] = getResponseSchema(context.schema.response, schemas.getSchemas())

@SkeLLLa , for what I tested it seems that the bug is also in v1, could you share your test?

PR with the fix is coming 👍

@SkeLLLa
Copy link
Contributor Author

SkeLLLa commented Mar 1, 2019

@Eomm I've tested it with code from the first post. And it worked in v1.
And unfortunately workaround will not work well in my case, because I need POST request as well :).

@Eomm
Copy link
Member

Eomm commented Mar 6, 2019

Fix released in v2.0.1
Thank you for reporting ✌

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 a pull request may close this issue.

3 participants