From d981a852397b825f136e75d0a22efa3552fafaf8 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 29 Nov 2021 11:43:43 +0530 Subject: [PATCH 1/5] Filter token out of stack trace --- src/logging/LogItem.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/logging/LogItem.ts b/src/logging/LogItem.ts index da009b06..bb8815db 100644 --- a/src/logging/LogItem.ts +++ b/src/logging/LogItem.ts @@ -19,6 +19,11 @@ import {LogLevel, LogFilter} from "./LogFilter"; import type {BaseLogger} from "./BaseLogger"; import type {ISerializedItem, ILogItem, LogItemValues, LabelOrValues, FilterCreator, LogCallback} from "./types"; +// Make sure that loginToken does not end up in the logs +function filterLoginToken(trace?: string): string | undefined { + return trace?.replace(/(?<=\/\?loginToken=).+/, ""); +} + export class LogItem implements ILogItem { public readonly start: number; public logLevel: LogLevel; @@ -155,7 +160,7 @@ export class LogItem implements ILogItem { if (this.error) { // (e)rror item.e = { - stack: this.error.stack, + stack: filterLoginToken(this.error.stack), name: this.error.name, message: this.error.message.split("\n")[0] }; @@ -259,3 +264,14 @@ export class LogItem implements ILogItem { return this._children; } } + +export function tests() { + return { + "Login token removed from item": (assert) => { + const str = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26"; + const result = filterLoginToken(str); + const index = result?.search("secret"); + assert.equal(index, -1); + } + } +} From 104590e34dbb278735005f237940bd7c2b43f118 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Mon, 29 Nov 2021 11:48:05 +0530 Subject: [PATCH 2/5] Use ! in test --- src/logging/LogItem.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logging/LogItem.ts b/src/logging/LogItem.ts index bb8815db..4ddd687c 100644 --- a/src/logging/LogItem.ts +++ b/src/logging/LogItem.ts @@ -270,7 +270,7 @@ export function tests() { "Login token removed from item": (assert) => { const str = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26"; const result = filterLoginToken(str); - const index = result?.search("secret"); + const index = result!.search("secret"); assert.equal(index, -1); } } From fe77b71c97d20e3d82c2ee39468e4550e195100d Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 30 Nov 2021 13:28:28 +0530 Subject: [PATCH 3/5] use transformer function --- src/logging/BaseLogger.ts | 6 +++-- src/logging/IDBLogger.ts | 13 ++++++---- src/logging/LogItem.ts | 27 +++++++++----------- src/platform/web/Platform.js | 48 ++++++++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 28 deletions(-) diff --git a/src/logging/BaseLogger.ts b/src/logging/BaseLogger.ts index 723c2f17..e32b9f0f 100644 --- a/src/logging/BaseLogger.ts +++ b/src/logging/BaseLogger.ts @@ -17,15 +17,17 @@ limitations under the License. import {LogItem} from "./LogItem"; import {LogLevel, LogFilter} from "./LogFilter"; -import type {ILogger, ILogExport, FilterCreator, LabelOrValues, LogCallback, ILogItem} from "./types"; +import type {ILogger, ILogExport, FilterCreator, LabelOrValues, LogCallback, ILogItem, ISerializedItem} from "./types"; import type {Platform} from "../platform/web/Platform.js"; export abstract class BaseLogger implements ILogger { protected _openItems: Set = new Set(); protected _platform: Platform; + protected _serializedTransformer: (item: ISerializedItem) => ISerializedItem; - constructor({platform}) { + constructor({platform, serializedTransformer = (item: ISerializedItem) => item}) { this._platform = platform; + this._serializedTransformer = serializedTransformer; } log(labelOrValues: LabelOrValues, logLevel: LogLevel = LogLevel.Info): void { diff --git a/src/logging/IDBLogger.ts b/src/logging/IDBLogger.ts index 96147ca1..8ab81964 100644 --- a/src/logging/IDBLogger.ts +++ b/src/logging/IDBLogger.ts @@ -26,7 +26,7 @@ import {BaseLogger} from "./BaseLogger"; import type {Interval} from "../platform/web/dom/Clock"; import type {Platform} from "../platform/web/Platform.js"; import type {BlobHandle} from "../platform/web/dom/BlobHandle.js"; -import type {ILogItem, ILogExport} from "./types"; +import type {ILogItem, ILogExport, ISerializedItem} from "./types"; import type {LogFilter} from "./LogFilter"; type QueuedItem = { @@ -40,7 +40,7 @@ export class IDBLogger extends BaseLogger { private readonly _flushInterval: Interval; private _queuedItems: QueuedItem[]; - constructor(options: {name: string, flushInterval?: number, limit?: number, platform: Platform}) { + constructor(options: {name: string, flushInterval?: number, limit?: number, platform: Platform, serializedTransformer: (item: ISerializedItem) => ISerializedItem}) { super(options); const {name, flushInterval = 60 * 1000, limit = 3000} = options; this._name = name; @@ -119,9 +119,12 @@ export class IDBLogger extends BaseLogger { _persistItem(logItem: ILogItem, filter: LogFilter, forced: boolean): void { const serializedItem = logItem.serialize(filter, undefined, forced); - this._queuedItems.push({ - json: JSON.stringify(serializedItem) - }); + if (serializedItem) { + const transformedSerializedItem = this._serializedTransformer(serializedItem); + this._queuedItems.push({ + json: JSON.stringify(transformedSerializedItem) + }); + } } _persistQueuedItems(items: QueuedItem[]): void { diff --git a/src/logging/LogItem.ts b/src/logging/LogItem.ts index 4ddd687c..5bd94f93 100644 --- a/src/logging/LogItem.ts +++ b/src/logging/LogItem.ts @@ -19,11 +19,6 @@ import {LogLevel, LogFilter} from "./LogFilter"; import type {BaseLogger} from "./BaseLogger"; import type {ISerializedItem, ILogItem, LogItemValues, LabelOrValues, FilterCreator, LogCallback} from "./types"; -// Make sure that loginToken does not end up in the logs -function filterLoginToken(trace?: string): string | undefined { - return trace?.replace(/(?<=\/\?loginToken=).+/, ""); -} - export class LogItem implements ILogItem { public readonly start: number; public logLevel: LogLevel; @@ -160,7 +155,7 @@ export class LogItem implements ILogItem { if (this.error) { // (e)rror item.e = { - stack: filterLoginToken(this.error.stack), + stack: this.error.stack, name: this.error.name, message: this.error.message.split("\n")[0] }; @@ -265,13 +260,13 @@ export class LogItem implements ILogItem { } } -export function tests() { - return { - "Login token removed from item": (assert) => { - const str = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26"; - const result = filterLoginToken(str); - const index = result!.search("secret"); - assert.equal(index, -1); - } - } -} +// export function tests() { +// return { +// "Login token removed from item": (assert) => { +// const str = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26"; +// const result = filterLoginToken(str); +// const index = result!.search("secret"); +// assert.equal(index, -1); +// } +// } +// } diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js index f6cd2895..c871bb9e 100644 --- a/src/platform/web/Platform.js +++ b/src/platform/web/Platform.js @@ -132,11 +132,7 @@ export class Platform { this.clock = new Clock(); this.encoding = new Encoding(); this.random = Math.random; - if (options?.development) { - this.logger = new ConsoleLogger({platform: this}); - } else { - this.logger = new IDBLogger({name: "hydrogen_logs", platform: this}); - } + this._createLogger(options?.development); this.history = new History(); this.onlineStatus = new OnlineStatus(); this._serviceWorkerHandler = null; @@ -162,6 +158,21 @@ export class Platform { this._disposables = new Disposables(); } + _createLogger(isDevelopment) { + // Make sure that loginToken does not end up in the logs + const transformer = (item) => { + if (item.e?.stack) { + item.e.stack = item.e.stack.replace(/(?<=\/\?loginToken=).+/, ""); + } + return item; + }; + if (isDevelopment) { + this.logger = new ConsoleLogger({platform: this}); + } else { + this.logger = new IDBLogger({name: "hydrogen_logs", platform: this, serializedTransformer: transformer}); + } + } + get updateService() { return this._serviceWorkerHandler; } @@ -272,3 +283,30 @@ export class Platform { this._disposables.dispose(); } } + +import {LogItem} from "../../logging/LogItem"; +export function tests() { + return { + "loginToken should not be in logs": (assert) => { + const transformer = (item) => { + if (item.e?.stack) { + item.e.stack = item.e.stack.replace(/(?<=\/\?loginToken=).+/, ""); + } + return item; + }; + const logger = { + _queuedItems: [], + _serializedTransformer: transformer, + _now: () => {} + }; + logger.persist = IDBLogger.prototype._persistItem.bind(logger); + const logItem = new LogItem("test", 1, logger); + logItem.error = new Error(); + logItem.error.stack = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26" + logger.persist(logItem, null, false); + const item = logger._queuedItems.pop(); + console.log(item); + assert.strictEqual(item.json.search("secret"), -1); + } + }; +} From 6699b71bd54cf70e43e9a0edfb41c8fb42bc92b6 Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 30 Nov 2021 13:38:25 +0530 Subject: [PATCH 4/5] transformer is optional --- src/logging/IDBLogger.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logging/IDBLogger.ts b/src/logging/IDBLogger.ts index 8ab81964..ab9474b0 100644 --- a/src/logging/IDBLogger.ts +++ b/src/logging/IDBLogger.ts @@ -40,7 +40,7 @@ export class IDBLogger extends BaseLogger { private readonly _flushInterval: Interval; private _queuedItems: QueuedItem[]; - constructor(options: {name: string, flushInterval?: number, limit?: number, platform: Platform, serializedTransformer: (item: ISerializedItem) => ISerializedItem}) { + constructor(options: {name: string, flushInterval?: number, limit?: number, platform: Platform, serializedTransformer?: (item: ISerializedItem) => ISerializedItem}) { super(options); const {name, flushInterval = 60 * 1000, limit = 3000} = options; this._name = name; From 66fbc37ec4e45a6603737312117539753c288e6e Mon Sep 17 00:00:00 2001 From: RMidhunSuresh Date: Tue, 30 Nov 2021 14:15:49 +0530 Subject: [PATCH 5/5] Remove comments --- src/logging/LogItem.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/logging/LogItem.ts b/src/logging/LogItem.ts index 5bd94f93..da009b06 100644 --- a/src/logging/LogItem.ts +++ b/src/logging/LogItem.ts @@ -259,14 +259,3 @@ export class LogItem implements ILogItem { return this._children; } } - -// export function tests() { -// return { -// "Login token removed from item": (assert) => { -// const str = "main http://localhost:3000/src/main.js:55\n http://localhost:3000/?loginToken=secret:26"; -// const result = filterLoginToken(str); -// const index = result!.search("secret"); -// assert.equal(index, -1); -// } -// } -// }