diff --git a/fix.txt b/fix.txt index d7205ba..d24395f 100644 --- a/fix.txt +++ b/fix.txt @@ -182,20 +182,20 @@ Priority 2: protocol and data correctness is 2x/4x/6x too high. Calculate bits per frame divided by the actual Config.AudioInterval, or set the bitrate once when configuration changes. -23. AudioInterval accepts invalid values +[x] 23. AudioInterval accepts invalid values File: gumble/gumble/config.go AudioFrameSize truncates arbitrary intervals to a count of 10 ms frames, while the ticker still uses the original interval. Validate and reject values other than 10/20/40/60 ms (and validate AudioDataBytes/Buffers). -24. Legacy/custom varint has a MinInt64 recursion failure +[x] 24. Legacy/custom varint has a MinInt64 recursion failure File: gumble/gumble/varint/write.go Encoding math.MinInt64 evaluates -value to the same negative number and recursively encodes forever until panic. Handle MinInt64 explicitly or encode negatives using an unsigned magnitude without overflow. Validate output buffer capacity in the exported encoder too. -25. Mumble version layout documentation is wrong +[x] 25. Mumble version layout documentation is wrong File: gumble/gumble/version.go The comment says major uses bits 0-15, but SemanticVersion and ClientVersion use bits 16-31. Correct the documentation and add known version tests. diff --git a/gumble/gumble/client.go b/gumble/gumble/client.go index f400f49..3c946f6 100644 --- a/gumble/gumble/client.go +++ b/gumble/gumble/client.go @@ -110,6 +110,9 @@ func Dial(config *Config) (*Client, error) { // min(time.Now() + dialer.Timeout, dialer.Deadline), or if the server rejects // the client. func DialWithDialer(dialer *net.Dialer, config *Config, tlsConfig *tls.Config) (*Client, error) { + if err := config.Validate(); err != nil { + return nil, err + } start := time.Now() conn, err := tls.DialWithDialer(dialer, "tcp", config.Address, tlsConfig) diff --git a/gumble/gumble/config.go b/gumble/gumble/config.go index f0add06..0400bca 100644 --- a/gumble/gumble/config.go +++ b/gumble/gumble/config.go @@ -1,6 +1,7 @@ package gumble import ( + "fmt" "time" ) @@ -43,6 +44,22 @@ func NewConfig() *Config { } } +// Validate checks values that are used by the audio ticker and encoder. +func (c *Config) Validate() error { + switch c.AudioInterval { + case 10 * time.Millisecond, 20 * time.Millisecond, 40 * time.Millisecond, 60 * time.Millisecond: + default: + return fmt.Errorf("gumble: AudioInterval must be 10ms, 20ms, 40ms, or 60ms") + } + if c.AudioDataBytes <= 0 { + return fmt.Errorf("gumble: AudioDataBytes must be positive") + } + if c.Buffers <= 0 { + return fmt.Errorf("gumble: Buffers must be positive") + } + return nil +} + // Attach is an alias of c.Listeners.Attach. func (c *Config) Attach(l EventListener) Detacher { return c.Listeners.Attach(l) diff --git a/gumble/gumble/config_regression_test.go b/gumble/gumble/config_regression_test.go new file mode 100644 index 0000000..70d78ca --- /dev/null +++ b/gumble/gumble/config_regression_test.go @@ -0,0 +1,20 @@ +package gumble + +import ( + "testing" + "time" +) + +// Regression: arbitrary intervals were truncated to 10 ms frames while the +// ticker kept the original duration, producing malformed audio timing. +func TestConfigValidateRejectsUnsupportedAudioInterval(t *testing.T) { + config := NewConfig() + config.AudioInterval = 15 * time.Millisecond + if err := config.Validate(); err == nil { + t.Fatal("invalid audio interval was accepted") + } + config.AudioInterval = 60 * time.Millisecond + if err := config.Validate(); err != nil { + t.Fatalf("valid audio interval rejected: %v", err) + } +}