diff --git a/config/config.go b/config/config.go index 132973f0..5be89889 100644 --- a/config/config.go +++ b/config/config.go @@ -13,6 +13,18 @@ import ( "github.com/spf13/viper" ) +// Configuration constants +const ( + DefaultWorkerNum = 0 // 0 means use runtime.NumCPU() + DefaultQueueNum = 8192 + DefaultMaxNotification = 100 + DefaultShutdownTimeout = 30 + DefaultFeedbackTimeout = 10 + DefaultPort = "8088" + DefaultGRPCPort = "9000" + DefaultMaxConcurrentPush = 100 +) + var defaultConf = []byte(` core: enabled: true # enable httpd server @@ -316,13 +328,13 @@ func setDefaults() { // Core defaults viper.SetDefault("core.enabled", true) viper.SetDefault("core.address", "") - viper.SetDefault("core.shutdown_timeout", 30) - viper.SetDefault("core.port", "8088") - viper.SetDefault("core.worker_num", 0) - viper.SetDefault("core.queue_num", 0) - viper.SetDefault("core.max_notification", 100) + viper.SetDefault("core.shutdown_timeout", DefaultShutdownTimeout) + viper.SetDefault("core.port", DefaultPort) + viper.SetDefault("core.worker_num", DefaultWorkerNum) + viper.SetDefault("core.queue_num", DefaultWorkerNum) // 0, will be set to DefaultQueueNum at runtime if still 0 + viper.SetDefault("core.max_notification", DefaultMaxNotification) viper.SetDefault("core.sync", false) - viper.SetDefault("core.feedback_timeout", 10) + viper.SetDefault("core.feedback_timeout", DefaultFeedbackTimeout) viper.SetDefault("core.mode", "release") viper.SetDefault("core.ssl", false) viper.SetDefault("core.cert_path", "cert.pem") @@ -337,7 +349,7 @@ func setDefaults() { viper.SetDefault("ios.enabled", false) viper.SetDefault("ios.key_type", "pem") viper.SetDefault("ios.production", false) - viper.SetDefault("ios.max_concurrent_pushes", uint(100)) + viper.SetDefault("ios.max_concurrent_pushes", uint(DefaultMaxConcurrentPush)) viper.SetDefault("ios.max_retry", 0) // Android defaults @@ -346,7 +358,7 @@ func setDefaults() { // gRPC defaults viper.SetDefault("grpc.enabled", false) - viper.SetDefault("grpc.port", "9000") + viper.SetDefault("grpc.port", DefaultGRPCPort) // Queue defaults viper.SetDefault("queue.engine", "local") @@ -505,12 +517,14 @@ func loadConfigFromViper() (*ConfYaml, error) { conf.GRPC.Enabled = viper.GetBool("grpc.enabled") conf.GRPC.Port = viper.GetString("grpc.port") - if conf.Core.WorkerNum == int64(0) { + // Apply runtime-computed defaults for zero or negative values + // This ensures optimal resource utilization and prevents panics from negative values + if conf.Core.WorkerNum <= 0 { conf.Core.WorkerNum = int64(runtime.NumCPU()) } - if conf.Core.QueueNum == int64(0) { - conf.Core.QueueNum = int64(8192) + if conf.Core.QueueNum <= 0 { + conf.Core.QueueNum = DefaultQueueNum } return conf, nil diff --git a/config/config_test.go b/config/config_test.go index 0169f9b1..13831b91 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -29,7 +29,7 @@ func TestInvalidYAMLFile(t *testing.T) { content := []byte("invalid: yaml: content: [unclosed") // Write invalid content to a temporary file - err := os.WriteFile(tmpFile, content, 0o644) + err := os.WriteFile(tmpFile, content, 0o600) assert.NoError(t, err) defer os.Remove(tmpFile) // Clean up @@ -67,7 +67,7 @@ func TestEmptyConfig(t *testing.T) { panic("failed to load config.yml from file") } - assert.Equal(t, uint(100), conf.Ios.MaxConcurrentPushes) + assert.Equal(t, uint(DefaultMaxConcurrentPush), conf.Ios.MaxConcurrentPushes) } type ConfigTestSuite struct { @@ -91,22 +91,22 @@ func (suite *ConfigTestSuite) SetupTest() { func (suite *ConfigTestSuite) TestValidateConfDefault() { // Core assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.Address) - assert.Equal(suite.T(), "8088", suite.ConfGorushDefault.Core.Port) - assert.Equal(suite.T(), int64(30), suite.ConfGorushDefault.Core.ShutdownTimeout) + assert.Equal(suite.T(), DefaultPort, suite.ConfGorushDefault.Core.Port) + assert.Equal(suite.T(), int64(DefaultShutdownTimeout), suite.ConfGorushDefault.Core.ShutdownTimeout) assert.Equal(suite.T(), true, suite.ConfGorushDefault.Core.Enabled) assert.Equal(suite.T(), int64(runtime.NumCPU()), suite.ConfGorushDefault.Core.WorkerNum) - assert.Equal(suite.T(), int64(8192), suite.ConfGorushDefault.Core.QueueNum) + assert.Equal(suite.T(), int64(DefaultQueueNum), suite.ConfGorushDefault.Core.QueueNum) assert.Equal(suite.T(), "release", suite.ConfGorushDefault.Core.Mode) assert.Equal(suite.T(), false, suite.ConfGorushDefault.Core.Sync) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.FeedbackURL) assert.Equal(suite.T(), 0, len(suite.ConfGorushDefault.Core.FeedbackHeader)) - assert.Equal(suite.T(), int64(10), suite.ConfGorushDefault.Core.FeedbackTimeout) + assert.Equal(suite.T(), int64(DefaultFeedbackTimeout), suite.ConfGorushDefault.Core.FeedbackTimeout) assert.Equal(suite.T(), false, suite.ConfGorushDefault.Core.SSL) assert.Equal(suite.T(), "cert.pem", suite.ConfGorushDefault.Core.CertPath) assert.Equal(suite.T(), "key.pem", suite.ConfGorushDefault.Core.KeyPath) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.KeyBase64) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.CertBase64) - assert.Equal(suite.T(), int64(100), suite.ConfGorushDefault.Core.MaxNotification) + assert.Equal(suite.T(), int64(DefaultMaxNotification), suite.ConfGorushDefault.Core.MaxNotification) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.HTTPProxy) // Pid assert.Equal(suite.T(), false, suite.ConfGorushDefault.Core.PID.Enabled) @@ -138,7 +138,7 @@ func (suite *ConfigTestSuite) TestValidateConfDefault() { assert.Equal(suite.T(), "pem", suite.ConfGorushDefault.Ios.KeyType) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Ios.Password) assert.Equal(suite.T(), false, suite.ConfGorushDefault.Ios.Production) - assert.Equal(suite.T(), uint(100), suite.ConfGorushDefault.Ios.MaxConcurrentPushes) + assert.Equal(suite.T(), uint(DefaultMaxConcurrentPush), suite.ConfGorushDefault.Ios.MaxConcurrentPushes) assert.Equal(suite.T(), 0, suite.ConfGorushDefault.Ios.MaxRetry) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Ios.KeyID) assert.Equal(suite.T(), "", suite.ConfGorushDefault.Ios.TeamID) @@ -186,20 +186,20 @@ func (suite *ConfigTestSuite) TestValidateConfDefault() { // gRPC assert.Equal(suite.T(), false, suite.ConfGorushDefault.GRPC.Enabled) - assert.Equal(suite.T(), "9000", suite.ConfGorushDefault.GRPC.Port) + assert.Equal(suite.T(), DefaultGRPCPort, suite.ConfGorushDefault.GRPC.Port) } func (suite *ConfigTestSuite) TestValidateConf() { // Core - assert.Equal(suite.T(), "8088", suite.ConfGorush.Core.Port) - assert.Equal(suite.T(), int64(30), suite.ConfGorush.Core.ShutdownTimeout) + assert.Equal(suite.T(), DefaultPort, suite.ConfGorush.Core.Port) + assert.Equal(suite.T(), int64(DefaultShutdownTimeout), suite.ConfGorush.Core.ShutdownTimeout) assert.Equal(suite.T(), true, suite.ConfGorush.Core.Enabled) assert.Equal(suite.T(), int64(runtime.NumCPU()), suite.ConfGorush.Core.WorkerNum) - assert.Equal(suite.T(), int64(8192), suite.ConfGorush.Core.QueueNum) + assert.Equal(suite.T(), int64(DefaultQueueNum), suite.ConfGorush.Core.QueueNum) assert.Equal(suite.T(), "release", suite.ConfGorush.Core.Mode) assert.Equal(suite.T(), false, suite.ConfGorush.Core.Sync) assert.Equal(suite.T(), "", suite.ConfGorush.Core.FeedbackURL) - assert.Equal(suite.T(), int64(10), suite.ConfGorush.Core.FeedbackTimeout) + assert.Equal(suite.T(), int64(DefaultFeedbackTimeout), suite.ConfGorush.Core.FeedbackTimeout) assert.Equal(suite.T(), 1, len(suite.ConfGorush.Core.FeedbackHeader)) assert.Equal(suite.T(), "x-gorush-token:4e989115e09680f44a645519fed6a976", suite.ConfGorush.Core.FeedbackHeader[0]) assert.Equal(suite.T(), false, suite.ConfGorush.Core.SSL) @@ -207,7 +207,7 @@ func (suite *ConfigTestSuite) TestValidateConf() { assert.Equal(suite.T(), "key.pem", suite.ConfGorush.Core.KeyPath) assert.Equal(suite.T(), "", suite.ConfGorush.Core.CertBase64) assert.Equal(suite.T(), "", suite.ConfGorush.Core.KeyBase64) - assert.Equal(suite.T(), int64(100), suite.ConfGorush.Core.MaxNotification) + assert.Equal(suite.T(), int64(DefaultMaxNotification), suite.ConfGorush.Core.MaxNotification) assert.Equal(suite.T(), "", suite.ConfGorush.Core.HTTPProxy) // Pid assert.Equal(suite.T(), false, suite.ConfGorush.Core.PID.Enabled) @@ -239,7 +239,7 @@ func (suite *ConfigTestSuite) TestValidateConf() { assert.Equal(suite.T(), "pem", suite.ConfGorush.Ios.KeyType) assert.Equal(suite.T(), "", suite.ConfGorush.Ios.Password) assert.Equal(suite.T(), false, suite.ConfGorush.Ios.Production) - assert.Equal(suite.T(), uint(100), suite.ConfGorush.Ios.MaxConcurrentPushes) + assert.Equal(suite.T(), uint(DefaultMaxConcurrentPush), suite.ConfGorush.Ios.MaxConcurrentPushes) assert.Equal(suite.T(), 0, suite.ConfGorush.Ios.MaxRetry) assert.Equal(suite.T(), "", suite.ConfGorush.Ios.KeyID) assert.Equal(suite.T(), "", suite.ConfGorush.Ios.TeamID) @@ -268,7 +268,7 @@ func (suite *ConfigTestSuite) TestValidateConf() { // gRPC assert.Equal(suite.T(), false, suite.ConfGorush.GRPC.Enabled) - assert.Equal(suite.T(), "9000", suite.ConfGorush.GRPC.Port) + assert.Equal(suite.T(), DefaultGRPCPort, suite.ConfGorush.GRPC.Port) } func TestConfigTestSuite(t *testing.T) { @@ -556,7 +556,7 @@ func TestValidateConfig(t *testing.T) { name: "valid config", cfg: &ConfYaml{ Core: SectionCore{ - Port: "8088", + Port: DefaultPort, Address: "0.0.0.0", }, Stat: SectionStat{ @@ -620,7 +620,7 @@ func TestSecurityValidationIntegration(t *testing.T) { // Test that all validation functions work together t.Run("complete security validation", func(t *testing.T) { // Test valid inputs - if err := ValidatePort("8088"); err != nil { + if err := ValidatePort(DefaultPort); err != nil { t.Errorf("Valid port should not error: %v", err) } @@ -646,3 +646,116 @@ func TestSecurityValidationIntegration(t *testing.T) { } }) } + +// TestNegativeValues tests that negative configuration values are handled safely +func TestNegativeValues(t *testing.T) { + tests := []struct { + name string + workerNum int64 + queueNum int64 + expectedWorker int64 + expectedQueue int64 + description string + }{ + { + name: "negative_worker_num_should_use_cpu_count", + workerNum: -5, + queueNum: DefaultQueueNum, + expectedWorker: int64(runtime.NumCPU()), + expectedQueue: DefaultQueueNum, + description: "Negative WorkerNum should be replaced with CPU count", + }, + { + name: "negative_queue_num_should_use_default", + workerNum: 4, // Valid positive value + queueNum: -100, + expectedWorker: 4, + expectedQueue: DefaultQueueNum, + description: "Negative QueueNum should be replaced with default", + }, + { + name: "both_negative_should_use_defaults", + workerNum: -1, + queueNum: -1, + expectedWorker: int64(runtime.NumCPU()), + expectedQueue: DefaultQueueNum, + description: "Both negative values should be replaced with defaults", + }, + { + name: "zero_worker_num_should_use_cpu_count", + workerNum: 0, + queueNum: DefaultQueueNum, + expectedWorker: int64(runtime.NumCPU()), + expectedQueue: DefaultQueueNum, + description: "Zero WorkerNum should be replaced with CPU count", + }, + { + name: "zero_queue_num_should_use_default", + workerNum: 4, + queueNum: 0, + expectedWorker: 4, + expectedQueue: DefaultQueueNum, + description: "Zero QueueNum should be replaced with default", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create config with test values + conf := &ConfYaml{} + conf.Core.WorkerNum = tt.workerNum + conf.Core.QueueNum = tt.queueNum + + // Apply the same logic as loadConfigFromViper + if conf.Core.WorkerNum <= 0 { + conf.Core.WorkerNum = int64(runtime.NumCPU()) + } + if conf.Core.QueueNum <= 0 { + conf.Core.QueueNum = DefaultQueueNum + } + + // Verify results + assert.Equal(t, tt.expectedWorker, conf.Core.WorkerNum, + "WorkerNum: %s", tt.description) + assert.Equal(t, tt.expectedQueue, conf.Core.QueueNum, + "QueueNum: %s", tt.description) + + // Ensure values are always positive (safety check) + assert.True(t, conf.Core.WorkerNum > 0, + "WorkerNum should be positive to prevent panics") + assert.True(t, conf.Core.QueueNum > 0, + "QueueNum should be positive to prevent panics") + }) + } +} + +// TestConfigSafetyBoundaries tests edge cases for configuration safety +func TestConfigSafetyBoundaries(t *testing.T) { + tests := []struct { + name string + value int64 + expected int64 + usesCPU bool + }{ + {"very_negative", -9999, DefaultQueueNum, false}, + {"negative_one", -1, DefaultQueueNum, false}, + {"zero", 0, DefaultQueueNum, false}, + {"positive_one", 1, 1, false}, + } + + for _, tt := range tests { + t.Run(tt.name+"_queue", func(t *testing.T) { + conf := &ConfYaml{} + conf.Core.QueueNum = tt.value + conf.Core.WorkerNum = 1 // Set to valid positive value + + // Apply safety logic + if conf.Core.QueueNum <= 0 { + conf.Core.QueueNum = DefaultQueueNum + } + + assert.Equal(t, tt.expected, conf.Core.QueueNum) + assert.True(t, conf.Core.QueueNum > 0, "QueueNum must be positive") + }) + } +}