From 2b9d5f54409f293ce5052b6229a1ab9776949bf4 Mon Sep 17 00:00:00 2001
From: rektide <rektide@voodoowarez.com>
Date: Wed, 23 Feb 2022 12:03:51 -0500
Subject: [PATCH] Fix restify (#88) by only using req.path if a string (#89)

* call req.path() if a function. restify compat, fixes #88

in #29 we preferred req.path, because req.url would throw in hapi if the path was invalid. in restify though, req.path is a function, and this change made the url no longer log. check req.path to see if it is a function, and invoke it if it is.

* add test for calling req.path() if it is a function.

* only get url from req.path if it's a string. fixes #88.

restify has a req.path but it's a function that returns an object. rather than try to determine how to handle path if we have it, only use path if it's a simple string. otherwise, keep using req.url, like we used to pre #29.

* guard against req.url being falsy

it's tempting to simply stop looking at req.url.path altogether, since that was introduced for hapi 17 in https://github.com/pinojs/pino-std-serializers/commit/83d58a7fbf9cdbeb65bf3c67d269e8c47a02947e#diff-2c4782cb20f7f0d7ca90c7bc04939f4420c4bdb42fdce618984da393ba9202f7R60 .

but since #29 we look at req.path first, which is safer in hapi, so hapi doesn't care about req.url.path anymore.

but perhaps someone else does depend on req.url.path . so for now, continuing to look at req.url.path , which means a bit more guarding in the req serializer.
---
 lib/req.js       |  5 +++--
 test/req.test.js | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/lib/req.js b/lib/req.js
index 85264d0..ff6fb46 100644
--- a/lib/req.js
+++ b/lib/req.js
@@ -74,8 +74,9 @@ function reqSerializer (req) {
     _req.query = req.query
     _req.params = req.params
   } else {
-    // req.url.path is  for hapi compat.
-    _req.url = req.path || (req.url ? (req.url.path || req.url) : undefined)
+    const path = req.path
+    // path for safe hapi compat.
+    _req.url = typeof path === 'string' ? path : (req.url ? req.url.path || req.url : undefined)
   }
   _req.headers = req.headers
   _req.remoteAddress = connection && connection.remoteAddress
diff --git a/test/req.test.js b/test/req.test.js
index 4ac9a58..6726e2d 100644
--- a/test/req.test.js
+++ b/test/req.test.js
@@ -252,6 +252,47 @@ test('req.url will be obtained from input request originalUrl when available', f
   }
 })
 
+test('req.url will be obtained from input request url when req path is a function', function (t) {
+  t.plan(1)
+
+  const server = http.createServer(handler)
+  server.unref()
+  server.listen(0, () => {
+    http.get(server.address(), () => {})
+  })
+
+  t.teardown(() => server.close())
+
+  function handler (req, res) {
+    req.path = function () {
+      throw new Error('unexpected invocation')
+    }
+    req.url = '/test'
+    const serialized = serializers.reqSerializer(req)
+    t.equal(serialized.url, '/test')
+    res.end()
+  }
+})
+
+test('req.url being undefined does not throw an error', function (t) {
+  t.plan(1)
+
+  const server = http.createServer(handler)
+  server.unref()
+  server.listen(0, () => {
+    http.get(server.address(), () => {})
+  })
+
+  t.teardown(() => server.close())
+
+  function handler (req, res) {
+    req.url = undefined
+    const serialized = serializers.reqSerializer(req)
+    t.equal(serialized.url, undefined)
+    res.end()
+  }
+})
+
 test('can wrap request serializers', function (t) {
   t.plan(3)