fix: do not corrupt state when Add/Set overshoots max
Add64 checked currentNum > max only after already mutating currentNum and currentBytes, so a single call that overshoots max returned an error but left the bar stuck: currentNum frozen above max, percent stuck above 100%, and every later Add/Set call failing with the same error forever, since the top-of-function currentNum < max guard could never pass again. Check the bound before mutating state instead, so an overshooting call is rejected as a whole and leaves currentNum/currentBytes/currentPercent exactly as they were. The now-unreachable post-hoc check is removed. Add TestAddExceedsMaxDoesNotCorruptState covering the corrupted-state case and that the bar keeps accepting valid progress afterwards.
This commit is contained in:
1 parent
b1e7a8656a
commit
cf25c2235d
2 files changed
+32
-3
No files matched your search
+4
-3
@@ -741,6 +741,10 @@ func (p *ProgressBar) Add64(num int64) error {
|
|||||||
return errors.New("max must be greater than 0")
|
return errors.New("max must be greater than 0")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !p.config.ignoreLength && p.state.currentNum+num > p.config.max {
|
||||||
|
return errors.New("current number exceeds max")
|
||||||
|
}
|
||||||
|
|
||||||
if p.state.currentNum < p.config.max {
|
if p.state.currentNum < p.config.max {
|
||||||
if p.config.ignoreLength {
|
if p.config.ignoreLength {
|
||||||
p.state.currentNum = (p.state.currentNum + num) % p.config.max
|
p.state.currentNum = (p.state.currentNum + num) % p.config.max
|
||||||
@@ -774,9 +778,6 @@ func (p *ProgressBar) Add64(num int64) error {
|
|||||||
updateBar := p.state.currentPercent != p.state.lastPercent && p.state.currentPercent > 0
|
updateBar := p.state.currentPercent != p.state.lastPercent && p.state.currentPercent > 0
|
||||||
|
|
||||||
p.state.lastPercent = p.state.currentPercent
|
p.state.lastPercent = p.state.currentPercent
|
||||||
if p.state.currentNum > p.config.max {
|
|
||||||
return errors.New("current number exceeds max")
|
|
||||||
}
|
|
||||||
|
|
||||||
// always update if show bytes/second or its/second
|
// always update if show bytes/second or its/second
|
||||||
if updateBar || p.config.showIterationsPerSecond || p.config.showIterationsCount {
|
if updateBar || p.config.showIterationsPerSecond || p.config.showIterationsCount {
|
||||||
|
|||||||
@@ -420,6 +420,34 @@ func TestBar(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAddExceedsMaxDoesNotCorruptState(t *testing.T) {
|
||||||
|
bar := NewOptions(100, OptionSetWriter(io.Discard))
|
||||||
|
|
||||||
|
if err := bar.Add(95); err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := bar.Add(10); err == nil {
|
||||||
|
t.Error("should have an error for adding past max")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A rejected Add must leave state untouched, not partially applied.
|
||||||
|
if bar.state.currentNum != 95 {
|
||||||
|
t.Errorf("expected currentNum to stay at 95 after a rejected Add, got %d", bar.state.currentNum)
|
||||||
|
}
|
||||||
|
if bar.state.currentPercent != 95 {
|
||||||
|
t.Errorf("expected currentPercent to stay at 95 after a rejected Add, got %d", bar.state.currentPercent)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The bar must still accept further, non-overshooting progress afterwards.
|
||||||
|
if err := bar.Add(5); err != nil {
|
||||||
|
t.Errorf("bar should still accept progress after a rejected Add, got error: %v", err)
|
||||||
|
}
|
||||||
|
if bar.state.currentNum != 100 {
|
||||||
|
t.Errorf("expected currentNum to reach 100, got %d", bar.state.currentNum)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestState(t *testing.T) {
|
func TestState(t *testing.T) {
|
||||||
bar := NewOptions(100, OptionSetWidth(10))
|
bar := NewOptions(100, OptionSetWidth(10))
|
||||||
bar.Reset()
|
bar.Reset()
|
||||||
|
|||||||
Reference in new issue
Block a user