diff --git a/barnard.go b/barnard.go index 7b52299..3dac560 100644 --- a/barnard.go +++ b/barnard.go @@ -115,10 +115,10 @@ func (b *Barnard) TreeItemKeyPress(ui *uiterm.Ui, tree *uiterm.Tree, item uiterm users := makeUsersArray(treeItem.Channel.Users) for _, u := range users { // Explicitly set user mute state to match channel state - if channelWillBeMuted && !u.LocallyMuted() { - b.UserConfig.ToggleMute(u) - } else if !channelWillBeMuted && u.LocallyMuted() { - b.UserConfig.ToggleMute(u) + if channelWillBeMuted != u.LocallyMuted() { + if err := b.UserConfig.ToggleMute(u); err != nil { + b.AddOutputLine("Mute: could not save setting: " + err.Error()) + } } if source := u.AudioSource(); source != nil { @@ -158,7 +158,9 @@ func (b *Barnard) TreeItemKeyPress(ui *uiterm.Ui, tree *uiterm.Tree, item uiterm if treeItem.User != nil { if key == *b.Hotkeys.MuteToggle { // Toggle mute for single user - b.UserConfig.ToggleMute(treeItem.User) + if err := b.UserConfig.ToggleMute(treeItem.User); err != nil { + b.AddOutputLine("Mute: could not save setting: " + err.Error()) + } if source := treeItem.User.AudioSource(); source != nil { if treeItem.User.LocallyMuted() { source.SetGain(0) diff --git a/config/user_config.go b/config/user_config.go index 5451565..3fdfdc4 100644 --- a/config/user_config.go +++ b/config/user_config.go @@ -6,12 +6,16 @@ import ( "git.stormux.org/storm/barnard/uiterm" "github.com/pelletier/go-toml/v2" "io/ioutil" + "net" "os" + "path/filepath" "strconv" "strings" + "sync" ) type Config struct { + mu sync.Mutex config *exportableConfig fn string } @@ -45,20 +49,40 @@ type eUser struct { LocallyMuted bool // Changed from Muted to LocallyMuted to match User struct } -func (c *Config) SaveConfig() { - var data []byte +// SaveConfig atomically replaces the persisted configuration. Errors are +// returned so an unavailable directory cannot crash the client. +func (c *Config) SaveConfig() error { + c.mu.Lock() + defer c.mu.Unlock() + return c.saveConfigLocked() +} + +func (c *Config) saveConfigLocked() error { + if err := os.MkdirAll(filepath.Dir(c.fn), 0700); err != nil { + return err + } data, err := toml.Marshal(c.config) if err != nil { - panic(err) + return err } - err = ioutil.WriteFile(c.fn+".tmp", data, 0600) + file, err := os.CreateTemp(filepath.Dir(c.fn), filepath.Base(c.fn)+".tmp-") if err != nil { - panic(err) + return err } - err = os.Rename(c.fn+".tmp", c.fn) - if err != nil { - panic(err) + tmp := file.Name() + defer os.Remove(tmp) + if err := file.Chmod(0600); err != nil { + file.Close() + return err } + if _, err := file.Write(data); err != nil { + file.Close() + return err + } + if err := file.Close(); err != nil { + return err + } + return os.Rename(tmp, c.fn) } func key(k uiterm.Key) *uiterm.Key { @@ -78,8 +102,11 @@ func (c *Config) LoadConfig() { Exit: key(uiterm.KeyF10), ToggleTimestamps: key(uiterm.KeyF3), SwitchViews: key(uiterm.KeyTab), + ClearOutput: key(uiterm.KeyCtrlL), ScrollUp: key(uiterm.KeyPgup), ScrollDown: key(uiterm.KeyPgdn), + ScrollToTop: key(uiterm.KeyHome), + ScrollToBottom: key(uiterm.KeyEnd), AdminMenu: key(uiterm.KeyF11), NoiseSuppressionToggle: key(uiterm.KeyF9), } @@ -156,8 +183,11 @@ func (c *Config) ensureHotkeys() { Exit: key(uiterm.KeyF10), ToggleTimestamps: key(uiterm.KeyF3), SwitchViews: key(uiterm.KeyTab), + ClearOutput: key(uiterm.KeyCtrlL), ScrollUp: key(uiterm.KeyPgup), ScrollDown: key(uiterm.KeyPgdn), + ScrollToTop: key(uiterm.KeyHome), + ScrollToBottom: key(uiterm.KeyEnd), AdminMenu: key(uiterm.KeyF11), NoiseSuppressionToggle: key(uiterm.KeyF9), } @@ -189,12 +219,21 @@ func (c *Config) ensureHotkeys() { if hotkeys.SwitchViews == nil { hotkeys.SwitchViews = defaults.SwitchViews } + if hotkeys.ClearOutput == nil { + hotkeys.ClearOutput = defaults.ClearOutput + } if hotkeys.ScrollUp == nil { hotkeys.ScrollUp = defaults.ScrollUp } if hotkeys.ScrollDown == nil { hotkeys.ScrollDown = defaults.ScrollDown } + if hotkeys.ScrollToTop == nil { + hotkeys.ScrollToTop = defaults.ScrollToTop + } + if hotkeys.ScrollToBottom == nil { + hotkeys.ScrollToBottom = defaults.ScrollToBottom + } if hotkeys.AdminMenu == nil { hotkeys.AdminMenu = defaults.AdminMenu } @@ -250,14 +289,18 @@ func (c *Config) findUser(address string, username string) *eUser { return t } -func (c *Config) ToggleMute(u *gumble.User) { +func (c *Config) ToggleMute(u *gumble.User) error { + c.mu.Lock() + defer c.mu.Unlock() j := c.findUser(u.GetClient().Config.Address, u.Name) j.LocallyMuted = !j.LocallyMuted u.SetLocallyMuted(j.LocallyMuted) - c.SaveConfig() + return c.saveConfigLocked() } func (c *Config) SetMicVolume(v float32) { + c.mu.Lock() + defer c.mu.Unlock() t := float32(v) c.config.MicVolume = &t } @@ -298,18 +341,24 @@ func (c *Config) GetCertificate() *string { } func (c *Config) GetNoiseSuppressionEnabled() bool { + c.mu.Lock() + defer c.mu.Unlock() if c.config.NoiseSuppressionEnabled == nil { return false } return *c.config.NoiseSuppressionEnabled } -func (c *Config) SetNoiseSuppressionEnabled(enabled bool) { +func (c *Config) SetNoiseSuppressionEnabled(enabled bool) error { + c.mu.Lock() + defer c.mu.Unlock() c.config.NoiseSuppressionEnabled = &enabled - c.SaveConfig() + return c.saveConfigLocked() } func (c *Config) GetRecordingFormat() string { + c.mu.Lock() + defer c.mu.Unlock() if c.config.RecordingFormat == nil { return "flac" } @@ -317,6 +366,8 @@ func (c *Config) GetRecordingFormat() string { } func (c *Config) GetRecordingDirectory() string { + c.mu.Lock() + defer c.mu.Unlock() if c.config.RecordingDirectory == nil { return resolvePath("~/Audio") } @@ -324,6 +375,8 @@ func (c *Config) GetRecordingDirectory() string { } func (c *Config) UpdateUser(u *gumble.User) { + c.mu.Lock() + defer c.mu.Unlock() var j *eUser var uc *gumble.Client uc = u.GetClient() @@ -339,6 +392,8 @@ func (c *Config) UpdateUser(u *gumble.User) { } func (c *Config) UpdateConfig(u *gumble.User) { + c.mu.Lock() + defer c.mu.Unlock() var j *eUser j = c.findUser(u.GetClient().Config.Address, u.Name) j.Boost = u.Boost() @@ -346,6 +401,20 @@ func (c *Config) UpdateConfig(u *gumble.User) { j.LocallyMuted = u.LocallyMuted() // Save LocallyMuted state to config } +// RequireConfigFile verifies that an explicitly requested configuration file +// exists and is a regular file. The default configuration remains optional. +func RequireConfigFile(fn string) error { + path := resolvePath(fn) + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("config file %q: %w", path, err) + } + if !info.Mode().IsRegular() { + return fmt.Errorf("config file %q is not a regular file", path) + } + return nil +} + func NewConfig(fn *string) *Config { var c *Config c = &Config{} @@ -367,7 +436,7 @@ func readFile(path string) []byte { func fileExists(path string) bool { info, err := os.Stat(path) - if os.IsNotExist(err) { + if err != nil { return false } return !info.IsDir() @@ -389,11 +458,16 @@ func resolvePath(path string) string { } func makeHostPort(addr string) (string, int) { - parts := strings.Split(addr, ":") - host := parts[0] - port, err := strconv.Atoi(parts[1]) + // SplitHostPort correctly handles bracketed IPv6. Invalid or portless + // addresses stay usable as a host with Mumble's default port instead of + // crashing configuration operations. + host, portText, err := net.SplitHostPort(addr) if err != nil { - panic(err) + return strings.Trim(addr, "[]"), 64738 + } + port, err := strconv.Atoi(portText) + if err != nil || port < 1 || port > 65535 { + return host, 64738 } return host, port } diff --git a/config/user_config_save_test.go b/config/user_config_save_test.go new file mode 100644 index 0000000..6fee7ad --- /dev/null +++ b/config/user_config_save_test.go @@ -0,0 +1,60 @@ +package config + +import ( + "os" + "path/filepath" + "sync" + "testing" +) + +func TestSaveConfigCreatesMissingParentDirectory(t *testing.T) { + parent := filepath.Join(t.TempDir(), "missing") + path := filepath.Join(parent, "barnard.toml") + cfg := NewConfig(&path) + if err := cfg.SaveConfig(); err != nil { + t.Fatal(err) + } + info, err := os.Stat(parent) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm()&0077 != 0 { + t.Fatalf("parent directory permissions = %o, want no group or other access", info.Mode().Perm()) + } +} + +func TestSaveConfigDoesNotUsePredictableTemporaryPath(t *testing.T) { + path := filepath.Join(t.TempDir(), "barnard.toml") + legacyTemp := path + ".tmp" + if err := os.WriteFile(legacyTemp, []byte("sentinel"), 0600); err != nil { + t.Fatal(err) + } + cfg := NewConfig(&path) + if err := cfg.SaveConfig(); err != nil { + t.Fatal(err) + } + contents, err := os.ReadFile(legacyTemp) + if err != nil { + t.Fatal(err) + } + if string(contents) != "sentinel" { + t.Fatalf("predictable temporary file was modified: %q", contents) + } +} + +func TestConcurrentConfigurationUpdatesAndWrites(t *testing.T) { + path := filepath.Join(t.TempDir(), "barnard.toml") + cfg := NewConfig(&path) + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + wg.Add(1) + go func(enabled bool) { + defer wg.Done() + cfg.SetNoiseSuppressionEnabled(enabled) + if err := cfg.SaveConfig(); err != nil { + t.Errorf("SaveConfig: %v", err) + } + }(i%2 == 0) + } + wg.Wait() +} diff --git a/config/user_config_test.go b/config/user_config_test.go index a82d078..2b01c81 100644 --- a/config/user_config_test.go +++ b/config/user_config_test.go @@ -8,6 +8,25 @@ import ( "git.stormux.org/storm/barnard/uiterm" ) +// Regression: an explicit -config path silently fell back to in-memory +// defaults, then overwrote the intended file on exit. +func TestRequireConfigFileRejectsMissingExplicitPath(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing.toml") + if err := RequireConfigFile(missing); err == nil { + t.Fatal("missing explicit config was accepted") + } +} + +func TestRequireConfigFileRejectsNonRegularFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.fifo") + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + if err := RequireConfigFile(path); err == nil { + t.Fatal("directory was accepted as an explicit config file") + } +} + func TestConfigBackfillsRecordingDefaults(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "barnard.toml") @@ -35,8 +54,35 @@ func TestConfigBackfillsRecordingDefaults(t *testing.T) { if got := *cfg.GetHotkeys().AdminMenu; got != uiterm.KeyF11 { t.Fatalf("expected admin menu f11, got %s", got) } + for name, got := range map[string]*uiterm.Key{ + "clear output": cfg.GetHotkeys().ClearOutput, + "scroll to top": cfg.GetHotkeys().ScrollToTop, + "scroll to bottom": cfg.GetHotkeys().ScrollToBottom, + } { + if got == nil { + t.Fatalf("expected %s hotkey to be backfilled", name) + } + } + if got := *cfg.GetHotkeys().ClearOutput; got != uiterm.KeyCtrlL { + t.Fatalf("expected clear output ctrl_l, got %s", got) + } + if got := *cfg.GetHotkeys().ScrollToTop; got != uiterm.KeyHome { + t.Fatalf("expected scroll to top home, got %s", got) + } + if got := *cfg.GetHotkeys().ScrollToBottom; got != uiterm.KeyEnd { + t.Fatalf("expected scroll to bottom end, got %s", got) + } +} +func TestMakeHostPortHandlesIPv6AndMalformedAddress(t *testing.T) { + host, port := makeHostPort("[2001:db8::1]:64739") + if host != "2001:db8::1" || port != 64739 { + t.Fatalf("got %q:%d", host, port) + } + host, port = makeHostPort("not-a-host-port") + if host != "not-a-host-port" || port != 64738 { + t.Fatalf("got %q:%d", host, port) + } } - func TestConfigUsesHomeEnvironmentForDefaultPath(t *testing.T) { dir := t.TempDir() t.Setenv("HOME", dir) diff --git a/main.go b/main.go index 57381e2..f11e015 100644 --- a/main.go +++ b/main.go @@ -114,6 +114,8 @@ func main() { fifo := flag.String("fifo", "", "path of a FIFO from which to read commands") serverSet := false usernameSet := false + configSet := false + certificateSet := false buffers := flag.Int("buffers", 16, "number of audio buffers to use") audioInterval := flag.Int("audio-interval", 10, "outgoing audio packet duration in ms (10, 20, 40, or 60)") jitterBuffer := flag.Int("jitter-buffer", 40, "incoming per-user audio buffer in ms (0, 20, 40, or 60)") @@ -165,19 +167,24 @@ func main() { }() } - userConfig := config.NewConfig(cfgfn) - - certificateSet := false flag.CommandLine.Visit(func(theFlag *flag.Flag) { switch theFlag.Name { case "server": serverSet = true case "username": usernameSet = true + case "config": + configSet = true case "certificate": certificateSet = true } }) + if configSet { + if err := config.RequireConfigFile(*cfgfn); err != nil { + handle_raw_error(err) + } + } + userConfig := config.NewConfig(cfgfn) if !serverSet { server = userConfig.GetDefaultServer() @@ -229,13 +236,19 @@ func main() { b.Config.IncomingAudioBuffer = selectedJitterBuffer b.Hotkeys = b.UserConfig.GetHotkeys() - b.UserConfig.SaveConfig() + if err := b.UserConfig.SaveConfig(); err != nil { + fmt.Fprintf(os.Stderr, "could not save configuration: %s\n", err) + os.Exit(1) + } // Configure noise suppression enabled := b.UserConfig.GetNoiseSuppressionEnabled() if *noiseSuppressionEnabled { enabled = true - b.UserConfig.SetNoiseSuppressionEnabled(true) + if err := b.UserConfig.SetNoiseSuppressionEnabled(true); err != nil { + fmt.Fprintf(os.Stderr, "could not save configuration: %s\n", err) + os.Exit(1) + } } b.NoiseSuppressor.SetEnabled(enabled) diff --git a/ui.go b/ui.go index c83e31a..ac7b9c0 100644 --- a/ui.go +++ b/ui.go @@ -99,7 +99,9 @@ func (b *Barnard) OnTimestampToggle(ui *uiterm.Ui, key uiterm.Key) { func (b *Barnard) OnNoiseSuppressionToggle(ui *uiterm.Ui, key uiterm.Key) { enabled := !b.UserConfig.GetNoiseSuppressionEnabled() - b.UserConfig.SetNoiseSuppressionEnabled(enabled) + if err := b.UserConfig.SetNoiseSuppressionEnabled(enabled); err != nil { + b.AddOutputLine("Noise suppression: could not save setting: " + err.Error()) + } b.NoiseSuppressor.SetEnabled(enabled) if enabled { @@ -161,7 +163,9 @@ func (b *Barnard) CommandMicDown(ui *uiterm.Ui, cmd string) { func (b *Barnard) CommandNoiseSuppressionToggle(ui *uiterm.Ui, cmd string) { enabled := !b.UserConfig.GetNoiseSuppressionEnabled() - b.UserConfig.SetNoiseSuppressionEnabled(enabled) + if err := b.UserConfig.SetNoiseSuppressionEnabled(enabled); err != nil { + b.AddOutputLine("Noise suppression: could not save setting: " + err.Error()) + } b.NoiseSuppressor.SetEnabled(enabled) if enabled { diff --git a/ui_tree.go b/ui_tree.go index 7057777..87ca153 100644 --- a/ui_tree.go +++ b/ui_tree.go @@ -63,7 +63,9 @@ func (b *Barnard) changeVolume(users []*gumble.User, change float32) { } b.UserConfig.UpdateConfig(u) } - b.UserConfig.SaveConfig() + if err := b.UserConfig.SaveConfig(); err != nil { + b.AddOutputLine("Volume: could not save setting: " + err.Error()) + } } func (b *Barnard) resetVolume(users []*gumble.User) { @@ -80,7 +82,9 @@ func (b *Barnard) resetVolume(users []*gumble.User) { } b.UserConfig.UpdateConfig(u) } - b.UserConfig.SaveConfig() + if err := b.UserConfig.SaveConfig(); err != nil { + b.AddOutputLine("Volume: could not save setting: " + err.Error()) + } } func makeUsersArray(users gumble.Users) []*gumble.User {