Skip to content

Commit 0212844

Browse files
jotaenjotaen
committed
Add safeguard against integer overflow
1 parent af7aaba commit 0212844

File tree

14 files changed

+350
-22
lines changed

14 files changed

+350
-22
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

‎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: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package klog
33
import (
44
"errors"
55
"fmt"
6+
"github.com/jotaen/klog/klog/service/safemath"
67
"regexp"
78
"strconv"
89
)
@@ -49,7 +50,12 @@ func NewDuration(amountHours int, amountMinutes int) Duration {
4950
}
5051

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

5561
type duration struct {
@@ -69,11 +75,15 @@ func (d duration) InMinutes() int {
6975
}
7076

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

7585
func (d duration) Minus(deductible Duration) Duration {
76-
return NewDuration(0, d.InMinutes()-deductible.InMinutes())
86+
return d.Plus(NewDuration(0, deductible.InMinutes()*-1))
7787
}
7888

7989
func (d duration) ToString() string {
@@ -129,9 +139,15 @@ func NewDurationFromString(hhmm string) (Duration, error) {
129139
if match[3] == "" && match[5] == "" {
130140
return nil, errors.New("MALFORMED_DURATION")
131141
}
132-
amountOfHours, _ := strconv.Atoi(match[3])
133-
amountOfMinutes, _ := strconv.Atoi(match[5])
134-
if amountOfHours != 0 && amountOfMinutes >= 60 {
142+
amountOfHours, a1Err := strconv.Atoi(match[3])
143+
if match[3] != "" && a1Err != nil {
144+
panic(a1Err)
145+
}
146+
amountOfMinutes, a2Err := strconv.Atoi(match[5])
147+
if match[5] != "" && a2Err != nil {
148+
panic(a2Err)
149+
}
150+
if match[3] != "" && amountOfMinutes >= 60 {
135151
return nil, errors.New("UNREPRESENTABLE_DURATION")
136152
}
137153
if amountOfHours == 0 && amountOfMinutes == 0 && match[1] != "" {

‎klog/duration_test.go‎

Lines changed: 97 additions & 11 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)
@@ -131,7 +147,7 @@ func TestParsingFailsWithInvalidValue(t *testing.T) {
131147
"6h asdf",
132148
"qwer 30m",
133149
"⠙⠛m", // Braille digits
134-
"四二h", // Japanese digits
150+
"四二h", // Japanese digits
135151
"᠒h᠐᠒m", // Mongolean digits
136152
} {
137153
duration, err := NewDurationFromString(d)
@@ -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/service/safemath/add.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package safemath
2+
3+
// Add calculates the sum of two integers.
4+
// It returns an error if the resulting integer would overflow.
5+
func Add(a int, b int) (int, error) {
6+
err := assertOperandInRange(a, b)
7+
if err != nil {
8+
return 0, err
9+
}
10+
11+
if b > 0 {
12+
if a > MaxInt-b {
13+
return 0, OverflowErr
14+
}
15+
} else {
16+
if a < MinInt-b {
17+
return 0, OverflowErr
18+
}
19+
}
20+
21+
return a + b, nil
22+
}

‎klog/service/safemath/add_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package safemath
2+
3+
import (
4+
"fmt"
5+
"github.com/stretchr/testify/assert"
6+
"testing"
7+
)
8+
9+
func TestAddNonOverflowing(t *testing.T) {
10+
for _, x := range []OpRes{
11+
{0, 0, 0},
12+
13+
{0, 1, 1},
14+
{0, 42, 42},
15+
{0, MaxInt, MaxInt},
16+
17+
{0, -1, -1},
18+
{0, -42, -42},
19+
{0, MinInt, MinInt},
20+
21+
{42, 42, 84},
22+
{1500000, 1500000, 3000000},
23+
24+
{MaxInt / 2, MaxInt/2 + 1, MaxInt},
25+
{MinInt / 2, MinInt/2 - 1, MinInt},
26+
27+
{MaxInt, -1, MaxInt - 1},
28+
{MinInt, 1, MinInt + 1},
29+
} {
30+
t.Run(fmt.Sprintf("%d %d (original)", x.a, x.b), func(t *testing.T) {
31+
res, err := Add(x.a, x.b)
32+
assert.Nil(t, err)
33+
assert.Equal(t, x.res, res)
34+
})
35+
t.Run(fmt.Sprintf("%d %d (commutative)", x.b, x.a), func(t *testing.T) {
36+
res, err := Add(x.b, x.a)
37+
assert.Nil(t, err)
38+
assert.Equal(t, x.res, res)
39+
})
40+
}
41+
}
42+
43+
func TestAddOverflowing(t *testing.T) {
44+
for _, x := range []OpErr{
45+
{1, MaxInt},
46+
{-1, MinInt},
47+
{MaxInt, MaxInt},
48+
{MaxInt/2 + 1, MaxInt/2 + 1},
49+
} {
50+
t.Run(fmt.Sprintf("%d %d (original)", x.a, x.b), func(t *testing.T) {
51+
res, err := Add(x.a, x.b)
52+
assert.Error(t, err)
53+
assert.Equal(t, 0, res)
54+
})
55+
t.Run(fmt.Sprintf("%d %d (commutative)", x.b, x.a), func(t *testing.T) {
56+
res, err := Add(x.b, x.a)
57+
assert.Error(t, err)
58+
assert.Equal(t, 0, res)
59+
})
60+
}
61+
}

‎klog/service/safemath/constants.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package safemath
2+
3+
import (
4+
"errors"
5+
"math"
6+
)
7+
8+
var (
9+
OverflowErr = errors.New("overflow")
10+
11+
// MaxInt represents the largest possible (positive) integer value.
12+
MaxInt = math.MaxInt
13+
// MinInt represents the smallest possible (negative) integer value.
14+
// It doesn’t fully exhaust the theoretical range, to be in line with the
15+
// MaxInt range, and to allow inverting values without worry.
16+
MinInt = math.MinInt + 1
17+
)
18+
19+
func assertOperandInRange(xs ...int) error {
20+
for _, x := range xs {
21+
if x < MinInt {
22+
return OverflowErr
23+
}
24+
}
25+
return nil
26+
}
27+
28+
func abs(x int) int {
29+
if x < 0 {
30+
return -1 * x
31+
}
32+
return x
33+
}

0 commit comments

Comments
 (0)