Merge pull request #1201 from log4js-node/filename-validation

chore(validation): added filename validation

lib/appenders/file.js

@@ -13,27 +13,10 @@ 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.
  *
- * @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,
@@ -44,6 +27,9 @@ function openTheStream(file, fileSize, numFiles, options) {
  * @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;
 
@@ -56,6 +42,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) {

lib/appenders/fileSync.js

@@ -168,22 +168,32 @@ 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,
  *   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) {
+  if (typeof file !== "string" || file.length === 0) {
+    throw new Error(`Invalid filename: ${file}`);
+  }
   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 +244,8 @@ function configure(config, layouts) {
     layout,
     config.maxLogSize,
     config.backups,
-    config.timezoneOffset,
-    options
+    options,
+    config.timezoneOffset
   );
 }
 

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);

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");