fix: no check on null or undefined values passed as fn (#4367)

fastify · Oct 23, 2022 · 3769cd0 · 3769cd0
1 parent b94441f
commit 3769cd0
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 7 deletions.
diff --git a/fastify.js b/fastify.js
@@ -566,17 +566,21 @@ function fastify (options) {
   function addHook (name, fn) {
     throwIfAlreadyStarted('Cannot call "addHook" when fastify instance is already started!')
 
+    if (fn == null) {
+      throw new errorCodes.FST_ERR_HOOK_INVALID_HANDLER(name, fn)
+    }
+
     if (name === 'onSend' || name === 'preSerialization' || name === 'onError' || name === 'preParsing') {
       if (fn.constructor.name === 'AsyncFunction' && fn.length === 4) {
-        throw new Error('Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+        throw new errorCodes.FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
       }
     } else if (name === 'onReady') {
       if (fn.constructor.name === 'AsyncFunction' && fn.length !== 0) {
-        throw new Error('Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+        throw new errorCodes.FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
       }
     } else {
       if (fn.constructor.name === 'AsyncFunction' && fn.length === 3) {
-        throw new Error('Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+        throw new errorCodes.FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
       }
     }
 

diff --git a/lib/errors.js b/lib/errors.js
@@ -101,6 +101,12 @@ const codes = {
     500,
     TypeError
   ),
+  FST_ERR_HOOK_INVALID_ASYNC_HANDLER: createError(
+    'FST_ERR_HOOK_INVALID_ASYNC_HANDLER',
+    'Async function has too many arguments. Async hooks should not use the \'done\' argument.',
+    500,
+    TypeError
+  ),
 
   /**
    * Middlewares

diff --git a/test/hooks-async.test.js b/test/hooks-async.test.js
@@ -585,33 +585,37 @@ test('preHandler respond with a stream', t => {
 
 test('Should log a warning if is an async function with `done`', t => {
   t.test('3 arguments', t => {
-    t.plan(1)
+    t.plan(2)
     const fastify = Fastify()
 
     try {
       fastify.addHook('onRequest', async (req, reply, done) => {})
     } catch (e) {
+      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
       t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
   })
 
   t.test('4 arguments', t => {
-    t.plan(3)
+    t.plan(6)
     const fastify = Fastify()
 
     try {
       fastify.addHook('onSend', async (req, reply, payload, done) => {})
     } catch (e) {
+      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
       t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
     try {
       fastify.addHook('preSerialization', async (req, reply, payload, done) => {})
     } catch (e) {
+      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
       t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
     try {
       fastify.addHook('onError', async (req, reply, payload, done) => {})
     } catch (e) {
+      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
       t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
   })

diff --git a/test/hooks.on-ready.test.js b/test/hooks.on-ready.test.js
@@ -291,12 +291,13 @@ t.test('onReady cannot add lifecycle hooks', t => {
 })
 
 t.test('onReady throw loading error', t => {
-  t.plan(1)
+  t.plan(2)
   const fastify = Fastify()
 
   try {
     fastify.addHook('onReady', async function (done) {})
   } catch (e) {
+    t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
     t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
   }
 })

diff --git a/test/hooks.test.js b/test/hooks.test.js
@@ -23,7 +23,7 @@ function getUrl (app) {
 }
 
 test('hooks', t => {
-  t.plan(43)
+  t.plan(49)
   const fastify = Fastify({ exposeHeadRoutes: false })
 
   try {
@@ -41,6 +41,22 @@ test('hooks', t => {
     t.fail()
   }
 
+  try {
+    fastify.addHook('preHandler', null)
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_HANDLER')
+    t.equal(e.message, 'preHandler hook should be a function, instead got null')
+    t.pass()
+  }
+
+  try {
+    fastify.addHook('preParsing')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_HANDLER')
+    t.equal(e.message, 'preParsing hook should be a function, instead got undefined')
+    t.pass()
+  }
+
   try {
     fastify.addHook('preParsing', function (request, reply, payload, done) {
       request.preParsing = true