From 8d2a211a9fca667c7addf1e4570c20b6b5f1ed11 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 6 Mar 2022 21:22:08 +0800 Subject: [PATCH 1/3] chore(refactor): fileSyncAppender to have same internal code ordering as fileAppender --- lib/appenders/fileSync.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index b587dbe..72e6e80 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -175,15 +175,22 @@ class RollingFileSync { * if not provided then logs won't be rotated. * @param numBackups - the number of log files to keep after logSize * has been reached (default 5) - * @param timezoneOffset - optional timezone offset in minutes - * (default system local) - * @param options - passed as is to fs options + * @param options - options to be passed to the underlying stream + * @param timezoneOffset - optional timezone offset in minutes (default system local) */ -function fileAppender(file, layout, logSize, numBackups, timezoneOffset, options) { - debug('fileSync appender created'); +function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; + debug( + 'Creating fileSync appender (', + file, ', ', + logSize, ', ', + numBackups, ', ', + options, ', ', + timezoneOffset, ')' + ); + function openTheStream(filePath, fileSize, numFiles) { let stream; @@ -234,8 +241,8 @@ function configure(config, layouts) { layout, config.maxLogSize, config.backups, - config.timezoneOffset, - options + options, + config.timezoneOffset ); } From 4bc77b68a996885656907e689fbe053c43512800 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 6 Mar 2022 21:22:53 +0800 Subject: [PATCH 2/3] chore(refactor): fileAppender to have same internal code ordering as fileSyncAppender --- lib/appenders/file.js | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/appenders/file.js b/lib/appenders/file.js index aeccddb..0e10939 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -13,23 +13,6 @@ function mainSighupHandler() { }); } -function openTheStream(file, fileSize, numFiles, options) { - const stream = new streams.RollingFileStream( - file, - fileSize, - numFiles, - options - ); - stream.on('error', (err) => { - console.error('log4js.fileAppender - Writing to file %s, error happened ', file, err); // eslint-disable-line - }); - stream.on('drain', () => { - process.emit("log4js:pause", false); - }); - return stream; -} - - /** * File Appender writing the logs to a text file. Supports rolling of logs by size. * @@ -56,6 +39,22 @@ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset timezoneOffset, ')' ); + function openTheStream(filePath, fileSize, numFiles, opt) { + const stream = new streams.RollingFileStream( + filePath, + fileSize, + numFiles, + opt + ); + stream.on('error', (err) => { + console.error('log4js.fileAppender - Writing to file %s, error happened ', filePath, err); // eslint-disable-line + }); + stream.on('drain', () => { + process.emit("log4js:pause", false); + }); + return stream; + } + let writer = openTheStream(file, logSize, numBackups, options); const app = function (loggingEvent) { From e3a36db2327df0e3ee165afc55d2391a44c501d0 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 6 Mar 2022 19:43:13 +0800 Subject: [PATCH 3/3] chore(validation): added filename validation for fileAppender and filesyncAppender --- lib/appenders/file.js | 5 ++++- lib/appenders/fileSync.js | 5 ++++- test/tap/fileAppender-test.js | 21 +++++++++++++++++++++ test/tap/fileSyncAppender-test.js | 21 +++++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/appenders/file.js b/lib/appenders/file.js index 0e10939..2680fc1 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -16,7 +16,7 @@ function mainSighupHandler() { /** * File Appender writing the logs to a text file. Supports rolling of logs by size. * - * @param file file log messages will be written to + * @param file the file log messages will be written to * @param layout a function that takes a logEvent and returns a string * (defaults to basicLayout). * @param logSize - the maximum size (in bytes) for a log file, @@ -27,6 +27,9 @@ function mainSighupHandler() { * @param timezoneOffset - optional timezone offset in minutes (default system local) */ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { + if (typeof file !== "string" || file.length === 0) { + throw new Error(`Invalid filename: ${file}`); + } file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index 72e6e80..995ed78 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -168,7 +168,7 @@ class RollingFileSync { /** * File Appender writing the logs to a text file. Supports rolling of logs by size. * - * @param file file log messages will be written to + * @param file the file log messages will be written to * @param layout a function that takes a logevent and returns a string * (defaults to basicLayout). * @param logSize - the maximum size (in bytes) for a log file, @@ -179,6 +179,9 @@ class RollingFileSync { * @param timezoneOffset - optional timezone offset in minutes (default system local) */ function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { + if (typeof file !== "string" || file.length === 0) { + throw new Error(`Invalid filename: ${file}`); + } file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; diff --git a/test/tap/fileAppender-test.js b/test/tap/fileAppender-test.js index 95ee8a4..17f3b4b 100644 --- a/test/tap/fileAppender-test.js +++ b/test/tap/fileAppender-test.js @@ -46,6 +46,27 @@ test("log4js fileAppender", batch => { t.end(); }); + batch.test("should give error if invalid filename", async t => { + const file = ""; + const expectedError = new Error(`Invalid filename: ${file}`); + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "file", + filename: file + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + expectedError + ); + t.end(); + }); + batch.test("should flush logs on shutdown", async t => { const testFile = path.join(__dirname, "fa-default-test.log"); await removeFile(testFile); diff --git a/test/tap/fileSyncAppender-test.js b/test/tap/fileSyncAppender-test.js index ff4d60c..c7f0190 100644 --- a/test/tap/fileSyncAppender-test.js +++ b/test/tap/fileSyncAppender-test.js @@ -40,6 +40,27 @@ test("log4js fileSyncAppender", batch => { }); }); + batch.test("should give error if invalid filename", async t => { + const file = ""; + const expectedError = new Error(`Invalid filename: ${file}`); + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "fileSync", + filename: file + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + expectedError + ); + t.end(); + }); + batch.test("with a max file size and no backups", t => { const testFile = path.join(__dirname, "/fa-maxFileSize-sync-test.log"); const logger = log4js.getLogger("max-file-size");