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

fix: custom validator should not mutate headers schema #4295

Merged
merged 3 commits into from Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/route.js
Expand Up @@ -327,7 +327,8 @@ function buildRouting (options) {
schemaController.setupValidator(this[kOptions])
}
try {
compileSchemasForValidation(context, opts.validatorCompiler || schemaController.validatorCompiler)
const isCustom = typeof opts?.validatorCompiler === 'function' || schemaController.isCustomValidatorCompiler
compileSchemasForValidation(context, opts.validatorCompiler || schemaController.validatorCompiler, isCustom)
Copy link
Member

Choose a reason for hiding this comment

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

Should we do the same for compileSchemasForSerialization for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have thought about it before. But serialization didn't mutate the schema and I would leave it for now.

The isCustomSerializerCompiler is added only because it maybe useful in future.

} catch (error) {
throw new FST_ERR_SCH_VALIDATION_BUILD(opts.method, url, error.message)
}
Expand Down
8 changes: 7 additions & 1 deletion lib/schema-controller.js
Expand Up @@ -25,7 +25,9 @@ function buildSchemaController (parentSchemaCtrl, opts) {

const option = {
bucket: (opts && opts.bucket) || buildSchemas,
compilersFactory
compilersFactory,
isCustomValidatorCompiler: typeof opts?.compilersFactory?.buildValidator === 'function',
isCustomSerializerCompiler: typeof opts?.compilersFactory?.buildValidator === 'function'
}

return new SchemaController(undefined, option)
Expand All @@ -37,6 +39,8 @@ class SchemaController {
this.addedSchemas = false

this.compilersFactory = this.opts.compilersFactory
this.isCustomValidatorCompiler = this.opts.isCustomValidatorCompiler || false
this.isCustomSerializerCompiler = this.opts.isCustomSerializerCompiler || false
climba03003 marked this conversation as resolved.
Show resolved Hide resolved

if (parent) {
this.schemaBucket = this.opts.bucket(parent.getSchemas())
Expand Down Expand Up @@ -65,10 +69,12 @@ class SchemaController {
// Schema Controller compilers holder
setValidatorCompiler (validatorCompiler) {
this.validatorCompiler = validatorCompiler
this.isCustomValidatorCompiler = true
}

setSerializerCompiler (serializerCompiler) {
this.serializerCompiler = serializerCompiler
this.isCustomSerializerCompiler = true
}

getValidatorCompiler () {
Expand Down
7 changes: 4 additions & 3 deletions lib/validation.js
Expand Up @@ -32,7 +32,7 @@ function compileSchemasForSerialization (context, compile) {
}, {})
}

function compileSchemasForValidation (context, compile) {
function compileSchemasForValidation (context, compile, isCustom) {
const { schema } = context
if (!schema) {
return
Expand All @@ -41,8 +41,9 @@ function compileSchemasForValidation (context, compile) {
const { method, url } = context.config || {}

const headers = schema.headers
if (headers && Object.getPrototypeOf(headers) !== Object.prototype) {
// do not mess with non-literals, e.g. Joi schemas
// the or part is used for backward compatibility
if (headers && (isCustom || Object.getPrototypeOf(headers) !== Object.prototype)) {
// do not mess with schema when custom validator applied, e.g. Joi, Typebox
context[headersSchema] = compile({ schema: headers, method, url, httpPart: 'headers' })
} else if (headers) {
// The header keys are case insensitive
Expand Down
15 changes: 15 additions & 0 deletions test/internals/validation.test.js
Expand Up @@ -268,6 +268,21 @@ test('build schema - headers are not lowercased in case of custom object', t =>
})
})

test('build schema - headers are not lowercased in case of custom validator provided', t => {
t.plan(1)

class Headers {}
const opts = {
schema: {
headers: new Headers()
}
}
validation.compileSchemasForValidation(opts, ({ schema, method, url, httpPart }) => {
t.type(schema, Headers)
return () => {}
}, true)
})

test('build schema - uppercased headers are not included', t => {
t.plan(1)
const opts = {
Expand Down
25 changes: 25 additions & 0 deletions test/schema-feature.test.js
Expand Up @@ -1779,3 +1779,28 @@ test('Should return a human-friendly error if response status codes are not spec
t.match(err.message, 'Failed building the serialization schema for GET: /, due to error response schemas should be nested under a valid status code, e.g { 2xx: { type: "object" } }')
})
})

test('setSchemaController: custom validator instance should not mutate headers schema', async t => {
t.plan(2)
class Headers {}
const fastify = Fastify()

fastify.setSchemaController({
compilersFactory: {
buildValidator: function () {
return ({ schema, method, url, httpPart }) => {
t.type(schema, Headers)
return () => {}
}
}
}
})

fastify.get('/', {
schema: {
headers: new Headers()
}
}, () => {})

await fastify.ready()
})
19 changes: 19 additions & 0 deletions test/schema-validation.test.js
Expand Up @@ -1017,3 +1017,22 @@ test("The same $id in route's schema must not overwrite others", t => {
t.same(res.payload, 'ok')
})
})

test('Custom validator compiler should not mutate schema', async t => {
t.plan(2)
class Headers {}
const fastify = Fastify()

fastify.setValidatorCompiler(({ schema, method, url, httpPart }) => {
t.type(schema, Headers)
return () => {}
})

fastify.get('/', {
schema: {
headers: new Headers()
}
}, () => {})

await fastify.ready()
})