(v3.x) fix: handle invalid url (#3806)

* fix: handle invalid url (#3128)

* fix: handle invalid url

* refactor: handle as 404 instead of 400

* fix: respect disableRequestLogging

* chore: linting

* feat: allow to customize 404 onBadUrl handler

lib/fourOhFour.js

@@ -37,14 +37,17 @@ function fourOhFour (options) {
   const { logger, genReqId } = options
 
   // 404 router, used for handling encapsulated 404 handlers
-  const router = FindMyWay({ defaultRoute: fourOhFourFallBack })
+  const router = FindMyWay({ onBadUrl: createOnBadUrl(), defaultRoute: fourOhFourFallBack })
+  let _onBadUrlHandler = null
 
   return { router, setNotFoundHandler, setContext, arrange404 }
 
   function arrange404 (instance) {
     // Change the pointer of the fastify instance to itself, so register + prefix can add new 404 handler
     instance[kFourOhFourLevelInstance] = instance
     instance[kCanSetNotFoundHandler] = true
+    // we need to bind instance for the context
+    router.onBadUrl = router.onBadUrl.bind(instance)
   }
 
   function basic404 (request, reply) {
@@ -58,6 +61,18 @@ function fourOhFour (options) {
     })
   }
 
+  function createOnBadUrl () {
+    return function onBadUrl (path, req, res) {
+      const id = genReqId(req)
+      const childLogger = logger.child({ reqId: id })
+      const fourOhFourContext = this[kFourOhFourLevelInstance][kFourOhFourContext]
+      const request = new Request(id, null, req, null, childLogger, fourOhFourContext)
+      const reply = new Reply(res, request, childLogger)
+
+      _onBadUrlHandler(request, reply)
+    }
+  }
+
   function setContext (instance, context) {
     const _404Context = Object.assign({}, instance[kFourOhFourContext])
     _404Context.onSend = context.onSend
@@ -107,8 +122,12 @@ function fourOhFour (options) {
     if (handler) {
       this[kFourOhFourLevelInstance][kCanSetNotFoundHandler] = false
       handler = handler.bind(this)
+      // update onBadUrl handler
+      _onBadUrlHandler = handler
     } else {
       handler = basic404
+      // update onBadUrl handler
+      _onBadUrlHandler = basic404
     }
 
     this.after((notHandledErr, done) => {

test/404s.test.js

@@ -1738,6 +1738,58 @@ test('400 in case of bad url (pre find-my-way v2.2.0 was a 404)', t => {
     })
   })
 
+  t.test('No route registered', t => {
+    t.plan(3)
+    const fastify = Fastify()
+    fastify.inject({
+      url: '/%c0',
+      method: 'GET'
+    }, (err, response) => {
+      t.error(err)
+      t.equal(response.statusCode, 404)
+      t.same(JSON.parse(response.payload), {
+        error: 'Not Found',
+        message: 'Route GET:/%c0 not found',
+        statusCode: 404
+      })
+    })
+  })
+
+  t.test('Only / is registered', t => {
+    t.plan(3)
+    const fastify = Fastify({ logger: true })
+    fastify.get('/', () => t.fail('we should not be here'))
+    fastify.inject({
+      url: '/%c0',
+      method: 'GET'
+    }, (err, response) => {
+      t.error(err)
+      t.equal(response.statusCode, 404)
+      t.same(JSON.parse(response.payload), {
+        error: 'Not Found',
+        message: 'Route GET:/%c0 not found',
+        statusCode: 404
+      })
+    })
+  })
+
+  t.test('customized 404', t => {
+    t.plan(3)
+    const fastify = Fastify({ logger: true })
+    fastify.get('/', () => t.fail('we should not be here'))
+    fastify.setNotFoundHandler(function (req, reply) {
+      reply.code(404).send('this was not found')
+    })
+    fastify.inject({
+      url: '/%c0',
+      method: 'GET'
+    }, (err, response) => {
+      t.error(err)
+      t.equal(response.statusCode, 404)
+      t.same(response.payload, 'this was not found')
+    })
+  })
+
   t.end()
 })
 

test/logger.test.js

@@ -1481,6 +1481,26 @@ test('should not log incoming request and outgoing response when disabled', t =>
   })
 })
 
+test('should not log incoming request and outgoing response for 404 onBadUrl when disabled', t => {
+  t.plan(1)
+  const lines = []
+  const dest = new stream.Writable({
+    write: function (chunk, enc, cb) {
+      lines.push(JSON.parse(chunk))
+      cb()
+    }
+  })
+  const fastify = Fastify({ disableRequestLogging: true, logger: { level: 'info', stream: dest } })
+
+  fastify.inject({
+    url: '/%c0',
+    method: 'GET'
+  }, (e, res) => {
+    // it will log 1 line only because of basic404
+    t.same(lines.length, 1)
+  })
+})
+
 test('should pass when using unWritable props in the logger option', t => {
   t.plan(1)
   Fastify({