diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index e1cb0984..a22c3fd3 100644 --- a/cmd/launcher/main.go +++ b/cmd/launcher/main.go @@ -43,9 +43,10 @@ import ( ) const ( - appName = "Proton Mail Launcher" - exeName = "bridge" - guiName = "bridge-gui" + appName = "Proton Mail Launcher" + exeName = "bridge" + guiName = "bridge-gui" + launcherName = "launcher" FlagCLI = "cli" FlagCLIShort = "c" @@ -53,6 +54,7 @@ const ( FlagNonInteractiveShort = "n" FlagLauncher = "--launcher" FlagWait = "--wait" + FlagSessionID = "--session-id" ) func main() { //nolint:funlen @@ -75,9 +77,11 @@ func main() { //nolint:funlen if err != nil { l.WithError(err).Fatal("Failed to get logs path") } - crashHandler.AddRecoveryAction(logging.DumpStackTrace(logsPath)) - if err := logging.Init(logsPath, os.Getenv("VERBOSITY")); err != nil { + sessionID := logging.NewSessionID() + crashHandler.AddRecoveryAction(logging.DumpStackTrace(logsPath, sessionID, launcherName)) + + if err := logging.Init(logsPath, sessionID, launcherName, os.Getenv("VERBOSITY")); err != nil { l.WithError(err).Fatal("Failed to setup logging") } @@ -134,7 +138,7 @@ func main() { //nolint:funlen } } - cmd := execabs.Command(exe, appendLauncherPath(launcher, args)...) //nolint:gosec + cmd := execabs.Command(exe, appendLauncherPath(launcher, append(args, FlagSessionID, string(sessionID)))...) //nolint:gosec cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/internal/app/app.go b/internal/app/app.go index fb2e63cc..c1fafb16 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -76,10 +76,12 @@ const ( flagNoWindow = "no-window" flagParentPID = "parent-pid" flagSoftwareRenderer = "software-renderer" + flagSessionID = "session-id" ) const ( - appUsage = "Proton Mail IMAP and SMTP Bridge" + appUsage = "Proton Mail IMAP and SMTP Bridge" + appShortName = "bridge" ) func New() *cli.App { @@ -150,6 +152,10 @@ func New() *cli.App { Hidden: true, Value: false, }, + &cli.StringFlag{ + Name: flagSessionID, + Hidden: true, + }, } app.Action = run @@ -311,12 +317,13 @@ func withLogging(c *cli.Context, crashHandler *crash.Handler, locations *locatio logrus.WithField("path", logsPath).Debug("Received logs path") // Initialize logging. - if err := logging.Init(logsPath, c.String(flagLogLevel)); err != nil { + sessionID := logging.NewSessionIDFromString(c.String(flagSessionID)) + if err := logging.Init(logsPath, sessionID, appShortName, c.String(flagLogLevel)); err != nil { return fmt.Errorf("could not initialize logging: %w", err) } // Ensure we dump a stack trace if we crash. - crashHandler.AddRecoveryAction(logging.DumpStackTrace(logsPath)) + crashHandler.AddRecoveryAction(logging.DumpStackTrace(logsPath, sessionID, appShortName)) logrus. WithField("appName", constants.FullAppName). diff --git a/internal/app/bridge.go b/internal/app/bridge.go index 280d444f..0600e39c 100644 --- a/internal/app/bridge.go +++ b/internal/app/bridge.go @@ -44,7 +44,7 @@ import ( // deleteOldGoIMAPFiles Set with `-ldflags -X app.deleteOldGoIMAPFiles=true` to enable cleanup of old imap cache data. var deleteOldGoIMAPFiles bool //nolint:gochecknoglobals -// withBridge creates creates and tears down the bridge. +// withBridge creates and tears down the bridge. func withBridge( c *cli.Context, exe string, diff --git a/internal/bridge/bug_report.go b/internal/bridge/bug_report.go index 10fb30e0..cd240c75 100644 --- a/internal/bridge/bug_report.go +++ b/internal/bridge/bug_report.go @@ -54,14 +54,14 @@ func (bridge *Bridge) ReportBug(ctx context.Context, osType, osVersion, descript if attachLogs { logs, err := getMatchingLogs(bridge.locator, func(filename string) bool { - return logging.MatchLogName(filename) && !logging.MatchStackTraceName(filename) + return logging.MatchBridgeLogName(filename) && !logging.MatchStackTraceName(filename) }) if err != nil { return err } crashes, err := getMatchingLogs(bridge.locator, func(filename string) bool { - return logging.MatchLogName(filename) && logging.MatchStackTraceName(filename) + return logging.MatchBridgeLogName(filename) && logging.MatchStackTraceName(filename) }) if err != nil { return err diff --git a/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp b/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp index 420e7901..f01def14 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/CommandLine.cpp @@ -19,6 +19,7 @@ #include "Pch.h" #include "CommandLine.h" #include "Settings.h" +#include using namespace bridgepp; @@ -142,5 +143,12 @@ CommandLineOptions parseCommandLine(int argc, char *argv[]) { options.logLevel = parseLogLevel(argc, argv); + options.sessionID = parseGoCLIStringArgument(argc, argv, { "session-id" }); + if (options.sessionID.isEmpty()) { + options.sessionID = newSessionID(); + options.bridgeArgs.append("--session-id"); + options.bridgeArgs.append(options.sessionID); + } + return options; } diff --git a/internal/frontend/bridge-gui/bridge-gui/CommandLine.h b/internal/frontend/bridge-gui/bridge-gui/CommandLine.h index 9accfee8..6668c9ef 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CommandLine.h +++ b/internal/frontend/bridge-gui/bridge-gui/CommandLine.h @@ -34,6 +34,7 @@ struct CommandLineOptions { bridgepp::Log::Level logLevel { bridgepp::Log::defaultLevel }; ///< The log level bool noWindow { false }; ///< Should the application start without displaying the main window? bool useSoftwareRenderer { false }; ///< Should QML be renderer in software (i.e. without rendering hardware interface). + QString sessionID; ///< The sessionID. }; diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp index 556699a6..f3961b4b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.cpp @@ -19,7 +19,6 @@ #include "LogUtils.h" #include "BuildConfig.h" #include -#include using namespace bridgepp; @@ -28,7 +27,7 @@ using namespace bridgepp; //**************************************************************************************************************************************************** /// \return A reference to the log. //**************************************************************************************************************************************************** -Log &initLog() { +Log &initLog(QString const &sessionID) { Log &log = app().log(); log.registerAsQtMessageHandler(); log.setEchoInConsole(true); @@ -41,7 +40,7 @@ Log &initLog() { // create new GUI log file QString error; - if (!log.startWritingToFile(logsDir.absoluteFilePath(QString("gui_v%1_%2.log").arg(PROJECT_VER).arg(QDateTime::currentSecsSinceEpoch())), &error)) { + if (!log.startWritingToFile(logsDir.absoluteFilePath(QString("%1_000_gui_v%2_%3.log").arg(sessionID, PROJECT_VER, PROJECT_TAG)), &error)) { log.error(error); } diff --git a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h index b324301e..34161fdc 100644 --- a/internal/frontend/bridge-gui/bridge-gui/LogUtils.h +++ b/internal/frontend/bridge-gui/bridge-gui/LogUtils.h @@ -23,7 +23,7 @@ #include -bridgepp::Log &initLog(); ///< Initialize the application log. +bridgepp::Log &initLog(QString const &sessionID); ///< Initialize the application log. #endif //BRIDGE_GUI_LOG_UTILS_H diff --git a/internal/frontend/bridge-gui/bridge-gui/main.cpp b/internal/frontend/bridge-gui/bridge-gui/main.cpp index 617d8b85..c357562f 100644 --- a/internal/frontend/bridge-gui/bridge-gui/main.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/main.cpp @@ -286,7 +286,8 @@ int main(int argc, char *argv[]) { initQtApplication(); - Log &log = initLog(); + CommandLineOptions const cliOptions = parseCommandLine(argc, argv); + Log &log = initLog(cliOptions.sessionID); QLockFile lock(bridgepp::userCacheDir() + "/" + bridgeGUILock); if (!checkSingleInstance(lock)) { @@ -294,8 +295,6 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - CommandLineOptions const cliOptions = parseCommandLine(argc, argv); - #ifdef Q_OS_MACOS registerSecondInstanceHandler(); setDockIconVisibleState(!cliOptions.noWindow); diff --git a/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt b/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt index c1c4a04f..1d26b26d 100644 --- a/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt +++ b/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt @@ -32,9 +32,9 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) if (NOT DEFINED BRIDGE_APP_VERSION) message(FATAL_ERROR "BRIDGE_APP_VERSION is not defined.") -else() +else () message(STATUS "Bridge version is ${BRIDGE_APP_VERSION}") -endif() +endif () #**************************************************************************************************************************************************** @@ -148,6 +148,7 @@ add_library(bridgepp bridgepp/Log/Log.h bridgepp/Log/Log.cpp bridgepp/Log/LogUtils.h bridgepp/Log/LogUtils.cpp bridgepp/ProcessMonitor.cpp bridgepp/ProcessMonitor.h + bridgepp/SessionID/SessionID.cpp bridgepp/SessionID/SessionID.h bridgepp/User/User.cpp bridgepp/User/User.h bridgepp/Worker/Worker.h bridgepp/Worker/Overseer.h bridgepp/Worker/Overseer.cpp) @@ -167,7 +168,7 @@ target_precompile_headers(bridgepp PRIVATE Pch.h) if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.24.0") cmake_policy(SET CMP0135 NEW) # avoid warning DOWNLOAD_EXTRACT_TIMESTAMP -endif() +endif () include(FetchContent) FetchContent_Declare( @@ -188,7 +189,9 @@ enable_testing() add_executable(bridgepp-test EXCLUDE_FROM_ALL Test/TestBridgeUtils.cpp Test/TestException.cpp - Test/TestWorker.cpp Test/TestWorker.h) + Test/TestSessionID.cpp + Test/TestWorker.cpp Test/TestWorker.h + ) add_dependencies(bridgepp-test bridgepp) target_precompile_headers(bridgepp-test PRIVATE Pch.h) target_link_libraries(bridgepp-test diff --git a/internal/frontend/bridge-gui/bridgepp/Test/TestSessionID.cpp b/internal/frontend/bridge-gui/bridgepp/Test/TestSessionID.cpp new file mode 100644 index 00000000..d1b95ae6 --- /dev/null +++ b/internal/frontend/bridge-gui/bridgepp/Test/TestSessionID.cpp @@ -0,0 +1,36 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#include "QtCore/qdatetime.h" +#include +#include + + +using namespace bridgepp; + + +TEST(SessionID, SessionID) { + QString const sessionID = newSessionID(); + EXPECT_TRUE(sessionID.size() > 0); + + EXPECT_FALSE(sessionIDToDateTime("invalidSessionID").isValid()); + + QDateTime const dateTime = sessionIDToDateTime(sessionID); + EXPECT_TRUE(dateTime.isValid()); + EXPECT_TRUE(qAbs(dateTime.secsTo(QDateTime::currentDateTime())) < 5); +} diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.cpp new file mode 100644 index 00000000..9af45e65 --- /dev/null +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.cpp @@ -0,0 +1,53 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#include "SessionID.h" +#include "QtCore/qdatetime.h" + + +namespace { + + +QString const dateTimeFormat = "yyyyMMdd_hhmmsszzz"; ///< The format string for date/time used by the sessionID. + + +} + + +namespace bridgepp { + + +//**************************************************************************************************************************************************** +/// \return a new session ID based on the current local date/time +//**************************************************************************************************************************************************** +QString newSessionID() { + return QDateTime::currentDateTime().toString(dateTimeFormat); +} + + +//**************************************************************************************************************************************************** +/// \param[in] sessionID The sessionID. +/// \return The date/time corresponding to the sessionID. +/// \return An invalid date/time if an error occurs. +//**************************************************************************************************************************************************** +QDateTime sessionIDToDateTime(QString const &sessionID) { + return QDateTime::fromString(sessionID, dateTimeFormat); +} + + +} // namespace diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.h new file mode 100644 index 00000000..7c223959 --- /dev/null +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/SessionID/SessionID.h @@ -0,0 +1,32 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#ifndef BRIDGE_PP_SESSION_ID_H +#define BRIDGE_PP_SESSION_ID_H + + +namespace bridgepp { + + +QString newSessionID(); ///< Create a new sessions +QDateTime sessionIDToDateTime(QString const &sessionID); ///< Parse the date/time from a sessionID. + + +} // namespace + +#endif //BRIDGE_PP_SESSION_ID_H diff --git a/internal/logging/clear.go b/internal/logging/clear.go index 7ae090f1..34e938dc 100644 --- a/internal/logging/clear.go +++ b/internal/logging/clear.go @@ -40,12 +40,12 @@ func clearLogs(logDir string, maxLogs int, maxCrashes int) error { // Remove old logs. removeOldLogs(logDir, xslices.Filter(names, func(name string) bool { - return MatchLogName(name) && !MatchStackTraceName(name) + return MatchBridgeLogName(name) && !MatchStackTraceName(name) }), maxLogs) // Remove old stack traces. removeOldLogs(logDir, xslices.Filter(names, func(name string) bool { - return MatchLogName(name) && MatchStackTraceName(name) + return MatchBridgeLogName(name) && MatchStackTraceName(name) }), maxCrashes) return nil @@ -58,7 +58,7 @@ func removeOldLogs(dir string, names []string, max int) { // Sort by timestamp, oldest first. slices.SortFunc(names, func(a, b string) bool { - return getLogTime(a) < getLogTime(b) + return getLogTime(a).Before(getLogTime(b)) }) for _, path := range xslices.Map(names[:len(names)-max], func(name string) string { return filepath.Join(dir, name) }) { diff --git a/internal/logging/crash.go b/internal/logging/crash.go index dca72751..40f2c32f 100644 --- a/internal/logging/crash.go +++ b/internal/logging/crash.go @@ -23,16 +23,15 @@ import ( "path/filepath" "regexp" "runtime/pprof" - "time" "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/ProtonMail/proton-bridge/v3/internal/crash" "github.com/sirupsen/logrus" ) -func DumpStackTrace(logsPath string) crash.RecoveryAction { +func DumpStackTrace(logsPath string, sessionID SessionID, appName string) crash.RecoveryAction { return func(r interface{}) error { - file := filepath.Join(logsPath, getStackTraceName(constants.Version, constants.Revision)) + file := filepath.Join(logsPath, getStackTraceName(sessionID, appName, constants.Version, constants.Tag)) f, err := os.OpenFile(filepath.Clean(file), os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) if err != nil { @@ -53,10 +52,10 @@ func DumpStackTrace(logsPath string) crash.RecoveryAction { } } -func getStackTraceName(version, revision string) string { - return fmt.Sprintf("v%v_%v_crash_%v.log", version, revision, time.Now().Unix()) +func getStackTraceName(sessionID SessionID, appName, version, tag string) string { + return fmt.Sprintf("%v_000_%v_v%v_%v_crash.log", sessionID, appName, version, tag) } func MatchStackTraceName(name string) bool { - return regexp.MustCompile(`^v.*_crash_.*\.log$`).MatchString(name) + return regexp.MustCompile(`^\d{8}_\d{9}_000_.*_crash\.log$`).MatchString(name) } diff --git a/internal/logging/crash_test.go b/internal/logging/crash_test.go new file mode 100644 index 00000000..dfc4ab0f --- /dev/null +++ b/internal/logging/crash_test.go @@ -0,0 +1,32 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +package logging + +import ( + "testing" + + "github.com/ProtonMail/proton-bridge/v3/internal/constants" + "github.com/stretchr/testify/require" +) + +func TestMatchStackTraceName(t *testing.T) { + filename := getStackTraceName(NewSessionID(), constants.AppName, constants.Version, constants.Tag) + require.True(t, len(filename) > 0) + require.True(t, MatchStackTraceName(filename)) + require.False(t, MatchStackTraceName("Invalid.log")) +} diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 483cf395..70046c5b 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -19,26 +19,21 @@ package logging import ( "context" - "fmt" - "io" "os" - "path/filepath" "regexp" - "strconv" "time" - "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/sirupsen/logrus" ) const ( // MaxLogSize defines the maximum log size we should permit: 5 MB // - // The Zendesk limit for an attachement is 50MB and this is what will + // The Zendesk limit for an attachment is 50MB and this is what will // be allowed via the API. However, if that fails for some reason, the // fallback is sending the report via email, which has a limit of 10mb // total or 7MB per file. Since we can produce up to 6 logs, and we - // compress all the files (avarage compression - 80%), we need to have + // compress all the files (average compression - 80%), we need to have // a limit of 30MB total before compression, hence 5MB per log file. MaxLogSize = 5 * 1024 * 1024 @@ -82,7 +77,7 @@ func (cs *coloredStdOutHook) Fire(entry *logrus.Entry) error { return nil } -func Init(logsPath, level string) error { +func Init(logsPath string, sessionID SessionID, appName, level string) error { logrus.SetFormatter(&logrus.TextFormatter{ DisableColors: true, FullTimestamp: true, @@ -91,13 +86,7 @@ func Init(logsPath, level string) error { logrus.AddHook(newColoredStdOutHook()) - rotator, err := NewRotator(MaxLogSize, func() (io.WriteCloser, error) { - if err := clearLogs(logsPath, MaxLogs, MaxLogs); err != nil { - return nil, err - } - - return os.Create(filepath.Join(logsPath, getLogName(constants.Version, constants.Revision))) //nolint:gosec // G304 - }) + rotator, err := NewDefaultRotator(logsPath, sessionID, appName, MaxLogSize) if err != nil { return err } @@ -137,34 +126,42 @@ func setLevel(level string) error { return nil } -func getLogName(version, revision string) string { - return fmt.Sprintf("v%v_%v_%v.log", version, revision, time.Now().Unix()) -} +func getLogTime(filename string) time.Time { + re := regexp.MustCompile(`^(?P\d{8}_\d{9})_.*\.log$`) -func getLogTime(name string) int { - re := regexp.MustCompile(`^v.*_.*_(?P\d+).log$`) - - match := re.FindStringSubmatch(name) + match := re.FindStringSubmatch(filename) if len(match) == 0 { - logrus.Warn("Could not parse log name: ", name) - return 0 + logrus.WithField("filename", filename).Warn("Could not parse log filename") + return time.Time{} } - timestamp, err := strconv.Atoi(match[re.SubexpIndex("timestamp")]) - if err != nil { - return 0 + index := re.SubexpIndex("sessionID") + if index < 0 { + logrus.WithField("filename", filename).Warn("Could not parse log filename") + return time.Time{} } - return timestamp + return SessionID(match[index]).toTime() } -func MatchLogName(name string) bool { - return regexp.MustCompile(`^v.*\.log$`).MatchString(name) +// MatchBridgeLogName return true iff filename is a bridge log filename. +func MatchBridgeLogName(filename string) bool { + return matchLogName(filename, "bridge") } -func MatchGUILogName(name string) bool { - return regexp.MustCompile(`^gui_v.*\.log$`).MatchString(name) +// MatchGUILogName return true iff filename is a bridge-gui log filename. +func MatchGUILogName(filename string) bool { + return matchLogName(filename, "gui") +} + +// MatchLauncherLogName return true iff filename is a launcher log filename. +func MatchLauncherLogName(filename string) bool { + return matchLogName(filename, "launcher") +} + +func matchLogName(logName, appName string) bool { + return regexp.MustCompile(`^\d{8}_\d{9}_\d{3}_` + appName + `.*\.log$`).MatchString(logName) } type logKey string @@ -180,12 +177,3 @@ func WithLogrusField(ctx context.Context, key string, value interface{}) context fields[key] = value return context.WithValue(ctx, logrusFields, fields) } - -func LogFromContext(ctx context.Context) *logrus.Entry { - fields, ok := ctx.Value(logrusFields).(logrus.Fields) - if !ok || fields == nil { - return logrus.WithField("ctx", "empty") - } - - return logrus.WithFields(fields) -} diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index 898234de..3351ea03 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -25,59 +25,95 @@ import ( "github.com/stretchr/testify/require" ) -// TestClearLogs tests that cearLogs removes only bridge old log files keeping last three of them. -func TestClearLogs(t *testing.T) { - dir := t.TempDir() - - // Create some old log files. - require.NoError(t, os.WriteFile(filepath.Join(dir, "other.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.7_debe87f2f5_0000000001.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.8_debe87f2f5_0000000002.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.9_debe87f2f5_0000000003.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.0_debe87f2f5_0000000004.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.1_debe87f2f5_0000000005.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.2_debe87f2f5_0000000006.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.3_debe87f2f5_0000000007.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.4_debe87f2f5_0000000008.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.5_debe87f2f5_0000000009.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.6_debe87f2f5_0000000010.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.7_debe87f2f5_0000000011.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.8_debe87f2f5_0000000012.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.12_debe87f2f5_0000000013.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.9_debe87f2f5_0000000014.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.10_debe87f2f5_0000000015.log"), []byte("Hello"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.11_debe87f2f5_0000000016.log"), []byte("Hello"), 0o755)) - - // Clear the logs. - require.NoError(t, clearLogs(dir, 3, 0)) - - // We should only clear matching files, and keep the 3 most recent ones. - checkFileNames(t, dir, []string{ - "other.log", - "v2.5.9_debe87f2f5_0000000014.log", - "v2.5.10_debe87f2f5_0000000015.log", - "v2.5.11_debe87f2f5_0000000016.log", - }) -} - -func checkFileNames(t *testing.T, dir string, expectedFileNames []string) { - require.ElementsMatch(t, expectedFileNames, getFileNames(t, dir)) -} - -func getFileNames(t *testing.T, dir string) []string { - files, err := os.ReadDir(dir) +func TestGetLogTime(t *testing.T) { + sessionID := NewSessionID() + fp := defaultFileProvider(os.TempDir(), sessionID, "bridge-test") + wc, err := fp(0) require.NoError(t, err) + file, ok := wc.(*os.File) + require.True(t, ok) - fileNames := []string{} - for _, file := range files { - fileNames = append(fileNames, file.Name()) - if file.IsDir() { - subDir := filepath.Join(dir, file.Name()) - subFileNames := getFileNames(t, subDir) - for _, subFileName := range subFileNames { - fileNames = append(fileNames, file.Name()+"/"+subFileName) - } - } - } - return fileNames + sessionIDTime := sessionID.toTime() + require.False(t, sessionIDTime.IsZero()) + logTime := getLogTime(filepath.Base(file.Name())) + require.False(t, logTime.IsZero()) + require.Equal(t, sessionIDTime, logTime) } + +func TestMatchLogName(t *testing.T) { + bridgeLog := "20230602_094633102_000_bridge_v3.0.99+git_5b650b1be3.log" + crashLog := "20230602_094633102_000_bridge_v3.0.99+git_5b650b1be3_crash.log" + guiLog := "20230602_094633102_000_gui_v3.0.99+git_5b650b1be3.log" + launcherLog := "20230602_094633102_000_launcher_v3.0.99+git_5b650b1be3.log" + require.True(t, MatchBridgeLogName(bridgeLog)) + require.False(t, MatchGUILogName(bridgeLog)) + require.False(t, MatchLauncherLogName(bridgeLog)) + require.True(t, MatchBridgeLogName(crashLog)) + require.False(t, MatchGUILogName(crashLog)) + require.False(t, MatchLauncherLogName(crashLog)) + require.False(t, MatchBridgeLogName(guiLog)) + require.True(t, MatchGUILogName(guiLog)) + require.False(t, MatchLauncherLogName(guiLog)) + require.False(t, MatchBridgeLogName(launcherLog)) + require.False(t, MatchGUILogName(launcherLog)) + require.True(t, MatchLauncherLogName(launcherLog)) +} + +// The test below is temporarily disabled, and will be restored when implementing new retention policy GODT-2668 + +// TestClearLogs tests that clearLogs removes only bridge old log files keeping last three of them. +// func TestClearLogs(t *testing.T) { +// dir := t.TempDir() +// +// // Create some old log files. +// require.NoError(t, os.WriteFile(filepath.Join(dir, "other.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.7_debe87f2f5_0000000001.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.8_debe87f2f5_0000000002.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.4.9_debe87f2f5_0000000003.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.0_debe87f2f5_0000000004.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.1_debe87f2f5_0000000005.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.2_debe87f2f5_0000000006.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.3_debe87f2f5_0000000007.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.4_debe87f2f5_0000000008.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.5_debe87f2f5_0000000009.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.6_debe87f2f5_0000000010.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.7_debe87f2f5_0000000011.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.8_debe87f2f5_0000000012.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.12_debe87f2f5_0000000013.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.9_debe87f2f5_0000000014.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.10_debe87f2f5_0000000015.log"), []byte("Hello"), 0o755)) +// require.NoError(t, os.WriteFile(filepath.Join(dir, "v2.5.11_debe87f2f5_0000000016.log"), []byte("Hello"), 0o755)) +// +// // Clear the logs. +// require.NoError(t, clearLogs(dir, 3, 0)) +// +// // We should only clear matching files, and keep the 3 most recent ones. +// checkFileNames(t, dir, []string{ +// "other.log", +// "v2.5.9_debe87f2f5_0000000014.log", +// "v2.5.10_debe87f2f5_0000000015.log", +// "v2.5.11_debe87f2f5_0000000016.log", +// }) +// } +// +// func checkFileNames(t *testing.T, dir string, expectedFileNames []string) { +// require.ElementsMatch(t, expectedFileNames, getFileNames(t, dir)) +// } +// +// func getFileNames(t *testing.T, dir string) []string { +// files, err := os.ReadDir(dir) +// require.NoError(t, err) +// +// fileNames := []string{} +// for _, file := range files { +// fileNames = append(fileNames, file.Name()) +// if file.IsDir() { +// subDir := filepath.Join(dir, file.Name()) +// subFileNames := getFileNames(t, subDir) +// for _, subFileName := range subFileNames { +// fileNames = append(fileNames, file.Name()+"/"+subFileName) +// } +// } +// } +// return fileNames +// } diff --git a/internal/logging/rotator.go b/internal/logging/rotator.go index 89b39375..089cc4d5 100644 --- a/internal/logging/rotator.go +++ b/internal/logging/rotator.go @@ -17,16 +17,36 @@ package logging -import "io" +import ( + "fmt" + "io" + "os" + "path/filepath" + + "github.com/ProtonMail/proton-bridge/v3/internal/constants" +) type Rotator struct { - getFile FileProvider - wc io.WriteCloser - size int - maxSize int + getFile FileProvider + wc io.WriteCloser + size int + maxSize int + nextIndex int } -type FileProvider func() (io.WriteCloser, error) +type FileProvider func(index int) (io.WriteCloser, error) + +func defaultFileProvider(logsPath string, sessionID SessionID, appName string) FileProvider { + return func(index int) (io.WriteCloser, error) { + if err := clearLogs(logsPath, MaxLogs, MaxLogs); err != nil { + return nil, err + } + + return os.Create(filepath.Join(logsPath, //nolint:gosec // G304 + fmt.Sprintf("%v_%03d_%v_v%v_%v.log", sessionID, index, appName, constants.Version, constants.Tag), + )) + } +} func NewRotator(maxSize int, getFile FileProvider) (*Rotator, error) { r := &Rotator{ @@ -41,6 +61,10 @@ func NewRotator(maxSize int, getFile FileProvider) (*Rotator, error) { return r, nil } +func NewDefaultRotator(logsPath string, sessionID SessionID, appName string, maxSize int) (*Rotator, error) { + return NewRotator(maxSize, defaultFileProvider(logsPath, sessionID, appName)) +} + func (r *Rotator) Write(p []byte) (int, error) { if r.size+len(p) > r.maxSize { if err := r.rotate(); err != nil { @@ -63,11 +87,12 @@ func (r *Rotator) rotate() error { _ = r.wc.Close() } - wc, err := r.getFile() + wc, err := r.getFile(r.nextIndex) if err != nil { return err } + r.nextIndex++ r.wc = wc r.size = 0 diff --git a/internal/logging/rotator_test.go b/internal/logging/rotator_test.go index 5eecb053..df1ee96a 100644 --- a/internal/logging/rotator_test.go +++ b/internal/logging/rotator_test.go @@ -19,8 +19,10 @@ package logging import ( "bytes" + "fmt" "io" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -38,7 +40,7 @@ func (c *WriteCloser) Close() error { func TestRotator(t *testing.T) { n := 0 - getFile := func() (io.WriteCloser, error) { + getFile := func(_ int) (io.WriteCloser, error) { n++ return &WriteCloser{}, nil } @@ -75,11 +77,70 @@ func TestRotator(t *testing.T) { assert.Equal(t, 4, n) } +func countFilesMatching(pattern string) int { + files, err := filepath.Glob(pattern) + if err != nil { + return -1 + } + + return len(files) +} + +func cleanupLogs(t *testing.T, sessionID SessionID) { + paths, err := filepath.Glob(filepath.Join(os.TempDir(), string(sessionID)+"*.log")) + require.NoError(t, err) + for _, path := range paths { + require.NoError(t, os.Remove(path)) + } +} + +func TestDefaultRotator(t *testing.T) { + fiveBytes := []byte("00000") + tmpDir := os.TempDir() + + sessionID := NewSessionID() + basePath := filepath.Join(tmpDir, string(sessionID)) + + r, err := NewDefaultRotator(tmpDir, sessionID, "bridge", 10) + require.NoError(t, err) + require.Equal(t, 1, countFilesMatching(basePath+"_000_*.log")) + require.Equal(t, 1, countFilesMatching(basePath+"*.log")) + + _, err = r.Write(fiveBytes) + require.NoError(t, err) + require.Equal(t, 1, countFilesMatching(basePath+"*.log")) + + _, err = r.Write(fiveBytes) + require.NoError(t, err) + require.Equal(t, 1, countFilesMatching(basePath+"*.log")) + + _, err = r.Write(fiveBytes) + require.NoError(t, err) + require.Equal(t, 2, countFilesMatching(basePath+"*.log")) + require.Equal(t, 1, countFilesMatching(basePath+"_001_*.log")) + + for i := 0; i < 4; i++ { + _, err = r.Write(fiveBytes) + require.NoError(t, err) + } + + require.NoError(t, r.wc.Close()) + + // total written: 35 bytes, i.e. 4 log files + logFileCount := countFilesMatching(basePath + "*.log") + require.Equal(t, 4, logFileCount) + for i := 0; i < logFileCount; i++ { + require.Equal(t, 1, countFilesMatching(basePath+fmt.Sprintf("_%03d_*.log", i))) + } + + cleanupLogs(t, sessionID) +} + func BenchmarkRotate(b *testing.B) { benchRotate(b, MaxLogSize, getTestFile(b, b.TempDir(), MaxLogSize-1)) } -func benchRotate(b *testing.B, logSize int, getFile func() (io.WriteCloser, error)) { +func benchRotate(b *testing.B, logSize int, getFile func(index int) (io.WriteCloser, error)) { r, err := NewRotator(logSize, getFile) require.NoError(b, err) @@ -92,8 +153,8 @@ func benchRotate(b *testing.B, logSize int, getFile func() (io.WriteCloser, erro } } -func getTestFile(b *testing.B, dir string, length int) func() (io.WriteCloser, error) { - return func() (io.WriteCloser, error) { +func getTestFile(b *testing.B, dir string, length int) func(int) (io.WriteCloser, error) { + return func(index int) (io.WriteCloser, error) { b.StopTimer() defer b.StartTimer() diff --git a/internal/logging/session_id.go b/internal/logging/session_id.go new file mode 100644 index 00000000..21ab3f2d --- /dev/null +++ b/internal/logging/session_id.go @@ -0,0 +1,64 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License + +package logging + +import ( + "fmt" + "strconv" + "time" +) + +type SessionID string + +const ( + timeFormat = "20060102_150405" // time format in Go does not support milliseconds without dot, so we'll process it manually. +) + +// NewSessionID creates a sessionID based on the current time. +func NewSessionID() SessionID { + now := time.Now() + return SessionID(now.Format(timeFormat) + fmt.Sprintf("%03d", now.Nanosecond()/1000000)) +} + +// NewSessionIDFromString Return a new sessionID from string. If the str is empty a new time based sessionID is returned, otherwise the string +// is used as the sessionID. +func NewSessionIDFromString(str string) SessionID { + if (len(str)) > 0 { + return SessionID(str) + } + + return NewSessionID() +} + +// toTime converts a sessionID to a date/Time, considering the time zone is local. +func (s SessionID) toTime() time.Time { + if len(s) < 3 { + return time.Time{} + } + + t, err := time.ParseInLocation(timeFormat, string(s)[:len(s)-3], time.Local) + if err != nil { + return time.Time{} + } + + var ms int + if ms, err = strconv.Atoi(string(s)[len(s)-3:]); err != nil { + return time.Time{} + } + + return t.Add(time.Duration(ms) * time.Millisecond) +} diff --git a/internal/logging/session_id_test.go b/internal/logging/session_id_test.go new file mode 100644 index 00000000..53c3092b --- /dev/null +++ b/internal/logging/session_id_test.go @@ -0,0 +1,38 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License + +package logging + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestSessionID(t *testing.T) { + now := time.Now() + sessionID := NewSessionID() + sessionTime := sessionID.toTime() + require.False(t, sessionTime.IsZero()) + require.WithinRange(t, sessionTime, now.Add(-1*time.Millisecond), now.Add(1*time.Millisecond)) + + fromString := NewSessionIDFromString("") + require.True(t, len(fromString) > 0) + fromString = NewSessionIDFromString(string(sessionID)) + require.True(t, len(fromString) > 0) + require.Equal(t, sessionID, fromString) +} diff --git a/pkg/restarter/restarter.go b/pkg/restarter/restarter.go index a8908239..5bd3db1e 100644 --- a/pkg/restarter/restarter.go +++ b/pkg/restarter/restarter.go @@ -77,7 +77,7 @@ func (restarter *Restarter) Restart() { //nolint:gosec cmd := execabs.Command( restarter.exe, - xslices.Join(removeFlagWithValue(removeFlag(os.Args[1:], "no-window"), "parent-pid"), restarter.flags)..., + xslices.Join(removeFlagsWithValue(removeFlag(os.Args[1:], "no-window"), "parent-pid", "session-id"), restarter.flags)..., ) l := logrus.WithFields(logrus.Fields{ @@ -157,6 +157,16 @@ func removeFlagWithValue(argList []string, flag string) []string { return result } +// removeFlagWithValue removes flags that require a value from a list of command line parameters. +// The flags can be of the following form `-flag value`, `--flag value`, `-flag=value` or `--flags=value`. +func removeFlagsWithValue(argList []string, flags ...string) []string { + for _, flag := range flags { + argList = removeFlagWithValue(argList, flag) + } + + return argList +} + func removeFlag(argList []string, flag string) []string { return xslices.Filter(argList, func(arg string) bool { return (arg != "-"+flag) && (arg != "--"+flag) }) } diff --git a/pkg/restarter/restarter_test.go b/pkg/restarter/restarter_test.go index 92c966aa..62be7f25 100644 --- a/pkg/restarter/restarter_test.go +++ b/pkg/restarter/restarter_test.go @@ -42,6 +42,23 @@ func TestRemoveFlagWithValue(t *testing.T) { } } +func TestRemoveFlagsWithValue(t *testing.T) { + tests := []struct { + argList []string + flags []string + expected []string + }{ + {[]string{}, []string{"a", "b"}, nil}, + {[]string{"-a", "-b=value", "-c"}, []string{"b"}, []string{"-a", "-c"}}, + {[]string{"-a", "--b=value", "-c"}, []string{"b", "c"}, []string{"-a"}}, + {[]string{"-a", "-b", "value", "-c"}, []string{"c", "b"}, []string{"-a"}}, + } + + for _, tt := range tests { + require.Equal(t, removeFlagsWithValue(tt.argList, tt.flags...), tt.expected) + } +} + func TestRemoveFlag(t *testing.T) { tests := []struct { argList []string