Skip to content

Commit 308cb67

Browse files
authored
Add safeguard against integer overflow
This PR adds a protection mechanism against integer overflow. This might occur internally when dealing with extremely large time values, such as `9223372036854775808m` (~17 trillion years). If overflow occurs, the CLI tool fails rather ungracefully, but I guess that’s fine considering the overall likelihood of this issue in practice.
1 parent af7aaba commit 308cb67

File tree

10 files changed

+131
-23
lines changed

10 files changed

+131
-23
lines changed

.github/benchmark.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ func main() {
2525
// Generate records
2626
date := klog.Ɀ_Date_(0, 1, 1)
2727
for i := 0; i < iterations; i++ {
28+
if date.IsEqualTo(klog.Ɀ_Date_(9999, 12, 31)) {
29+
// Prevent date overflow
30+
date = klog.Ɀ_Date_(0, 1, 1)
31+
}
2832
date = date.PlusDays(1)
2933
r := klog.NewRecord(date)
3034

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/alecthomas/kong v1.8.0
88
github.com/jotaen/genie v0.0.1
99
github.com/jotaen/kong-completion v0.0.6
10+
github.com/jotaen/safemath v0.0.1
1011
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
1112
github.com/posener/complete v1.2.3
1213
github.com/stretchr/testify v1.10.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ cloud.google.com/go v0.118.2 h1:bKXO7RXMFDkniAAvvuMrAPtQ/VHrs9e7J5UT3yrGdTY=
22
cloud.google.com/go v0.118.2/go.mod h1:CFO4UPEPi8oV21xoezZCrd3d81K4fFkDTEJu4R8K+9M=
33
github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0=
44
github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
5-
github.com/alecthomas/kong v1.7.0 h1:MnT8+5JxFDCvISeI6vgd/mFbAJwueJ/pqQNzZMsiqZE=
6-
github.com/alecthomas/kong v1.7.0/go.mod h1:p2vqieVMeTAnaC83txKtXe8FLke2X07aruPWXyMPQrU=
75
github.com/alecthomas/kong v1.8.0 h1:LEDIdSYrHU+4oTF2BL0NAfw++wH6lg/LzAJodTkLikM=
86
github.com/alecthomas/kong v1.8.0/go.mod h1:p2vqieVMeTAnaC83txKtXe8FLke2X07aruPWXyMPQrU=
97
github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc=
@@ -25,6 +23,8 @@ github.com/jotaen/genie v0.0.1 h1:gURxhYIpVEJ7SKjjNRDLV5OrgMxCbkAdWhjD86ad9P8=
2523
github.com/jotaen/genie v0.0.1/go.mod h1:bu+PbJDEJ9915yp4xml7OXoM4iBsSDfgtGVwv5Ag0Gg=
2624
github.com/jotaen/kong-completion v0.0.6 h1:VP1KGvXPeB7MytYR+zZQoWw1gf/HIV1/EvWC38BHZN4=
2725
github.com/jotaen/kong-completion v0.0.6/go.mod h1:fuWw9snL6joY5mXbI0Dd5FWEZODaWXAeqaRxo6dAvLk=
26+
github.com/jotaen/safemath v0.0.1 h1:YcUhSIUtwQY1rUUT3AeP+alzTHUAsM4Pap8ZMn3GOlc=
27+
github.com/jotaen/safemath v0.0.1/go.mod h1:KlKBnI3qvGcr3+iuvp3vABBZNFRjRcwRUVQa/jM38xQ=
2828
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
2929
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
3030
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

klog/date.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func NewDateFromGo(t gotime.Time) Date {
9898
d, err := NewDate(t.Year(), int(t.Month()), t.Day())
9999
if err != nil {
100100
// This can/should never occur
101-
panic("ILLEGAL_DATE")
101+
panic("Illegal date")
102102
}
103103
return d
104104
}

klog/duration.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"fmt"
66
"regexp"
77
"strconv"
8+
9+
"github.com/jotaen/safemath/safemath"
810
)
911

1012
// Duration represents a time span.
@@ -49,7 +51,12 @@ func NewDuration(amountHours int, amountMinutes int) Duration {
4951
}
5052

5153
func NewDurationWithFormat(amountHours int, amountMinutes int, format DurationFormat) Duration {
52-
return &duration{minutes: amountHours*60 + amountMinutes, format: format}
54+
hoursToMins, err1 := safemath.Multiply(amountHours, 60)
55+
totalMins, err2 := safemath.Add(hoursToMins, amountMinutes)
56+
if err1 != nil || err2 != nil {
57+
panic("Integer overflow")
58+
}
59+
return &duration{minutes: totalMins, format: format}
5360
}
5461

5562
type duration struct {
@@ -69,11 +76,15 @@ func (d duration) InMinutes() int {
6976
}
7077

7178
func (d duration) Plus(additional Duration) Duration {
72-
return NewDuration(0, d.InMinutes()+additional.InMinutes())
79+
mins, err := safemath.Add(d.InMinutes(), additional.InMinutes())
80+
if err != nil {
81+
panic("Integer overflow")
82+
}
83+
return NewDuration(0, mins)
7384
}
7485

7586
func (d duration) Minus(deductible Duration) Duration {
76-
return NewDuration(0, d.InMinutes()-deductible.InMinutes())
87+
return d.Plus(NewDuration(0, deductible.InMinutes()*-1))
7788
}
7889

7990
func (d duration) ToString() string {
@@ -129,9 +140,15 @@ func NewDurationFromString(hhmm string) (Duration, error) {
129140
if match[3] == "" && match[5] == "" {
130141
return nil, errors.New("MALFORMED_DURATION")
131142
}
132-
amountOfHours, _ := strconv.Atoi(match[3])
133-
amountOfMinutes, _ := strconv.Atoi(match[5])
134-
if amountOfHours != 0 && amountOfMinutes >= 60 {
143+
amountOfHours, a1Err := strconv.Atoi(match[3])
144+
if match[3] != "" && a1Err != nil {
145+
panic(a1Err)
146+
}
147+
amountOfMinutes, a2Err := strconv.Atoi(match[5])
148+
if match[5] != "" && a2Err != nil {
149+
panic(a2Err)
150+
}
151+
if match[3] != "" && amountOfMinutes >= 60 {
135152
return nil, errors.New("UNREPRESENTABLE_DURATION")
136153
}
137154
if amountOfHours == 0 && amountOfMinutes == 0 && match[1] != "" {

klog/duration_test.go

Lines changed: 96 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,33 @@ package klog
22

33
import (
44
"github.com/stretchr/testify/assert"
5+
"github.com/stretchr/testify/require"
56
"testing"
67
)
78

89
func TestSerialiseDurationOnlyWithMeaningfulValues(t *testing.T) {
910
assert.Equal(t, "0m", NewDuration(0, 0).ToString())
1011
assert.Equal(t, "1m", NewDuration(0, 1).ToString())
12+
assert.Equal(t, "34m", NewDuration(0, 34).ToString())
13+
assert.Equal(t, "59m", NewDuration(0, 59).ToString())
14+
assert.Equal(t, "1h", NewDuration(1, 0).ToString())
1115
assert.Equal(t, "15h", NewDuration(15, 0).ToString())
12-
}
13-
14-
func TestSerialiseDurationOfLargeHourValues(t *testing.T) {
16+
assert.Equal(t, "15h3m", NewDuration(15, 3).ToString())
1517
assert.Equal(t, "265h45m", NewDuration(265, 45).ToString())
18+
assert.Equal(t, "4716278h48m", NewDuration(4716278, 48).ToString())
19+
assert.Equal(t, "153722867280912930h7m", NewDuration(0, 9223372036854775807).ToString())
1620
}
1721

1822
func TestSerialiseDurationWithoutLeadingZeros(t *testing.T) {
1923
assert.Equal(t, "2h6m", NewDuration(2, 6).ToString())
2024
}
2125

2226
func TestSerialiseDurationOfNegativeValues(t *testing.T) {
27+
assert.Equal(t, "-2h4m", NewDuration(-2, -4).ToString())
2328
assert.Equal(t, "-3h18m", NewDuration(-3, -18).ToString())
24-
assert.Equal(t, "-3h", NewDuration(-3, 0).ToString())
29+
assert.Equal(t, "-812747h", NewDuration(-812747, 0).ToString())
2530
assert.Equal(t, "-18m", NewDuration(0, -18).ToString())
31+
assert.Equal(t, "-153722867280912930h7m", NewDuration(0, -9223372036854775807).ToString())
2632
}
2733

2834
func TestSerialiseDurationWithSign(t *testing.T) {
@@ -70,13 +76,19 @@ func TestParsingDurationWithHoursAndMinutes(t *testing.T) {
7076
}
7177

7278
func TestParsingDurationWithHourValueOnly(t *testing.T) {
73-
for _, d := range []string{
74-
"13h",
75-
"13h0m",
79+
for _, d := range []struct {
80+
text string
81+
expect Duration
82+
}{
83+
{"0h", NewDuration(0, 0)},
84+
{"1h", NewDuration(1, 0)},
85+
{"13h", NewDuration(13, 0)},
86+
{"9882187612h", NewDuration(9882187612, 0)},
87+
{"13h0m", NewDuration(13, 0)},
7688
} {
77-
duration, err := NewDurationFromString(d)
89+
duration, err := NewDurationFromString(d.text)
7890
assert.Nil(t, err)
79-
assert.Equal(t, NewDuration(13, 0), duration)
91+
assert.Equal(t, d.expect, duration)
8092
}
8193
}
8294

@@ -85,12 +97,16 @@ func TestParsingDurationWithMinuteValueOnly(t *testing.T) {
8597
text string
8698
expect Duration
8799
}{
100+
{"1m", NewDuration(0, 1)},
88101
{"48m", NewDuration(0, 48)},
102+
{"59m", NewDuration(0, 59)},
103+
89104
{"0h48m", NewDuration(0, 48)},
90105

91106
// Minutes >60 are okay if there is no hour part present
107+
{"60m", NewDuration(1, 0)},
92108
{"120m", NewDuration(2, 0)},
93-
{"150m", NewDuration(2, 30)},
109+
{"568721940327m", NewDuration(0, 568721940327)},
94110
} {
95111
duration, err := NewDurationFromString(d.text)
96112
assert.Nil(t, err)
@@ -143,6 +159,7 @@ func TestParsingFailsWithInvalidValue(t *testing.T) {
143159
func TestParsingFailsWithMinutesGreaterThan60WhenHourPartPresent(t *testing.T) {
144160
for _, d := range []string{
145161
"1h60m",
162+
"0h60m",
146163
"8h1653m",
147164
"-8h1653m",
148165
} {
@@ -151,3 +168,72 @@ func TestParsingFailsWithMinutesGreaterThan60WhenHourPartPresent(t *testing.T) {
151168
assert.Equal(t, nil, duration)
152169
}
153170
}
171+
172+
func TestParsingDurationWithMaxValue(t *testing.T) {
173+
t.Run("max", func(t *testing.T) {
174+
d, err := NewDurationFromString("9223372036854775807m")
175+
require.Nil(t, err)
176+
assert.Equal(t, NewDuration(0, 9223372036854775807), d)
177+
})
178+
t.Run("max", func(t *testing.T) {
179+
d, err := NewDurationFromString("153722867280912930h7m")
180+
require.Nil(t, err)
181+
assert.Equal(t, NewDuration(153722867280912930, 7), d)
182+
})
183+
t.Run("min", func(t *testing.T) {
184+
d, err := NewDurationFromString("-9223372036854775807m")
185+
require.Nil(t, err)
186+
assert.Equal(t, NewDuration(0, -9223372036854775807), d)
187+
})
188+
t.Run("max", func(t *testing.T) {
189+
d, err := NewDurationFromString("-153722867280912930h7m")
190+
require.Nil(t, err)
191+
assert.Equal(t, NewDuration(-153722867280912930, -7), d)
192+
})
193+
}
194+
195+
func TestParsingDurationTooBigToRepresent(t *testing.T) {
196+
for _, d := range []string{
197+
"9223372036854775808m",
198+
"-9223372036854775808m",
199+
"9223372036854775808h",
200+
"-9223372036854775808h",
201+
"153722867280912930h08m",
202+
"-153722867280912930h08m",
203+
} {
204+
assert.Panics(t, func() {
205+
_, _ = NewDurationFromString(d)
206+
}, d)
207+
}
208+
}
209+
210+
func TestDurationPlusMinus(t *testing.T) {
211+
for _, d := range []struct {
212+
sum Duration
213+
expect int
214+
}{
215+
{NewDuration(0, 0).Plus(NewDuration(0, 0)), 0},
216+
{NewDuration(0, 0).Plus(NewDuration(0, 1)), 1},
217+
{NewDuration(0, 0).Plus(NewDuration(1, 2)), 62},
218+
{NewDuration(1382, 9278).Plus(NewDuration(4718, 5010)), 380288},
219+
{NewDuration(0, 9223372036854775806).Plus(NewDuration(0, 1)), 9223372036854775807},
220+
{NewDuration(0, 0).Plus(NewDuration(0, -9223372036854775807)), -9223372036854775807},
221+
222+
{NewDuration(0, 0).Minus(NewDuration(0, 0)), 0},
223+
{NewDuration(0, 0).Minus(NewDuration(0, 1)), -1},
224+
{NewDuration(0, 0).Minus(NewDuration(1, 2)), -62},
225+
{NewDuration(1382, 9278).Minus(NewDuration(4718, 5010)), -195892},
226+
} {
227+
assert.Equal(t, d.sum.InMinutes(), d.expect)
228+
}
229+
}
230+
231+
func TestPanicsIfAdditionOverflows(t *testing.T) {
232+
assert.Panics(t, func() {
233+
NewDuration(0, 9223372036854775807).Plus(NewDuration(0, 1))
234+
})
235+
236+
assert.Panics(t, func() {
237+
NewDuration(0, -9223372036854775807).Plus(NewDuration(0, -1))
238+
})
239+
}

klog/parser/engine/parallel.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type batchResult[T any] struct {
2323

2424
func (p ParallelBatchParser[T]) Parse(text string) ([]T, []txt.Block, []txt.Error) {
2525
if p.NumberOfWorkers <= 0 {
26-
panic("ILLEGAL_WORKER_SIZE")
26+
panic("Illegal number of workers")
2727
}
2828
batches := splitIntoChunks(text, p.NumberOfWorkers)
2929
allResults := p.processAsync(batches, func(batchIndex int, batchText string) batchResult[T] {

klog/parser/reconciling/start_open_range.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func (r *Reconciler) StartOpenRange(startTime klog.Time, format ReformatDirectiv
1414
// Re-parse time to apply format.
1515
reformattedTime, err := klog.NewTimeFromString(startTime.ToStringWithFormat(f))
1616
if err != nil {
17-
panic("INVALID_TIME")
17+
panic("Invalid time")
1818
}
1919
startTime = reformattedTime
2020
})

klog/parser/reconciling/style.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func ascertain[T comparable](e *election[T], defaultStyle styleProp[T]) stylePro
133133
// `base` style that had been set explicitly take precedence.
134134
func elect(base style, rs []klog.Record, bs []txt.Block) *style {
135135
if len(rs) != len(bs) {
136-
panic("ASSERTION_ERROR")
136+
panic("Internal error")
137137
}
138138
lineEndingElection := newElection[string]()
139139
indentationElection := newElection[string]()

klog/time.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func NewTimeFromGo(t gotime.Time) Time {
128128
time, err := NewTime(t.Hour(), t.Minute())
129129
if err != nil {
130130
// This can/should never occur
131-
panic("ILLEGAL_TIME")
131+
panic("Illegal time")
132132
}
133133
return time
134134
}

0 commit comments

Comments
 (0)