fix(cef): address PR review findings
- commit
- 2921b0be637c42b0db0c51b80c8931476c37633d
- author
- brice · · 2026-04-21 06:05 UTC
- parent
- c1b9511
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +70 -110
M cmd/dumber/main.go +20 -21
| @@ -120,10 +120,10 @@ func configureBrowserLaunchRelay(cfg *config.Config) { | |||
| 120 | 120 | func main() { | |
| 121 | 121 | // CEF subprocess handling: when CEF re-launches this binary with | |
| 122 | 122 | // --type=renderer/gpu/etc, we must call ExecuteProcess before anything | |
| 123 | - | // else (Cobra, config, arg stripping). We detect subprocesses by the | |
| 124 | - | // presence of a --type flag in os.Args rather than checking the engine | |
| 125 | - | // env var, because subprocesses may not inherit the environment and the | |
| 126 | - | // engine can also be selected via config. | |
| 123 | + | // else (Cobra, config, arg stripping). We only treat a leading Chromium | |
| 124 | + | // helper-process --type flag as a subprocess marker rather than scanning | |
| 125 | + | // the full argv, so normal user commands like `dumber browse --type=foo` | |
| 126 | + | // are not misclassified. | |
| 127 | 127 | if isCEFSubprocess(os.Args) { | |
| 128 | 128 | runtime.LockOSThread() | |
| 129 | 129 | executed, exitCode, err := cef.ExecuteSubprocessWithApp(infracef.NewSubprocessApp()) | |
| @@ -134,6 +134,7 @@ func main() { | |||
| 134 | 134 | if executed { | |
| 135 | 135 | os.Exit(exitCode) | |
| 136 | 136 | } | |
| 137 | + | runtime.UnlockOSThread() | |
| 137 | 138 | // If Chromium-style helper args were present but purego-cef declined to | |
| 138 | 139 | // handle them as a subprocess, drop them before normal startup so Cobra | |
| 139 | 140 | // and the GUI bootstrap do not choke on unknown --type/... flags. | |
| @@ -739,25 +740,23 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |||
| 739 | 740 | } | |
| 740 | 741 | } | |
| 741 | 742 | ||
| 742 | - | // isCEFSubprocess returns true if args contains a --type flag, indicating | |
| 743 | - | // this process was spawned by CEF as a renderer, GPU, or utility subprocess. | |
| 743 | + | // isCEFSubprocess returns true when argv begins with a Chromium helper-process | |
| 744 | + | // --type flag, indicating this process was spawned by CEF as a renderer, GPU, | |
| 745 | + | // or utility subprocess. | |
| 744 | 746 | func isCEFSubprocess(args []string) bool { | |
| 745 | - | for i, arg := range args { | |
| 746 | - | if strings.HasPrefix(arg, "--type=") { | |
| 747 | - | value := strings.TrimPrefix(arg, "--type=") | |
| 748 | - | if value != "" && !strings.HasPrefix(value, "-") { | |
| 749 | - | return true | |
| 750 | - | } | |
| 751 | - | continue | |
| 752 | - | } | |
| 753 | - | if arg == "--type" && i+1 < len(args) { | |
| 754 | - | next := args[i+1] | |
| 755 | - | if next != "" && !strings.HasPrefix(next, "-") { | |
| 756 | - | return true | |
| 757 | - | } | |
| 758 | - | } | |
| 747 | + | if len(args) < 2 { | |
| 748 | + | return false | |
| 759 | 749 | } | |
| 760 | - | return false | |
| 750 | + | ||
| 751 | + | if strings.HasPrefix(args[1], "--type=") { | |
| 752 | + | value := strings.TrimPrefix(args[1], "--type=") | |
| 753 | + | return value != "" && !strings.HasPrefix(value, "-") | |
| 754 | + | } | |
| 755 | + | ||
| 756 | + | return args[1] == "--type" && | |
| 757 | + | len(args) > 2 && | |
| 758 | + | args[2] != "" && | |
| 759 | + | !strings.HasPrefix(args[2], "-") | |
| 761 | 760 | } | |
| 762 | 761 | ||
| 763 | 762 | func buildUIDependencies( | |
| @@ -120,10 +120,10 @@ func configureBrowserLaunchRelay(cfg *config.Config) { | |||||
| 120 | func main() { | 120 | func main() { | ||
| 121 | // CEF subprocess handling: when CEF re-launches this binary with | 121 | // CEF subprocess handling: when CEF re-launches this binary with | ||
| 122 | // --type=renderer/gpu/etc, we must call ExecuteProcess before anything | 122 | // --type=renderer/gpu/etc, we must call ExecuteProcess before anything | ||
| 123 | - | // else (Cobra, config, arg stripping). We detect subprocesses by the | 123 | + | // else (Cobra, config, arg stripping). We only treat a leading Chromium |
| 124 | - | // presence of a --type flag in os.Args rather than checking the engine | 124 | + | // helper-process --type flag as a subprocess marker rather than scanning |
| 125 | - | // env var, because subprocesses may not inherit the environment and the | 125 | + | // the full argv, so normal user commands like `dumber browse --type=foo` |
| 126 | - | // engine can also be selected via config. | 126 | + | // are not misclassified. |
| 127 | if isCEFSubprocess(os.Args) { | 127 | if isCEFSubprocess(os.Args) { | ||
| 128 | runtime.LockOSThread() | 128 | runtime.LockOSThread() | ||
| 129 | executed, exitCode, err := cef.ExecuteSubprocessWithApp(infracef.NewSubprocessApp()) | 129 | executed, exitCode, err := cef.ExecuteSubprocessWithApp(infracef.NewSubprocessApp()) | ||
| @@ -134,6 +134,7 @@ func main() { | |||||
| 134 | if executed { | 134 | if executed { | ||
| 135 | os.Exit(exitCode) | 135 | os.Exit(exitCode) | ||
| 136 | } | 136 | } | ||
| 137 | + | runtime.UnlockOSThread() | |||
| 137 | // If Chromium-style helper args were present but purego-cef declined to | 138 | // If Chromium-style helper args were present but purego-cef declined to | ||
| 138 | // handle them as a subprocess, drop them before normal startup so Cobra | 139 | // handle them as a subprocess, drop them before normal startup so Cobra | ||
| 139 | // and the GUI bootstrap do not choke on unknown --type/... flags. | 140 | // and the GUI bootstrap do not choke on unknown --type/... flags. | ||
| @@ -739,25 +740,23 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |||||
| 739 | } | 740 | } | ||
| 740 | } | 741 | } | ||
| 741 | 742 | ||||
| 742 | - | // isCEFSubprocess returns true if args contains a --type flag, indicating | 743 | + | // isCEFSubprocess returns true when argv begins with a Chromium helper-process |
| 743 | - | // this process was spawned by CEF as a renderer, GPU, or utility subprocess. | 744 | + | // --type flag, indicating this process was spawned by CEF as a renderer, GPU, |
| 745 | + | // or utility subprocess. | |||
| 744 | func isCEFSubprocess(args []string) bool { | 746 | func isCEFSubprocess(args []string) bool { | ||
| 745 | - | for i, arg := range args { | 747 | + | if len(args) < 2 { |
| 746 | - | if strings.HasPrefix(arg, "--type=") { | 748 | + | return false |
| 747 | - | value := strings.TrimPrefix(arg, "--type=") | |||
| 748 | - | if value != "" && !strings.HasPrefix(value, "-") { | |||
| 749 | - | return true | |||
| 750 | - | } | |||
| 751 | - | continue | |||
| 752 | - | } | |||
| 753 | - | if arg == "--type" && i+1 < len(args) { | |||
| 754 | - | next := args[i+1] | |||
| 755 | - | if next != "" && !strings.HasPrefix(next, "-") { | |||
| 756 | - | return true | |||
| 757 | - | } | |||
| 758 | - | } | |||
| 759 | } | 749 | } | ||
| 760 | - | return false | 750 | + | |
| 751 | + | if strings.HasPrefix(args[1], "--type=") { | |||
| 752 | + | value := strings.TrimPrefix(args[1], "--type=") | |||
| 753 | + | return value != "" && !strings.HasPrefix(value, "-") | |||
| 754 | + | } | |||
| 755 | + | ||||
| 756 | + | return args[1] == "--type" && | |||
| 757 | + | len(args) > 2 && | |||
| 758 | + | args[2] != "" && | |||
| 759 | + | !strings.HasPrefix(args[2], "-") | |||
| 761 | } | 760 | } | ||
| 762 | 761 | ||||
| 763 | func buildUIDependencies( | 762 | func buildUIDependencies( | ||
M cmd/dumber/main_test.go +15 -0
| @@ -108,6 +108,21 @@ func TestIsCEFSubprocess(t *testing.T) { | |||
| 108 | 108 | args: []string{"dumber", "--unexpected"}, | |
| 109 | 109 | want: false, | |
| 110 | 110 | }, | |
| 111 | + | { | |
| 112 | + | name: "type flag after user command", | |
| 113 | + | args: []string{"dumber", "browse", "--type=renderer"}, | |
| 114 | + | want: false, | |
| 115 | + | }, | |
| 116 | + | { | |
| 117 | + | name: "type flag not in second argv slot", | |
| 118 | + | args: []string{"dumber", "browse", "https://example.com", "--type", "renderer"}, | |
| 119 | + | want: false, | |
| 120 | + | }, | |
| 121 | + | { | |
| 122 | + | name: "equals form with dash-prefixed value", | |
| 123 | + | args: []string{"dumber", "--type=--renderer"}, | |
| 124 | + | want: false, | |
| 125 | + | }, | |
| 111 | 126 | } | |
| 112 | 127 | ||
| 113 | 128 | for _, tc := range tests { | |
| @@ -108,6 +108,21 @@ func TestIsCEFSubprocess(t *testing.T) { | |||||
| 108 | args: []string{"dumber", "--unexpected"}, | 108 | args: []string{"dumber", "--unexpected"}, | ||
| 109 | want: false, | 109 | want: false, | ||
| 110 | }, | 110 | }, | ||
| 111 | + | { | |||
| 112 | + | name: "type flag after user command", | |||
| 113 | + | args: []string{"dumber", "browse", "--type=renderer"}, | |||
| 114 | + | want: false, | |||
| 115 | + | }, | |||
| 116 | + | { | |||
| 117 | + | name: "type flag not in second argv slot", | |||
| 118 | + | args: []string{"dumber", "browse", "https://example.com", "--type", "renderer"}, | |||
| 119 | + | want: false, | |||
| 120 | + | }, | |||
| 121 | + | { | |||
| 122 | + | name: "equals form with dash-prefixed value", | |||
| 123 | + | args: []string{"dumber", "--type=--renderer"}, | |||
| 124 | + | want: false, | |||
| 125 | + | }, | |||
| 111 | } | 126 | } | ||
| 112 | 127 | ||||
| 113 | for _, tc := range tests { | 128 | for _, tc := range tests { | ||
M internal/infrastructure/cef/engine_init.go +9 -29
| @@ -141,13 +141,12 @@ func prepareCEFSettings( | |||
| 141 | 141 | } else { | |
| 142 | 142 | settings.LogFile = runtimeLogFile | |
| 143 | 143 | } | |
| 144 | - | if bootstrapLogFile, err := prepareCEFInitTraceFile(paths.LogFile, cfg.LogFile); err != nil { | |
| 145 | - | logger.Warn().Err(err).Msg("cef: failed to prepare init trace file") | |
| 146 | - | } else if bootstrapLogFile != "" { | |
| 147 | - | logger.Warn(). | |
| 148 | - | Str("bootstrap_log_file", bootstrapLogFile). | |
| 149 | - | Str("env_var", puregoCEFInitTraceEnvVar). | |
| 150 | - | Msg("cef: init bootstrap diagnostics requested, but current purego-cef no longer consumes this trace file") | |
| 144 | + | if puregoCEFInitTraceEnabled() { | |
| 145 | + | warn := logger.Warn().Str("env_var", puregoCEFInitTraceEnvVar) | |
| 146 | + | if bootstrapLogFile := resolveCEFInitTraceFile(paths.LogFile, cfg.LogFile); bootstrapLogFile != "" { | |
| 147 | + | warn = warn.Str("bootstrap_log_file", bootstrapLogFile) | |
| 148 | + | } | |
| 149 | + | warn.Msg("cef: init bootstrap diagnostics requested, but current purego-cef no longer consumes this trace file") | |
| 151 | 150 | } | |
| 152 | 151 | if cfg.LogSeverity != 0 { | |
| 153 | 152 | settings.LogSeverity = cfg.LogSeverity | |
| @@ -472,31 +471,12 @@ func openFileNoFollow(path string, flags int, perm os.FileMode) (*os.File, error | |||
| 472 | 471 | return os.NewFile(uintptr(fd), path), nil | |
| 473 | 472 | } | |
| 474 | 473 | ||
| 475 | - | func prepareCEFInitTraceFile(defaultLogFile, logFile string) (string, error) { | |
| 476 | - | if !puregoCEFInitTraceEnabled() { | |
| 477 | - | return "", nil | |
| 478 | - | } | |
| 479 | - | ||
| 474 | + | func resolveCEFInitTraceFile(defaultLogFile, logFile string) string { | |
| 480 | 475 | runtimeLogFile := resolveLogFile(defaultLogFile, logFile) | |
| 481 | 476 | if runtimeLogFile == "" { | |
| 482 | - | return "", nil | |
| 483 | - | } | |
| 484 | - | bootstrapLogFile := strings.TrimSuffix(runtimeLogFile, filepath.Ext(runtimeLogFile)) + ".bootstrap.log" | |
| 485 | - | if err := os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm); err != nil { | |
| 486 | - | return "", fmt.Errorf("mkdir %s: %w", filepath.Dir(bootstrapLogFile), err) | |
| 487 | - | } | |
| 488 | - | file, err := openRegularLogFile(bootstrapLogFile) | |
| 489 | - | if err != nil { | |
| 490 | - | return "", fmt.Errorf("reset %s: %w", bootstrapLogFile, err) | |
| 491 | - | } | |
| 492 | - | defer func() { _ = file.Close() }() | |
| 493 | - | if err := file.Truncate(0); err != nil { | |
| 494 | - | return "", fmt.Errorf("reset %s: %w", bootstrapLogFile, err) | |
| 495 | - | } | |
| 496 | - | if err := file.Chmod(filePerm); err != nil { | |
| 497 | - | return "", fmt.Errorf("chmod %s: %w", bootstrapLogFile, err) | |
| 477 | + | return "" | |
| 498 | 478 | } | |
| 499 | - | return bootstrapLogFile, nil | |
| 479 | + | return strings.TrimSuffix(runtimeLogFile, filepath.Ext(runtimeLogFile)) + ".bootstrap.log" | |
| 500 | 480 | } | |
| 501 | 481 | ||
| 502 | 482 | func resolveLogFile(defaultLogFile, logFile string) string { | |
| @@ -141,13 +141,12 @@ func prepareCEFSettings( | |||||
| 141 | } else { | 141 | } else { | ||
| 142 | settings.LogFile = runtimeLogFile | 142 | settings.LogFile = runtimeLogFile | ||
| 143 | } | 143 | } | ||
| 144 | - | if bootstrapLogFile, err := prepareCEFInitTraceFile(paths.LogFile, cfg.LogFile); err != nil { | 144 | + | if puregoCEFInitTraceEnabled() { |
| 145 | - | logger.Warn().Err(err).Msg("cef: failed to prepare init trace file") | 145 | + | warn := logger.Warn().Str("env_var", puregoCEFInitTraceEnvVar) |
| 146 | - | } else if bootstrapLogFile != "" { | 146 | + | if bootstrapLogFile := resolveCEFInitTraceFile(paths.LogFile, cfg.LogFile); bootstrapLogFile != "" { |
| 147 | - | logger.Warn(). | 147 | + | warn = warn.Str("bootstrap_log_file", bootstrapLogFile) |
| 148 | - | Str("bootstrap_log_file", bootstrapLogFile). | 148 | + | } |
| 149 | - | Str("env_var", puregoCEFInitTraceEnvVar). | 149 | + | warn.Msg("cef: init bootstrap diagnostics requested, but current purego-cef no longer consumes this trace file") |
| 150 | - | Msg("cef: init bootstrap diagnostics requested, but current purego-cef no longer consumes this trace file") | |||
| 151 | } | 150 | } | ||
| 152 | if cfg.LogSeverity != 0 { | 151 | if cfg.LogSeverity != 0 { | ||
| 153 | settings.LogSeverity = cfg.LogSeverity | 152 | settings.LogSeverity = cfg.LogSeverity | ||
| @@ -472,31 +471,12 @@ func openFileNoFollow(path string, flags int, perm os.FileMode) (*os.File, error | |||||
| 472 | return os.NewFile(uintptr(fd), path), nil | 471 | return os.NewFile(uintptr(fd), path), nil | ||
| 473 | } | 472 | } | ||
| 474 | 473 | ||||
| 475 | - | func prepareCEFInitTraceFile(defaultLogFile, logFile string) (string, error) { | 474 | + | func resolveCEFInitTraceFile(defaultLogFile, logFile string) string { |
| 476 | - | if !puregoCEFInitTraceEnabled() { | |||
| 477 | - | return "", nil | |||
| 478 | - | } | |||
| 479 | - | ||||
| 480 | runtimeLogFile := resolveLogFile(defaultLogFile, logFile) | 475 | runtimeLogFile := resolveLogFile(defaultLogFile, logFile) | ||
| 481 | if runtimeLogFile == "" { | 476 | if runtimeLogFile == "" { | ||
| 482 | - | return "", nil | 477 | + | return "" |
| 483 | - | } | |||
| 484 | - | bootstrapLogFile := strings.TrimSuffix(runtimeLogFile, filepath.Ext(runtimeLogFile)) + ".bootstrap.log" | |||
| 485 | - | if err := os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm); err != nil { | |||
| 486 | - | return "", fmt.Errorf("mkdir %s: %w", filepath.Dir(bootstrapLogFile), err) | |||
| 487 | - | } | |||
| 488 | - | file, err := openRegularLogFile(bootstrapLogFile) | |||
| 489 | - | if err != nil { | |||
| 490 | - | return "", fmt.Errorf("reset %s: %w", bootstrapLogFile, err) | |||
| 491 | - | } | |||
| 492 | - | defer func() { _ = file.Close() }() | |||
| 493 | - | if err := file.Truncate(0); err != nil { | |||
| 494 | - | return "", fmt.Errorf("reset %s: %w", bootstrapLogFile, err) | |||
| 495 | - | } | |||
| 496 | - | if err := file.Chmod(filePerm); err != nil { | |||
| 497 | - | return "", fmt.Errorf("chmod %s: %w", bootstrapLogFile, err) | |||
| 498 | } | 478 | } | ||
| 499 | - | return bootstrapLogFile, nil | 479 | + | return strings.TrimSuffix(runtimeLogFile, filepath.Ext(runtimeLogFile)) + ".bootstrap.log" |
| 500 | } | 480 | } | ||
| 501 | 481 | ||||
| 502 | func resolveLogFile(defaultLogFile, logFile string) string { | 482 | func resolveLogFile(defaultLogFile, logFile string) string { | ||
M internal/infrastructure/cef/engine_init_test.go +26 -60
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package cef | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "os" | |
| 5 | 6 | "path/filepath" | |
| 6 | 7 | "testing" | |
| @@ -59,13 +60,8 @@ func TestPrepareCEFSettings_AllowsNoThirdPartyCookiePolicy(t *testing.T) { | |||
| 59 | 60 | require.NoError(t, err) | |
| 60 | 61 | } | |
| 61 | 62 | ||
| 62 | - | func TestPrepareCEFInitTraceFile_DisabledByDefault(t *testing.T) { | |
| 63 | - | t.Setenv(puregoCEFInitTraceEnvVar, "") | |
| 64 | - | profile := testCEFDevProfile(t) | |
| 65 | - | ||
| 66 | - | path, err := prepareCEFInitTraceFile(profile.CEFLogFile(), "") | |
| 67 | - | require.NoError(t, err) | |
| 68 | - | require.Empty(t, path) | |
| 63 | + | func TestResolveCEFInitTraceFile_EmptyWithoutLogPath(t *testing.T) { | |
| 64 | + | require.Empty(t, resolveCEFInitTraceFile("", "")) | |
| 69 | 65 | } | |
| 70 | 66 | ||
| 71 | 67 | func TestPrepareCEFLogFile_CreatesPrivateDirectoryAndFile(t *testing.T) { | |
| @@ -124,71 +120,41 @@ func TestPrepareCEFLogFile_RejectsDirectoryPath(t *testing.T) { | |||
| 124 | 120 | require.Empty(t, path) | |
| 125 | 121 | } | |
| 126 | 122 | ||
| 127 | - | func TestPrepareCEFInitTraceFile_EnabledViaExplicitLogFile(t *testing.T) { | |
| 128 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |
| 123 | + | func TestResolveCEFInitTraceFile_UsesExplicitLogFile(t *testing.T) { | |
| 129 | 124 | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | |
| 130 | 125 | ||
| 131 | - | path, err := prepareCEFInitTraceFile("", logFile) | |
| 132 | - | require.NoError(t, err) | |
| 126 | + | path := resolveCEFInitTraceFile("", logFile) | |
| 133 | 127 | require.Equal(t, filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log"), path) | |
| 134 | - | require.FileExists(t, path) | |
| 135 | - | ||
| 136 | - | info, err := os.Stat(path) | |
| 137 | - | require.NoError(t, err) | |
| 138 | - | require.Equal(t, os.FileMode(filePerm), info.Mode().Perm()) | |
| 139 | - | ||
| 140 | - | dirInfo, err := os.Stat(filepath.Dir(path)) | |
| 141 | - | require.NoError(t, err) | |
| 142 | - | require.Equal(t, os.FileMode(dirPerm), dirInfo.Mode().Perm()) | |
| 143 | - | } | |
| 144 | - | ||
| 145 | - | func TestPrepareCEFInitTraceFile_TruncatesExistingFile(t *testing.T) { | |
| 146 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |
| 147 | - | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | |
| 148 | - | bootstrapLogFile := filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log") | |
| 149 | - | require.NoError(t, os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm)) | |
| 150 | - | require.NoError(t, os.WriteFile(bootstrapLogFile, []byte("existing"), 0o644)) | |
| 151 | - | ||
| 152 | - | path, err := prepareCEFInitTraceFile("", logFile) | |
| 153 | - | require.NoError(t, err) | |
| 154 | - | require.Equal(t, bootstrapLogFile, path) | |
| 155 | - | ||
| 156 | - | content, readErr := os.ReadFile(path) | |
| 157 | - | require.NoError(t, readErr) | |
| 158 | - | require.Empty(t, content) | |
| 159 | - | ||
| 160 | - | info, statErr := os.Stat(path) | |
| 161 | - | require.NoError(t, statErr) | |
| 162 | - | require.Equal(t, os.FileMode(filePerm), info.Mode().Perm()) | |
| 163 | 128 | } | |
| 164 | 129 | ||
| 165 | - | func TestPrepareCEFInitTraceFile_RejectsSymlink(t *testing.T) { | |
| 166 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |
| 167 | - | target := filepath.Join(t.TempDir(), "target.bootstrap.log") | |
| 168 | - | require.NoError(t, os.WriteFile(target, []byte("existing"), 0o644)) | |
| 169 | - | ||
| 170 | - | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | |
| 171 | - | bootstrapLogFile := filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log") | |
| 172 | - | require.NoError(t, os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm)) | |
| 173 | - | require.NoError(t, os.Symlink(target, bootstrapLogFile)) | |
| 174 | - | ||
| 175 | - | path, err := prepareCEFInitTraceFile("", logFile) | |
| 176 | - | require.ErrorContains(t, err, "must not be a symlink") | |
| 177 | - | require.Empty(t, path) | |
| 130 | + | func TestResolveCEFInitTraceFile_UsesResolvedProfileLogDirByDefault(t *testing.T) { | |
| 131 | + | profile := testCEFDevProfile(t) | |
| 178 | 132 | ||
| 179 | - | targetInfo, statErr := os.Stat(target) | |
| 180 | - | require.NoError(t, statErr) | |
| 181 | - | require.Equal(t, os.FileMode(0o644), targetInfo.Mode().Perm()) | |
| 133 | + | path := resolveCEFInitTraceFile(profile.CEFLogFile(), "") | |
| 134 | + | require.Equal(t, filepath.Join(profile.EnginePaths.LogDir, "cef_runtime.bootstrap.log"), path) | |
| 182 | 135 | } | |
| 183 | 136 | ||
| 184 | - | func TestPrepareCEFInitTraceFile_UsesResolvedProfileLogDirByDefault(t *testing.T) { | |
| 137 | + | func TestPrepareCEFSettings_IgnoresInitTraceRequestWithoutTouchingFilesystem(t *testing.T) { | |
| 185 | 138 | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |
| 186 | 139 | profile := testCEFDevProfile(t) | |
| 140 | + | bootstrapLogFile := resolveCEFInitTraceFile(profile.CEFLogFile(), "") | |
| 187 | 141 | ||
| 188 | - | path, err := prepareCEFInitTraceFile(profile.CEFLogFile(), "") | |
| 142 | + | var logs bytes.Buffer | |
| 143 | + | logger := zerolog.New(&logs) | |
| 144 | + | ||
| 145 | + | settings, err := prepareCEFSettings( | |
| 146 | + | port.EngineOptions{}, | |
| 147 | + | RuntimePaths{StateRoot: profile.CEFUserDataDir(), LogFile: profile.CEFLogFile()}, | |
| 148 | + | RuntimeConfig{}, | |
| 149 | + | &logger, | |
| 150 | + | ) | |
| 189 | 151 | require.NoError(t, err) | |
| 190 | - | require.Equal(t, filepath.Join(profile.EnginePaths.LogDir, "cef_runtime.bootstrap.log"), path) | |
| 191 | - | require.FileExists(t, path) | |
| 152 | + | require.Equal(t, profile.CEFLogFile(), settings.LogFile) | |
| 153 | + | ||
| 154 | + | _, statErr := os.Stat(bootstrapLogFile) | |
| 155 | + | require.ErrorIs(t, statErr, os.ErrNotExist) | |
| 156 | + | require.Contains(t, logs.String(), puregoCEFInitTraceEnvVar) | |
| 157 | + | require.Contains(t, logs.String(), bootstrapLogFile) | |
| 192 | 158 | } | |
| 193 | 159 | ||
| 194 | 160 | // TestCreateAudioOutputFactory_CreatesUsableFactory verifies that the audio | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package cef | 1 | package cef | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "os" | 5 | "os" | ||
| 5 | "path/filepath" | 6 | "path/filepath" | ||
| 6 | "testing" | 7 | "testing" | ||
| @@ -59,13 +60,8 @@ func TestPrepareCEFSettings_AllowsNoThirdPartyCookiePolicy(t *testing.T) { | |||||
| 59 | require.NoError(t, err) | 60 | require.NoError(t, err) | ||
| 60 | } | 61 | } | ||
| 61 | 62 | ||||
| 62 | - | func TestPrepareCEFInitTraceFile_DisabledByDefault(t *testing.T) { | 63 | + | func TestResolveCEFInitTraceFile_EmptyWithoutLogPath(t *testing.T) { |
| 63 | - | t.Setenv(puregoCEFInitTraceEnvVar, "") | 64 | + | require.Empty(t, resolveCEFInitTraceFile("", "")) |
| 64 | - | profile := testCEFDevProfile(t) | |||
| 65 | - | ||||
| 66 | - | path, err := prepareCEFInitTraceFile(profile.CEFLogFile(), "") | |||
| 67 | - | require.NoError(t, err) | |||
| 68 | - | require.Empty(t, path) | |||
| 69 | } | 65 | } | ||
| 70 | 66 | ||||
| 71 | func TestPrepareCEFLogFile_CreatesPrivateDirectoryAndFile(t *testing.T) { | 67 | func TestPrepareCEFLogFile_CreatesPrivateDirectoryAndFile(t *testing.T) { | ||
| @@ -124,71 +120,41 @@ func TestPrepareCEFLogFile_RejectsDirectoryPath(t *testing.T) { | |||||
| 124 | require.Empty(t, path) | 120 | require.Empty(t, path) | ||
| 125 | } | 121 | } | ||
| 126 | 122 | ||||
| 127 | - | func TestPrepareCEFInitTraceFile_EnabledViaExplicitLogFile(t *testing.T) { | 123 | + | func TestResolveCEFInitTraceFile_UsesExplicitLogFile(t *testing.T) { |
| 128 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |||
| 129 | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | 124 | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | ||
| 130 | 125 | ||||
| 131 | - | path, err := prepareCEFInitTraceFile("", logFile) | 126 | + | path := resolveCEFInitTraceFile("", logFile) |
| 132 | - | require.NoError(t, err) | |||
| 133 | require.Equal(t, filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log"), path) | 127 | require.Equal(t, filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log"), path) | ||
| 134 | - | require.FileExists(t, path) | |||
| 135 | - | ||||
| 136 | - | info, err := os.Stat(path) | |||
| 137 | - | require.NoError(t, err) | |||
| 138 | - | require.Equal(t, os.FileMode(filePerm), info.Mode().Perm()) | |||
| 139 | - | ||||
| 140 | - | dirInfo, err := os.Stat(filepath.Dir(path)) | |||
| 141 | - | require.NoError(t, err) | |||
| 142 | - | require.Equal(t, os.FileMode(dirPerm), dirInfo.Mode().Perm()) | |||
| 143 | - | } | |||
| 144 | - | ||||
| 145 | - | func TestPrepareCEFInitTraceFile_TruncatesExistingFile(t *testing.T) { | |||
| 146 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | |||
| 147 | - | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | |||
| 148 | - | bootstrapLogFile := filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log") | |||
| 149 | - | require.NoError(t, os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm)) | |||
| 150 | - | require.NoError(t, os.WriteFile(bootstrapLogFile, []byte("existing"), 0o644)) | |||
| 151 | - | ||||
| 152 | - | path, err := prepareCEFInitTraceFile("", logFile) | |||
| 153 | - | require.NoError(t, err) | |||
| 154 | - | require.Equal(t, bootstrapLogFile, path) | |||
| 155 | - | ||||
| 156 | - | content, readErr := os.ReadFile(path) | |||
| 157 | - | require.NoError(t, readErr) | |||
| 158 | - | require.Empty(t, content) | |||
| 159 | - | ||||
| 160 | - | info, statErr := os.Stat(path) | |||
| 161 | - | require.NoError(t, statErr) | |||
| 162 | - | require.Equal(t, os.FileMode(filePerm), info.Mode().Perm()) | |||
| 163 | } | 128 | } | ||
| 164 | 129 | ||||
| 165 | - | func TestPrepareCEFInitTraceFile_RejectsSymlink(t *testing.T) { | 130 | + | func TestResolveCEFInitTraceFile_UsesResolvedProfileLogDirByDefault(t *testing.T) { |
| 166 | - | t.Setenv(puregoCEFInitTraceEnvVar, "1") | 131 | + | profile := testCEFDevProfile(t) |
| 167 | - | target := filepath.Join(t.TempDir(), "target.bootstrap.log") | |||
| 168 | - | require.NoError(t, os.WriteFile(target, []byte("existing"), 0o644)) | |||
| 169 | - | ||||
| 170 | - | logFile := filepath.Join(t.TempDir(), "custom", "cef_runtime.log") | |||
| 171 | - | bootstrapLogFile := filepath.Join(filepath.Dir(logFile), "cef_runtime.bootstrap.log") | |||
| 172 | - | require.NoError(t, os.MkdirAll(filepath.Dir(bootstrapLogFile), dirPerm)) | |||
| 173 | - | require.NoError(t, os.Symlink(target, bootstrapLogFile)) | |||
| 174 | - | ||||
| 175 | - | path, err := prepareCEFInitTraceFile("", logFile) | |||
| 176 | - | require.ErrorContains(t, err, "must not be a symlink") | |||
| 177 | - | require.Empty(t, path) | |||
| 178 | 132 | ||||
| 179 | - | targetInfo, statErr := os.Stat(target) | 133 | + | path := resolveCEFInitTraceFile(profile.CEFLogFile(), "") |
| 180 | - | require.NoError(t, statErr) | 134 | + | require.Equal(t, filepath.Join(profile.EnginePaths.LogDir, "cef_runtime.bootstrap.log"), path) |
| 181 | - | require.Equal(t, os.FileMode(0o644), targetInfo.Mode().Perm()) | |||
| 182 | } | 135 | } | ||
| 183 | 136 | ||||
| 184 | - | func TestPrepareCEFInitTraceFile_UsesResolvedProfileLogDirByDefault(t *testing.T) { | 137 | + | func TestPrepareCEFSettings_IgnoresInitTraceRequestWithoutTouchingFilesystem(t *testing.T) { |
| 185 | t.Setenv(puregoCEFInitTraceEnvVar, "1") | 138 | t.Setenv(puregoCEFInitTraceEnvVar, "1") | ||
| 186 | profile := testCEFDevProfile(t) | 139 | profile := testCEFDevProfile(t) | ||
| 140 | + | bootstrapLogFile := resolveCEFInitTraceFile(profile.CEFLogFile(), "") | |||
| 187 | 141 | ||||
| 188 | - | path, err := prepareCEFInitTraceFile(profile.CEFLogFile(), "") | 142 | + | var logs bytes.Buffer |
| 143 | + | logger := zerolog.New(&logs) | |||
| 144 | + | ||||
| 145 | + | settings, err := prepareCEFSettings( | |||
| 146 | + | port.EngineOptions{}, | |||
| 147 | + | RuntimePaths{StateRoot: profile.CEFUserDataDir(), LogFile: profile.CEFLogFile()}, | |||
| 148 | + | RuntimeConfig{}, | |||
| 149 | + | &logger, | |||
| 150 | + | ) | |||
| 189 | require.NoError(t, err) | 151 | require.NoError(t, err) | ||
| 190 | - | require.Equal(t, filepath.Join(profile.EnginePaths.LogDir, "cef_runtime.bootstrap.log"), path) | 152 | + | require.Equal(t, profile.CEFLogFile(), settings.LogFile) |
| 191 | - | require.FileExists(t, path) | 153 | + | |
| 154 | + | _, statErr := os.Stat(bootstrapLogFile) | |||
| 155 | + | require.ErrorIs(t, statErr, os.ErrNotExist) | |||
| 156 | + | require.Contains(t, logs.String(), puregoCEFInitTraceEnvVar) | |||
| 157 | + | require.Contains(t, logs.String(), bootstrapLogFile) | |||
| 192 | } | 158 | } | ||
| 193 | 159 | ||||
| 194 | // TestCreateAudioOutputFactory_CreatesUsableFactory verifies that the audio | 160 | // TestCreateAudioOutputFactory_CreatesUsableFactory verifies that the audio | ||