From 505204addc02831f248a5aae9fe71f611125c611 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 22 May 2022 15:47:00 +0800 Subject: [PATCH 1/3] style: white-space --- lib/appenders/fileSync.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index 16dc538..763da33 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -14,7 +14,7 @@ function touchFile(file, options) { // attempt to create the directory const mkdir = (dir) => { try { - return fs.mkdirSync(dir, {recursive: true}); + return fs.mkdirSync(dir, { recursive: true }); } // backward-compatible fs.mkdirSync for nodejs pre-10.12.0 (without recursive option) catch (e) { From 6de1da298e7745fe036a523181a278d0ad66184a Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 22 May 2022 15:49:51 +0800 Subject: [PATCH 2/3] refactor: code flow and readability --- lib/appenders/fileSync.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index 763da33..7519de5 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -6,11 +6,6 @@ const os = require('os'); const eol = os.EOL; function touchFile(file, options) { - // if the file exists, nothing to do - if (fs.existsSync(file)) { - return; - } - // attempt to create the directory const mkdir = (dir) => { try { @@ -45,9 +40,8 @@ function touchFile(file, options) { }; mkdir(path.dirname(file)); - // touch the file to apply flags (like w to truncate the file) - const id = fs.openSync(file, options.flags, options.mode); - fs.closeSync(id); + // try to throw EISDIR, EROFS, EACCES + fs.appendFileSync(file, "", { mode: options.mode, flags: options.flag }); } class RollingFileSync { From d182204079eaa7ef8df8965c5b8045b559eb1a55 Mon Sep 17 00:00:00 2001 From: Lam Wei Li Date: Sun, 22 May 2022 15:48:09 +0800 Subject: [PATCH 3/3] fix: filename validation (cannot be directory) --- lib/appenders/file.js | 2 ++ lib/appenders/fileSync.js | 2 ++ test/tap/fileAppender-test.js | 19 +++++++++++++++++-- test/tap/fileSyncAppender-test.js | 19 +++++++++++++++++-- 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/appenders/file.js b/lib/appenders/file.js index 73dad71..673e348 100644 --- a/lib/appenders/file.js +++ b/lib/appenders/file.js @@ -29,6 +29,8 @@ function mainSighupHandler() { function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { if (typeof file !== "string" || file.length === 0) { throw new Error(`Invalid filename: ${file}`); + } else if (file.endsWith(path.sep)) { + throw new Error(`Filename is a directory: ${file}`); } file = path.normalize(file); numBackups = (!numBackups && numBackups !== 0) ? 5 : numBackups; diff --git a/lib/appenders/fileSync.js b/lib/appenders/fileSync.js index 7519de5..24b3f9a 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -164,6 +164,8 @@ class RollingFileSync { function fileAppender(file, layout, logSize, numBackups, options, timezoneOffset) { if (typeof file !== "string" || file.length === 0) { throw new Error(`Invalid filename: ${file}`); + } else if (file.endsWith(path.sep)) { + throw new Error(`Filename is a directory: ${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 5ca1f6a..ebc298b 100644 --- a/test/tap/fileAppender-test.js +++ b/test/tap/fileAppender-test.js @@ -50,7 +50,6 @@ test("log4js fileAppender", batch => { batch.test("should give error if invalid filename", async t => { const file = ""; - const expectedError = new Error(`Invalid filename: ${file}`); t.throws( () => log4js.configure({ @@ -64,7 +63,23 @@ test("log4js fileAppender", batch => { default: { appenders: ["file"], level: "debug" } } }), - expectedError + new Error(`Invalid filename: ${file}`) + ); + const dir = `.${path.sep}`; + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "file", + filename: dir + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + new Error(`Filename is a directory: ${dir}`) ); t.end(); }); diff --git a/test/tap/fileSyncAppender-test.js b/test/tap/fileSyncAppender-test.js index b36e3b0..4ea96ac 100644 --- a/test/tap/fileSyncAppender-test.js +++ b/test/tap/fileSyncAppender-test.js @@ -78,7 +78,6 @@ 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({ @@ -92,7 +91,23 @@ test("log4js fileSyncAppender", batch => { default: { appenders: ["file"], level: "debug" } } }), - expectedError + new Error(`Invalid filename: ${file}`) + ); + const dir = `.${path.sep}`; + t.throws( + () => + log4js.configure({ + appenders: { + file: { + type: "fileSync", + filename: dir + } + }, + categories: { + default: { appenders: ["file"], level: "debug" } + } + }), + new Error(`Filename is a directory: ${dir}`) ); t.end(); });