From d11c383eb743a0aae7b1026eb60c6dc9b5f715b8 Mon Sep 17 00:00:00 2001
From: Kevin Hira <kevin@hira.nz>
Date: Wed, 9 Mar 2022 02:05:31 +1300
Subject: [PATCH] Check if properRoute is found before acting on it (#73)

* Check if properRoute is found before acting on it

In the cases where the request route is not matched to any Koa route,
bail out before trying to format the path string.

Issue: #59

* Add test for koa middleware unmatched route scenario

Issue: #59

* Add integration test for wildcard path scenario

Issue: #59
---
 src/koa-middleware.js                         |  8 ++-
 test/integration-tests/koa/middleware-test.js | 17 +++++++
 .../koa/server/koa-server.js                  |  6 +++
 test/unit-test/metric-middleware-koa-test.js  | 51 +++++++++++++++++++
 4 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/src/koa-middleware.js b/src/koa-middleware.js
index 3b5dffc..5036637 100644
--- a/src/koa-middleware.js
+++ b/src/koa-middleware.js
@@ -33,7 +33,7 @@ class KoaMiddleware {
         }
     }
 
-    _handleResponse (ctx) {
+    _handleResponse(ctx) {
         const responseLength = parseInt(ctx.response.get('Content-Length')) || 0;
 
         const route = this._getRoute(ctx) || 'N/A';
@@ -93,6 +93,12 @@ class KoaMiddleware {
             }
         });
 
+        // If proper route is not found, send an undefined route
+        // The caller is responsible for setting a default "N/A" route in this case
+        if (!properRoute) {
+            return undefined;
+        }
+
         let route = properRoute.path;
         route = route.endsWith('/') ? route.substring(0, route.length - 1) : route;
         return route;
diff --git a/test/integration-tests/koa/middleware-test.js b/test/integration-tests/koa/middleware-test.js
index 07f22ca..978fa0f 100644
--- a/test/integration-tests/koa/middleware-test.js
+++ b/test/integration-tests/koa/middleware-test.js
@@ -679,6 +679,23 @@ describe('when using koa framework', () => {
                     });
             });
         });
+        describe('when calling wildcard endpoint', function () {
+            before(() => {
+                const wildcardPath = '/wild-path/' + Math.floor(Math.random() * 10);
+                return supertest(app)
+                    .get(wildcardPath)
+                    .expect(200)
+                    .then((res) => {});
+            });
+            it('should add it to the histogram', () => {
+                return supertest(app)
+                    .get('/metrics')
+                    .expect(200)
+                    .then((res) => {
+                        expect(res.text).to.contain('method="GET",route="N/A",code="200"');
+                    });
+            });
+        });
         it('should get metrics as json', () => {
             return supertest(app)
                 .get('/metrics.json')
diff --git a/test/integration-tests/koa/server/koa-server.js b/test/integration-tests/koa/server/koa-server.js
index fd5a04b..fa28c39 100644
--- a/test/integration-tests/koa/server/koa-server.js
+++ b/test/integration-tests/koa/server/koa-server.js
@@ -78,4 +78,10 @@ router.post('/test', async (ctx, next) => {
     return next();
 });
 
+router.get('/wild-path/(.*)', (ctx, next) => {
+    ctx.status = 200;
+    ctx.body = { message: 'Wildcard route reached!' };
+    return next();
+});
+
 module.exports = app;
diff --git a/test/unit-test/metric-middleware-koa-test.js b/test/unit-test/metric-middleware-koa-test.js
index ba7c959..5b284ec 100644
--- a/test/unit-test/metric-middleware-koa-test.js
+++ b/test/unit-test/metric-middleware-koa-test.js
@@ -653,6 +653,57 @@ describe('metrics-middleware', () => {
                 Prometheus.register.clear();
             });
         });
+        describe('when using middleware request and path not matched to any specific route', () => {
+            let match, func, req, res, ctx, next, requestSizeObserve, requestTimeObserve, endTimerStub;
+            before(async () => {
+                match = sinon.stub();
+                match.onFirstCall().returns({ path: [] });
+                match.onSecondCall().returns({ path: [{ path: '/(.*)' }] });
+                next = sinon.stub();
+                req = httpMocks.createRequest({
+                    url: '/not-path',
+                    method: 'GET',
+                    body: {
+                        foo: 'bar'
+                    },
+                    headers: {
+                        'content-length': '25'
+                    }
+                });
+                req.socket = {};
+                res = httpMocks.createResponse({
+                    eventEmitter: EventEmitter
+                });
+                res.statusCode = 404;
+                ctx = { req: req, res: res, request: req, response: res, router: { match: match }, _matchedRoute: '/(.*)', originalUrl: '/not-path' };
+                func = await middleware();
+                endTimerStub = sinon.stub();
+                requestTimeObserve = sinon.stub(Prometheus.register.getSingleMetric('http_request_duration_seconds'), 'startTimer').returns(endTimerStub);
+                func(ctx, next);
+                requestSizeObserve = sinon.spy(Prometheus.register.getSingleMetric('http_request_size_bytes'), 'observe');
+                res.emit('finish');
+            });
+            it('should update the histogram with the elapsed time and size', () => {
+                sinon.assert.calledWithExactly(requestSizeObserve, {
+                    method: 'GET',
+                    route: 'N/A',
+                    code: 404
+                }, 25);
+                sinon.assert.called(requestTimeObserve);
+                sinon.assert.calledWith(endTimerStub, {
+                    method: 'GET',
+                    route: 'N/A',
+                    code: 404
+                });
+                sinon.assert.calledOnce(requestTimeObserve);
+                sinon.assert.calledOnce(endTimerStub);
+            });
+            after(() => {
+                requestSizeObserve.restore();
+                requestTimeObserve.restore();
+                Prometheus.register.clear();
+            });
+        });
         describe('when _getConnections called', () => {
             let Middleware, server, numberOfConnectionsGauge, koaMiddleware, prometheusStub;
             before(() => {