Skip to content

Commit

Permalink
Merge pull request #47778 from dominictb/fix/47509-log-serialize
Browse files Browse the repository at this point in the history
fix: serialize request.data in logging middleware
  • Loading branch information
puneetlath authored Sep 4, 2024
2 parents 6760f96 + 876adf8 commit aae4c2e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
35 changes: 34 additions & 1 deletion src/libs/Middleware/Logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,34 @@ import type Request from '@src/types/onyx/Request';
import type Response from '@src/types/onyx/Response';
import type Middleware from './types';

function getCircularReplacer() {
const ancestors: unknown[] = [];
return function (this: unknown, key: string, value: unknown): unknown {
if (typeof value !== 'object' || value === null) {
return value;
}
// `this` is the object that value is contained in, i.e the direct parent
// eslint-disable-next-line no-invalid-this
while (ancestors.length > 0 && ancestors.at(-1) !== this) {
ancestors.pop();
}
if (ancestors.includes(value)) {
return '[Circular]';
}
ancestors.push(value);
return value;
};
}

function serializeLoggingData<T extends Record<string, unknown> | undefined>(logData: T): T | null {
try {
return JSON.parse(JSON.stringify(logData, getCircularReplacer())) as T;
} catch (error) {
Log.hmmm('Failed to serialize log data', {error});
return null;
}
}

function logRequestDetails(message: string, request: Request, response?: Response | void) {
// Don't log about log or else we'd cause an infinite loop
if (request.command === 'Log') {
Expand Down Expand Up @@ -38,7 +66,10 @@ function logRequestDetails(message: string, request: Request, response?: Respons
* requests because they contain sensitive information.
*/
if (request.command !== 'AuthenticatePusher') {
extraData.request = request;
extraData.request = {
...request,
data: serializeLoggingData(request.data),
};
extraData.response = response;
}

Expand Down Expand Up @@ -125,3 +156,5 @@ const Logging: Middleware = (response, request) => {
};

export default Logging;

export {serializeLoggingData};
28 changes: 28 additions & 0 deletions tests/unit/LoggingMiddlewareTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {serializeLoggingData} from '@libs/Middleware/Logging';

describe('LoggingMiddleware', () => {
describe('getCircularReplacer', () => {
it('should return "[Circular]" for circular references', () => {
const obj: Record<string, unknown> = {};
obj.obj = obj;
const result = serializeLoggingData(obj);
expect(result).toEqual({obj: '[Circular]'});
});

it('should return the original value for non-circular references', () => {
const obj: Record<string, unknown> = {};
obj.foo = 'bar';
const result = serializeLoggingData(obj);
expect(result).toEqual({foo: 'bar'});
});

it('should not stringify function in the object', () => {
const obj: Record<string, unknown> = {
foo: () => 'bar',
baz: 'baz',
};
const result = serializeLoggingData(obj);
expect(result).toEqual({baz: 'baz'});
});
});
});

0 comments on commit aae4c2e

Please sign in to comment.