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 16dc538..24b3f9a 100755 --- a/lib/appenders/fileSync.js +++ b/lib/appenders/fileSync.js @@ -6,15 +6,10 @@ 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 { - 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) { @@ -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 { @@ -170,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(); });