Unify logfile names and create with more secure open options and perms.

Fixes MisterTea#555 (Tribute to Colin McRae)

src/base/LogHandler.cpp

@@ -23,13 +23,43 @@ el::Configurations LogHandler::setupLogHandler(int *argc, char ***argv) {
   return defaultConf;
 }
 
-void LogHandler::setupLogFile(el::Configurations *defaultConf, string filename,
-                              string maxlogsize) {
+/**
+ *  LogHandler::setupLogFile(
+ *      &defaultConf, (tmpDir + "/etclient-%datetime{%Y-%M-%d_%H_%m_%s}.log"));
+ *  LogHandler::stderrToFile((tmpDir + "/etclient"));
+ */
+void LogHandler::setupLogFiles(el::Configurations *defaultConf,
+                               const string &path, const string &filenamePrefix,
+                               bool logToStdout, bool redirectStderrToFile,
+                               string maxlogsize) {
+  time_t rawtime;
+  struct tm *timeinfo;
+  char buffer[80];
+  time(&rawtime);
+  timeinfo = localtime(&rawtime);
+  strftime(buffer, sizeof(buffer), "%Y-%m-%d_%H-%M-%S", timeinfo);
+  string current_time(buffer);
+  string logFilename =
+      path + "/" + filenamePrefix + "-" + current_time + ".log";
+  string stderrFilename =
+      path + "/" + filenamePrefix + "-stderr-" + current_time + ".log";
+  createLogFile(logFilename.c_str());
+
   // Enable strict log file size check
   el::Loggers::addFlag(el::LoggingFlag::StrictLogFileSizeCheck);
-  defaultConf->setGlobally(el::ConfigurationType::Filename, filename);
+  defaultConf->setGlobally(el::ConfigurationType::Filename, logFilename);
   defaultConf->setGlobally(el::ConfigurationType::ToFile, "true");
   defaultConf->setGlobally(el::ConfigurationType::MaxLogFileSize, maxlogsize);
+
+  if (logToStdout) {
+    defaultConf->setGlobally(el::ConfigurationType::ToStandardOutput, "true");
+  } else {
+    defaultConf->setGlobally(el::ConfigurationType::ToStandardOutput, "false");
+  }
+
+  if (redirectStderrToFile) {
+    stderrToFile(stderrFilename);
+  }
 }
 
 void LogHandler::rolloutHandler(const char *filename, std::size_t size) {
@@ -38,27 +68,6 @@ void LogHandler::rolloutHandler(const char *filename, std::size_t size) {
   remove(filename);
 }
 
-string LogHandler::stderrToFile(const string &pathPrefix) {
-  time_t rawtime;
-  struct tm *timeinfo;
-  char buffer[80];
-  time(&rawtime);
-  timeinfo = localtime(&rawtime);
-  strftime(buffer, sizeof(buffer), "%Y-%m-%d_%I-%M", timeinfo);
-  string current_time(buffer);
-  string stderrFilename = pathPrefix + "_stderr_" + current_time;
-  FILE *stderr_stream = freopen(stderrFilename.c_str(), "w", stderr);
-  fs::permissions(
-      stderrFilename,
-      fs::perms::owner_read | fs::perms::owner_write | fs::perms::group_read,
-      fs::perm_options::replace);
-  if (!stderr_stream) {
-    STFATAL << "Invalid filename " << stderrFilename;
-  }
-  setvbuf(stderr_stream, NULL, _IOLBF, BUFSIZ);  // set to line buffering
-  return stderrFilename;
-}
-
 void LogHandler::setupStdoutLogger() {
   el::Logger *stdoutLogger = el::Loggers::getLogger("stdout");
   // Easylogging configurations
@@ -70,4 +79,23 @@ void LogHandler::setupStdoutLogger() {
   stdoutConf.setGlobally(el::ConfigurationType::ToFile, "false");
   el::Loggers::reconfigureLogger(stdoutLogger, stdoutConf);
 }
+
+void LogHandler::createLogFile(const string &filename) {
+#ifdef WIN32
+  // O_NOFOLLOW does not exist on windows
+  FATAL_FAIL(::open(filename.c_str(), O_EXCL | O_CREAT, 0600));
+#else
+  FATAL_FAIL(::open(filename.c_str(), O_NOFOLLOW | O_EXCL | O_CREAT, 0600));
+#endif
+}
+
+void LogHandler::stderrToFile(const string &stderrFilename) {
+  createLogFile(stderrFilename.c_str());
+  FILE *stderr_stream = freopen(stderrFilename.c_str(), "w", stderr);
+  if (!stderr_stream) {
+    STFATAL << "Invalid filename " << stderrFilename;
+  }
+  setvbuf(stderr_stream, NULL, _IOLBF, BUFSIZ);  // set to line buffering
+}
+
 }  // namespace et

src/base/LogHandler.hpp

@@ -7,11 +7,17 @@ namespace et {
 class LogHandler {
  public:
   static el::Configurations setupLogHandler(int *argc, char ***argv);
-  static void setupLogFile(el::Configurations *defaultConf, string filename,
-                           string maxlogsize = "20971520");
+  static void setupLogFiles(el::Configurations *defaultConf, const string &path,
+                            const string &filenamePrefix,
+                            bool logToStdout = false,
+                            bool redirectStderrToFile = false,
+                            string maxlogsize = "20971520");
   static void rolloutHandler(const char *filename, std::size_t size);
-  static string stderrToFile(const string &pathPrefix);
   static void setupStdoutLogger();
+
+ private:
+  static void stderrToFile(const string &stderrFilename);
+  static void createLogFile(const string &filename);
 };
 }  // namespace et
 #endif  // __ET_LOG_HANDLER__

src/htm/HtmClientMain.cpp

@@ -61,14 +61,9 @@ int main(int argc, char** argv) {
 
   // Setup easylogging configurations
   el::Configurations defaultConf = LogHandler::setupLogHandler(&argc, &argv);
-  defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false");
   el::Loggers::setVerboseLevel(3);
-  // default max log file size is 20MB for etserver
-  string maxlogsize = "20971520";
-  LogHandler::setupLogFile(&defaultConf, GetTempDirectory() + "htm.log",
-                           maxlogsize);
-  // Redirect std streams to a file
-  LogHandler::stderrToFile(GetTempDirectory() + "htm");
+  LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "htm", false,
+                            true);
 
   // Reconfigure default logger to apply settings above
   el::Loggers::reconfigureLogger("default", defaultConf);

src/htm/HtmServerMain.cpp

@@ -13,14 +13,9 @@ int main(int argc, char **argv) {
   // Setup easylogging configurations
   el::Configurations defaultConf =
       et::LogHandler::setupLogHandler(&argc, &argv);
-  defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false");
   el::Loggers::setVerboseLevel(3);
-  // default max log file size is 20MB for etserver
-  string maxlogsize = "20971520";
-  LogHandler::setupLogFile(&defaultConf, GetTempDirectory() + "htmd.log",
-                           maxlogsize);
-  // Redirect std streams to a file
-  LogHandler::stderrToFile(GetTempDirectory() + "htmd");
+  LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "htmd", false,
+                            true);
 
   // Reconfigure default logger to apply settings above
   el::Loggers::reconfigureLogger("default", defaultConf);

src/terminal/TerminalClientMain.cpp

@@ -119,21 +119,14 @@ int main(int argc, char** argv) {
 
     el::Loggers::setVerboseLevel(result["verbose"].as<int>());
 
-    if (result.count("logtostdout")) {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true");
-    } else {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false");
-      // Redirect std streams to a file
-      LogHandler::stderrToFile((tmpDir + "/etclient"));
-    }
-
     // silent Flag, since etclient doesn't read /etc/et.cfg file
     if (result.count("silent")) {
       defaultConf.setGlobally(el::ConfigurationType::Enabled, "false");
     }
 
-    LogHandler::setupLogFile(
-        &defaultConf, (tmpDir + "/etclient-%datetime{%Y-%M-%d_%H_%m_%s}.log"));
+    LogHandler::setupLogFiles(&defaultConf, tmpDir, "etclient",
+                              result.count("logtostdout"),
+                              !result.count("logtostdout"));
 
     el::Loggers::reconfigureLogger("default", defaultConf);
     // set thread name

src/terminal/TerminalMain.cpp

@@ -63,15 +63,6 @@ int main(int argc, char** argv) {
 
     el::Loggers::setVerboseLevel(result["verbose"].as<int>());
 
-    if (result.count("logtostdout")) {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true");
-    } else {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false");
-    }
-
-    // default max log file size is 20MB for etserver
-    string maxlogsize = "20971520";
-
     GOOGLE_PROTOBUF_VERIFY_VERSION;
     srand(1);
 
@@ -147,10 +138,9 @@ int main(int argc, char** argv) {
     string username = string(ssh_get_local_username());
     if (result.count("jump")) {
       // etserver with --jump cannot write to the default log file(root)
-      LogHandler::setupLogFile(
-          &defaultConf,
-          GetTempDirectory() + "etjump-" + username + "-" + id + ".log",
-          maxlogsize);
+      LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(),
+                                ("etjump-" + username + "-" + id),
+                                result.count("logtostdout"), false);
       // Reconfigure default logger to apply settings above
       el::Loggers::reconfigureLogger("default", defaultConf);
       // set thread name
@@ -177,10 +167,10 @@ int main(int argc, char** argv) {
     }
 
     // etserver with --idpasskey cannot write to the default log file(root)
-    LogHandler::setupLogFile(
-        &defaultConf,
-        GetTempDirectory() + "etterminal-" + username + "-" + id + ".log",
-        maxlogsize);
+    LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(),
+                              ("etterminal-" + username + "-" + id),
+                              result.count("logtostdout"), false);
+
     // Reconfigure default logger to apply settings above
     el::Loggers::reconfigureLogger("default", defaultConf);
     // set thread name

src/terminal/TerminalServerMain.cpp

@@ -70,14 +70,6 @@ int main(int argc, char **argv) {
       }
     }
 
-    if (result.count("logtostdout")) {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true");
-    } else {
-      defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "false");
-      // Redirect std streams to a file
-      LogHandler::stderrToFile(GetTempDirectory() + "etserver");
-    }
-
     ServerFifoPath serverFifo;
 
     // default max log file size is 20MB for etserver
@@ -165,9 +157,9 @@ int main(int argc, char **argv) {
     }
 
     // Set log file for etserver process here.
-    LogHandler::setupLogFile(&defaultConf,
-                             GetTempDirectory() + "etserver-%datetime.log",
-                             maxlogsize);
+    LogHandler::setupLogFiles(&defaultConf, GetTempDirectory(), "etserver",
+                              result.count("logtostdout"),
+                              !result.count("logtostdout"), maxlogsize);
     // Reconfigure default logger to apply settings above
     el::Loggers::reconfigureLogger("default", defaultConf);
     // set thread name

test/Main.cpp

@@ -13,17 +13,14 @@ int main(int argc, char **argv) {
   el::Configurations defaultConf =
       et::LogHandler::setupLogHandler(&argc, &argv);
   et::LogHandler::setupStdoutLogger();
-  defaultConf.setGlobally(el::ConfigurationType::ToStandardOutput, "true");
-  defaultConf.setGlobally(el::ConfigurationType::ToFile, "true");
   // el::Loggers::setVerboseLevel(9);
 
   et::HandleTerminate();
 
   string logDirectoryPattern = GetTempDirectory() + string("et_test_XXXXXXXX");
   string logDirectory = string(mkdtemp(&logDirectoryPattern[0]));
-  string logPath = string(logDirectory) + "/log";
-  CLOG(INFO, "stdout") << "Writing log to " << logPath << endl;
-  et::LogHandler::setupLogFile(&defaultConf, logPath);
+  CLOG(INFO, "stdout") << "Writing log to " << logDirectory << endl;
+  et::LogHandler::setupLogFiles(&defaultConf, logDirectory, "log", true);
 
   // Reconfigure default logger to apply settings above
   el::Loggers::reconfigureLogger("default", defaultConf);
@@ -35,7 +32,6 @@ int main(int argc, char **argv) {
   TelemetryService::get()->shutdown();
   TelemetryService::destroy();
 
-  FATAL_FAIL(::remove(logPath.c_str()));
-  FATAL_FAIL(::remove(logDirectory.c_str()));
+  FATAL_FAIL(fs::remove_all(logDirectory.c_str()));
   return result;
 }