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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema builder is mutating schemas #1767

Closed
jsumners opened this issue Jul 25, 2019 · 4 comments
Closed

Schema builder is mutating schemas #1767

jsumners opened this issue Jul 25, 2019 · 4 comments
Labels
bug Confirmed bug

Comments

@jsumners
Copy link
Member

jsumners commented Jul 25, 2019

馃悰 Bug Report

After a route is added a function afterRouteAdded is invoked which in turn invokes validation.build => schemas.getSchemaAnyway (via schema.resolveRefs) to build out schemas. This getSchemaAnyway function is injecting new properties into the schema object before passing the schema to the defined schema compiler. If said schema compiler does not recognize these newly added properties then it can fail to return a compiled schema that will validate as expected.

fastify/lib/schemas.js

Lines 121 to 126 in 9b233e8

if (!schema.type || !schema.properties) {
return {
type: 'object',
properties: schema
}
}

To Reproduce

Let's say you use Joi schemas and that your have a custom Joi schema compiler that recognizes some internal extensions to Joi. If you define a route like:

const route = {
  path: '/foo/:an_id',
  schema: {
    params: {
      an_id: joi.number()
    }
  },
  handler(req, res) {
    assert.deepStrictEqual(req.params, {an_id: 42})
    res.send({hello: 'world'})
  }
}

Then a request to /foo/42 will fail the assertion because the custom compiler will return a schema that does not recognize any properties in the param schema due to the extra data being inserted by getSchemaAnyway.

Expected behavior

getSchemaAnyway should not mutate schema definitions it does not recognize.

Your Environment

  • node version: 10
  • fastify version: >=2.6.0
  • os: Mac
@jsumners
Copy link
Member Author

Replication:

'use strict';

const joi = require('@hapi/joi');
const fastify = require('fastify')();

function schemaCompiler(schema) {
  return (data, opts) => joi.validate(data, schema);
}

fastify.setSchemaCompiler(schemaCompiler);

fastify.route({
  path: '/foo/:an_id',
  method: 'GET',
  schema: {
    params: {
      an_id: joi.number()
    }
  },
  handler(req, res) {
    res.send(req.params);
  }
});

fastify.inject({ url: '/foo/42' }, (err, result) => {
  console.log(result.payload);
});

@jsumners
Copy link
Member Author

Note that this reproduction script works under Fastify v1.14.6 without any changes.

@jsumners
Copy link
Member Author

Commit 4c4f775 is the one that introduces the bug.

@jsumners jsumners added the bug Confirmed bug label Jul 25, 2019
@jsumners
Copy link
Member Author

Fixed in #1768

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

No branches or pull requests

1 participant