fix: use req.originalUrl (express, fastify) (#167)

Use `req.originalUrl`, if available, when converting `req` to ECS fields.
This is necessary because Express, and possibly Fastify, will mutate req.url
during routing in some cases.

Author: trentm

packages/ecs-helpers/CHANGELOG.md

@@ -1,5 +1,11 @@
 # @elastic/ecs-helpers Changelog
 
+## v2.1.1
+
+- fix: Use `req.originalUrl`, if available, when converting `req` to ECS fields.
+  This is necessary because [Express will mutate req.url during routing](https://expressjs.com/en/4x/api.html#req.originalUrl).
+  (https://github.com/elastic/ecs-logging-nodejs/issues/114)
+
 ## v2.1.0
 
 - Bump the `ecs.version` from "1.6.0" to "8.10.0".

packages/ecs-helpers/lib/http-formatters.js

@@ -45,7 +45,6 @@ function formatHttpRequest (ecs, req) {
 
   const {
     method,
-    url,
     headers,
     hostname,
     httpVersion,
@@ -57,6 +56,12 @@ function formatHttpRequest (ecs, req) {
   ecs.http.request = ecs.http.request || {}
   ecs.http.request.method = method
 
+  // Express can strip leading parts of `req.url` during routing, so we must use
+  // https://expressjs.com/en/4x/api.html#req.originalUrl if available.
+  // Fastify docs (https://fastify.dev/docs/latest/Reference/Request/) imply
+  // that it might mutate `req.url` during routing as well.
+  const url = req.originalUrl || req.url
+
   ecs.url = ecs.url || {}
   ecs.url.full = (socket && socket.encrypted ? 'https://' : 'http://') + headers.host + url
   const hasQuery = url.indexOf('?')
@@ -78,6 +83,7 @@ function formatHttpRequest (ecs, req) {
   if (hostname) {
     const [host, port] = hostname.split(':')
     ecs.url.domain = host
+    // istanbul ignore next
     if (port) {
       ecs.url.port = Number(port)
     }

packages/ecs-helpers/test/express.test.js

@@ -27,6 +27,26 @@ const {
   formatHttpResponse
 } = require('../')
 
+async function makeARequest (url, opts) {
+  return new Promise((resolve, reject) => {
+    const clientReq = http.request(url, opts, function (clientRes) {
+      const chunks = []
+      clientRes.on('data', function (chunk) {
+        chunks.push(chunk)
+      })
+      clientRes.on('end', function () {
+        resolve({
+          statusCode: clientRes.statusCode,
+          headers: clientRes.headers,
+          body: chunks.join('')
+        })
+      })
+    })
+    clientReq.on('error', reject)
+    clientReq.end()
+  })
+}
+
 test('express res/req serialization', t => {
   const app = express()
   let server
@@ -78,26 +98,74 @@ test('express res/req serialization', t => {
     t.equal(typeof (rec.client.port), 'number')
   })
 
-  app.listen(0, '127.0.0.1', function () {
-    server = this
-    const req = http.get(
-      `http://127.0.0.1:${server.address().port}/apath?aquery#ahash`,
-      {
-        headers: {
-          'user-agent': 'cool-agent',
-          connection: 'close',
-          'x-request-id': 'arequestid'
+  const aRouter = express.Router()
+  aRouter.get('/asubpath', (req, res) => {
+    res.end('hi')
+
+    const rec = {}
+    let rv = formatHttpRequest(rec, req)
+    t.ok(rv, 'formatHttpRequest processed req')
+    rv = formatHttpResponse(rec, res)
+    t.ok(rv, 'formatHttpResponse processed res')
+
+    const port = server.address().port
+    t.same(rec, {
+      http: {
+        version: '1.1',
+        request: {
+          method: 'GET',
+          headers: {
+            host: `127.0.0.1:${port}`,
+            connection: 'close'
+          }
+        },
+        response: {
+          status_code: 200,
+          headers: {
+            'x-powered-by': 'Express'
+          }
         }
       },
-      function (res) {
-        res.on('data', function () {})
-        res.on('end', function () {
-          server.close(function () {
-            t.end()
-          })
-        })
+      url: {
+        full: `http://127.0.0.1:${port}/arouter/asubpath`,
+        path: '/arouter/asubpath',
+        domain: '127.0.0.1'
+      },
+      client: {
+        address: '127.0.0.1',
+        ip: '127.0.0.1',
+        port: req.socket.remotePort
       }
-    )
-    req.on('error', t.error)
+    })
+  })
+  app.use('/arouter', aRouter)
+
+  app.listen(0, '127.0.0.1', async function () {
+    server = this
+
+    await Promise.all([
+      makeARequest(
+        `http://127.0.0.1:${server.address().port}/apath?aquery#ahash`,
+        {
+          headers: {
+            'user-agent': 'cool-agent',
+            connection: 'close',
+            'x-request-id': 'arequestid'
+          }
+        }
+      ),
+      makeARequest(
+        `http://127.0.0.1:${server.address().port}/arouter/asubpath`,
+        {
+          headers: {
+            connection: 'close'
+          }
+        }
+      )
+    ])
+
+    server.close(() => {
+      t.end()
+    })
   })
 })