Skip to content

Commit 14b9c3f

Browse files
committed
fix: enforce positive defaults for queue and worker configuration values
- Ensure WorkerNum and QueueNum are set to positive defaults if configured with zero or negative values - Prevent potential panics by treating non-positive configuration values as invalid and replacing them at runtime - Add tests to verify negative and zero configuration values are safely corrected to defaults - Add new safety boundary tests for QueueNum to further validate configuration handling Signed-off-by: appleboy <[email protected]>
1 parent 0b02476 commit 14b9c3f

File tree

2 files changed

+117
-4
lines changed

2 files changed

+117
-4
lines changed

config/config.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -517,13 +517,13 @@ func loadConfigFromViper() (*ConfYaml, error) {
517517
conf.GRPC.Enabled = viper.GetBool("grpc.enabled")
518518
conf.GRPC.Port = viper.GetString("grpc.port")
519519

520-
// Apply runtime-computed defaults for zero values
521-
// This ensures optimal resource utilization based on system capabilities
522-
if conf.Core.WorkerNum == DefaultWorkerNum {
520+
// Apply runtime-computed defaults for zero or negative values
521+
// This ensures optimal resource utilization and prevents panics from negative values
522+
if conf.Core.WorkerNum <= 0 {
523523
conf.Core.WorkerNum = int64(runtime.NumCPU())
524524
}
525525

526-
if conf.Core.QueueNum == DefaultWorkerNum {
526+
if conf.Core.QueueNum <= 0 {
527527
conf.Core.QueueNum = DefaultQueueNum
528528
}
529529

config/config_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,3 +646,116 @@ func TestSecurityValidationIntegration(t *testing.T) {
646646
}
647647
})
648648
}
649+
650+
// TestNegativeValues tests that negative configuration values are handled safely
651+
func TestNegativeValues(t *testing.T) {
652+
tests := []struct {
653+
name string
654+
workerNum int64
655+
queueNum int64
656+
expectedWorker int64
657+
expectedQueue int64
658+
description string
659+
}{
660+
{
661+
name: "negative_worker_num_should_use_cpu_count",
662+
workerNum: -5,
663+
queueNum: DefaultQueueNum,
664+
expectedWorker: int64(runtime.NumCPU()),
665+
expectedQueue: DefaultQueueNum,
666+
description: "Negative WorkerNum should be replaced with CPU count",
667+
},
668+
{
669+
name: "negative_queue_num_should_use_default",
670+
workerNum: 4, // Valid positive value
671+
queueNum: -100,
672+
expectedWorker: 4,
673+
expectedQueue: DefaultQueueNum,
674+
description: "Negative QueueNum should be replaced with default",
675+
},
676+
{
677+
name: "both_negative_should_use_defaults",
678+
workerNum: -1,
679+
queueNum: -1,
680+
expectedWorker: int64(runtime.NumCPU()),
681+
expectedQueue: DefaultQueueNum,
682+
description: "Both negative values should be replaced with defaults",
683+
},
684+
{
685+
name: "zero_worker_num_should_use_cpu_count",
686+
workerNum: 0,
687+
queueNum: DefaultQueueNum,
688+
expectedWorker: int64(runtime.NumCPU()),
689+
expectedQueue: DefaultQueueNum,
690+
description: "Zero WorkerNum should be replaced with CPU count",
691+
},
692+
{
693+
name: "zero_queue_num_should_use_default",
694+
workerNum: 4,
695+
queueNum: 0,
696+
expectedWorker: 4,
697+
expectedQueue: DefaultQueueNum,
698+
description: "Zero QueueNum should be replaced with default",
699+
},
700+
}
701+
702+
for _, tt := range tests {
703+
t.Run(tt.name, func(t *testing.T) {
704+
// Create config with test values
705+
conf := &ConfYaml{}
706+
conf.Core.WorkerNum = tt.workerNum
707+
conf.Core.QueueNum = tt.queueNum
708+
709+
// Apply the same logic as loadConfigFromViper
710+
if conf.Core.WorkerNum <= 0 {
711+
conf.Core.WorkerNum = int64(runtime.NumCPU())
712+
}
713+
if conf.Core.QueueNum <= 0 {
714+
conf.Core.QueueNum = DefaultQueueNum
715+
}
716+
717+
// Verify results
718+
assert.Equal(t, tt.expectedWorker, conf.Core.WorkerNum,
719+
"WorkerNum: %s", tt.description)
720+
assert.Equal(t, tt.expectedQueue, conf.Core.QueueNum,
721+
"QueueNum: %s", tt.description)
722+
723+
// Ensure values are always positive (safety check)
724+
assert.True(t, conf.Core.WorkerNum > 0,
725+
"WorkerNum should be positive to prevent panics")
726+
assert.True(t, conf.Core.QueueNum > 0,
727+
"QueueNum should be positive to prevent panics")
728+
})
729+
}
730+
}
731+
732+
// TestConfigSafetyBoundaries tests edge cases for configuration safety
733+
func TestConfigSafetyBoundaries(t *testing.T) {
734+
tests := []struct {
735+
name string
736+
value int64
737+
expected int64
738+
usesCPU bool
739+
}{
740+
{"very_negative", -9999, DefaultQueueNum, false},
741+
{"negative_one", -1, DefaultQueueNum, false},
742+
{"zero", 0, DefaultQueueNum, false},
743+
{"positive_one", 1, 1, false},
744+
}
745+
746+
for _, tt := range tests {
747+
t.Run(tt.name+"_queue", func(t *testing.T) {
748+
conf := &ConfYaml{}
749+
conf.Core.QueueNum = tt.value
750+
conf.Core.WorkerNum = 1 // Set to valid positive value
751+
752+
// Apply safety logic
753+
if conf.Core.QueueNum <= 0 {
754+
conf.Core.QueueNum = DefaultQueueNum
755+
}
756+
757+
assert.Equal(t, tt.expected, conf.Core.QueueNum)
758+
assert.True(t, conf.Core.QueueNum > 0, "QueueNum must be positive")
759+
})
760+
}
761+
}

0 commit comments

Comments
 (0)