Merge pull request #1011 from ch-lepp/1010

new method for logger to handle circular objects before logging
OWASP · Jul 10, 2024 · 62ec335 · 62ec335
2 parents a0dca4a + 825eb46
commit 62ec335
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 13 deletions.
diff --git a/td.server/src/controllers/auth.js b/td.server/src/controllers/auth.js
@@ -9,7 +9,7 @@ import tokenRepo from '../repositories/token.js';
 const logger = loggerHelper.get('controllers/auth.js');
 
 const login = (req, res) => {
-    logger.debug(`API login request: ${JSON.stringify(req)}`);
+    logger.debug(`API login request: ${logger.transformToString(req)}`);
 
     try {
         const provider = providers.get(req.params.provider);
@@ -20,7 +20,7 @@ const login = (req, res) => {
 };
 
 const oauthReturn = (req, res) => {
-    logger.debug(`API oauthReturn request: ${JSON.stringify(req)}`);
+    logger.debug(`API oauthReturn request: ${logger.transformToString(req)}`);
 
     let returnUrl = `/#/oauth-return?code=${req.query.code}`;
     if (env.get().config.NODE_ENV === 'development') {
@@ -30,7 +30,7 @@ const oauthReturn = (req, res) => {
 };
 
 const completeLogin = (req, res) => {
-    logger.debug(`API completeLogin request: ${JSON.stringify(req)}`);
+    logger.debug(`API completeLogin request: ${logger.transformToString(req)}`);
 
     try {
         const provider = providers.get(req.params.provider);
@@ -51,7 +51,7 @@ const completeLogin = (req, res) => {
 };
 
 const logout = (req, res) => responseWrapper.sendResponse(() => {
-    logger.debug(`API logout request: ${JSON.stringify(req)}`);
+    logger.debug(`API logout request: ${logger.transformToString(req)}`);
 
     try {
         const refreshToken = req.body.refreshToken;
@@ -71,7 +71,7 @@ const logout = (req, res) => responseWrapper.sendResponse(() => {
 }, req, res, logger);
 
 const refresh = (req, res) => {
-    logger.debug(`API refresh request: ${JSON.stringify(req)}`);
+    logger.debug(`API refresh request: ${logger.transformToString(req)}`);
 
     const tokenBody = tokenRepo.verify(req.body.refreshToken);
     if (!tokenBody) {

diff --git a/td.server/src/controllers/threatmodelcontroller.js b/td.server/src/controllers/threatmodelcontroller.js
@@ -24,7 +24,7 @@ const repos = (req, res) => responseWrapper.sendResponseAsync(async () => {
     }
     const headers = reposResp[1];
     const pageLinks = reposResp[2];
-    logger.debug(`API repos request: ${JSON.stringify(req)}`);
+    logger.debug(`API repos request: ${logger.transformToString(req)}`);
 
     const pagination = getPagination(headers, pageLinks, page);
 
@@ -45,7 +45,7 @@ const branches = (req, res) => responseWrapper.sendResponseAsync(async () => {
         repo: req.params.repo,
         page: req.query.page || 1
     };
-    logger.debug(`API branches request: ${JSON.stringify(req)}`);
+    logger.debug(`API branches request: ${logger.transformToString(req)}`);
 
     const branchesResp = await repository.branchesAsync(repoInfo, req.provider.access_token);
     const branches = branchesResp[0];
@@ -70,7 +70,7 @@ const models = (req, res) => responseWrapper.sendResponseAsync(async () => {
         repo: req.params.repo,
         branch: req.params.branch
     };
-    logger.debug(`API models request: ${JSON.stringify(req)}`);
+    logger.debug(`API models request: ${logger.transformToString(req)}`);
 
     let modelsResp;
     try {
@@ -93,7 +93,7 @@ const model = (req, res) => responseWrapper.sendResponseAsync(async () => {
         branch: req.params.branch,
         model: req.params.model
     };
-    logger.debug(`API model request: ${JSON.stringify(req)}`);
+    logger.debug(`API model request: ${logger.transformToString(req)}`);
 
     const modelResp = await repository.modelAsync(modelInfo, req.provider.access_token);
     return JSON.parse(Buffer.from(modelResp[0].content, 'base64').toString('utf8'));
@@ -109,7 +109,7 @@ const create = async (req, res) => {
         model: req.params.model,
         body: req.body
     };
-    logger.debug(`API create request: ${JSON.stringify(req)}`);
+    logger.debug(`API create request: ${logger.transformToString(req)}`);
 
     try {
         const createResp = await repository.createAsync(modelBody, req.provider.access_token);
@@ -130,7 +130,7 @@ const update = async (req, res) => {
         model: req.params.model,
         body: req.body
     };
-    logger.debug(`API update request: ${JSON.stringify(req)}`);
+    logger.debug(`API update request: ${logger.transformToString(req)}`);
 
     try {
         const updateResp = await repository.updateAsync(modelBody, req.provider.access_token);
@@ -150,7 +150,7 @@ const deleteModel = async (req, res) => {
         branch: req.params.branch,
         model: req.params.model
     };
-    logger.debug(`API deleteModel request: ${JSON.stringify(req)}`);
+    logger.debug(`API deleteModel request: ${logger.transformToString(req)}`);
 
     try {
         const deleteResp = await repository.deleteAsync(modelInfo, req.provider.access_token);
@@ -205,7 +205,7 @@ const organisation = (req, res) => {
         hostname: env.get().config.GITHUB_ENTERPRISE_HOSTNAME || 'www.github.com',
         port: env.get().config.GITHUB_ENTERPRISE_PORT || '',
     };
-    logger.debug(`API organisation request: ${JSON.stringify(req)}`);
+    logger.debug(`API organisation request: ${logger.transformToString(req)}`);
 
     return res.status(200).send(organisation);
 };

diff --git a/td.server/src/helpers/logger.helper.js b/td.server/src/helpers/logger.helper.js
@@ -69,6 +69,21 @@ class Logger {
         this.logger.log(level, message);
     }
 
+    transformToString (complexObject) {
+        const cache = [];
+        const resultString = JSON.stringify(complexObject, function(key, value) {
+          if (typeof value === "object" && value !== null) {
+            if (cache.indexOf(value) !== -1) {
+              // Circular reference found
+              return "[Circular]";
+            }
+            cache.push(value);
+          }
+          return value;
+        });
+        return resultString;
+    }
+
     log (level, message) { this.logger.log(level, this._formatMessage(this.service, message)); }
 
     silly (message) { this.logger.silly(this._formatMessage(this.service, message, 'silly')); }

diff --git a/td.server/test/helpers/logger.helper.spec.js b/td.server/test/helpers/logger.helper.spec.js
@@ -82,4 +82,13 @@ describe('helpers/logger.helper.js', () => {
     it('throws an error for an unknown level', () => {
         expect(() => logger.fatal('fatal')).to.throw;
     });
+
+    it('stringifies complex objects', () => {
+        const complexObject = {a: 'a', b: { ba: 'ba', bb: 'bb'}};
+        let objectString = logger.transformToString(complexObject);
+        expect(objectString).to.equal('{"a":"a","b":{"ba":"ba","bb":"bb"}}');
+        complexObject.b.bb = complexObject.b
+        objectString = logger.transformToString(complexObject);
+        expect(objectString).to.equal('{"a":"a","b":{"ba":"ba","bb":"[Circular]"}}');
+    })
 });