From c6327a9b1156afa1611fcdec2aa8ab095ab58733 Mon Sep 17 00:00:00 2001 From: ccoVeille <3875889+ccoVeille@users.noreply.github.com> Date: Sun, 8 Sep 2024 21:32:07 +0200 Subject: [PATCH] fix: float overflow when converted to integer Only uint64 and uint was working as expected Fixes #23 --- README.md | 42 +++++++++++++++++++----- asserters.go | 62 +++++++++++++++++++++++++++++++++-- conversion.go | 81 +++++++++++++++++++++++----------------------- conversion_test.go | 20 +++++++++--- errors.go | 2 +- 5 files changed, 150 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index a602b3e..e00a6f0 100644 --- a/README.md +++ b/README.md @@ -18,20 +18,46 @@ package main import ( "fmt" + "math" "github.com/ccoveille/go-safecast" ) func main() { - _, err := safecast.ToInt8(uint16(1000)) - fmt.Println(err) // integer overflow: 1000 (uint16) is greater than 127 (int8): exceed upper boundary for this type - _, err = safecast.ToUint16(int64(-1)) - fmt.Println(err) // integer overflow: -1 (int64) is smaller than 0 (uint16): exceed lower boundary for this type + // when there is no overflow + // + fmt.Println(safecast.ToInt8(float64(42))) + // Output: 42, nil + fmt.Println(safecast.ToInt8(int64(-1))) + // Output: -1, nil + + // when there is an overflow + // + fmt.Println(safecast.ToInt8(float64(20000))) + // Output: 0 conversion issue: 20000 is greater than 127 + fmt.Println(safecast.ToUint8(int64(-1))) + // Output: 0 conversion issue: -1 is negative + fmt.Println(safecast.ToInt16(int32(40000))) + // Output: 0 conversion issue: 40000 is greater than 32767 + fmt.Println(safecast.ToUint16(int64(-1))) + // Output: 0 conversion issue: -1 is negative + fmt.Println(safecast.ToInt32(math.MaxUint32 + 1)) + // Output: 0 conversion issue: 4294967296 is greater than 2147483647 + fmt.Println(safecast.ToUint32(int64(-1))) + // Output: 0 conversion issue: -1 is negative + fmt.Println(safecast.ToInt64(uint64(math.MaxInt64) + 1)) + // Output: 0 conversion issue: 9223372036854775808 is greater than 9223372036854775807 + fmt.Println(safecast.ToUint64(int8(-1))) + // Output: 0 conversion issue: -1 is negative + fmt.Println(safecast.ToInt(uint64(math.MaxInt) + 1)) + // Output: 0 conversion issue: 9223372036854775808 is greater than 9223372036854775807 + fmt.Println(safecast.ToUint(int8(-1))) + // Output: 0 conversion issue: -1 is negative } ``` -[Go Playground](https://go.dev/play/p/ciic-m0cdfb) +[Go Playground](https://go.dev/play/p/VCrv1aLJjMQ) ## Conversion overflows @@ -54,11 +80,11 @@ func main() { a = 255 + 1 b = uint8(a) - fmt.Println(b) // 0 integer overflow + fmt.Println(b) // 0 conversion overflow a = -1 b = uint8(a) - fmt.Println(b) // 255 integer overflow + fmt.Println(b) // 255 conversion overflow } ``` @@ -68,7 +94,7 @@ func main() { The gosec project raised this to my attention when the gosec [G115 rule was added](https://github.com/securego/gosec/pull/1149) -> G115: Potential integer overflow when converting between integer types. +> G115: Potential overflow when converting between integer types. This issue was way more complex than expected, and required multiple fixes. diff --git a/asserters.go b/asserters.go index c4ab60e..6d47981 100644 --- a/asserters.go +++ b/asserters.go @@ -2,9 +2,65 @@ package safecast import "fmt" -func assertNotNegative[T Type](i T) error { - if i < 0 { - return fmt.Errorf("%w: %v is negative", ErrOutOfRange, i) +func assertNotNegative[T Type](value T) error { + if value < 0 { + return fmt.Errorf("%w: %v is negative", ErrConversionIssue, value) } return nil } + +func checkUpperBoundary[T Type](value T, boundary uint64) error { + if value <= 0 { + return nil + } + + var greater bool + switch f := any(value).(type) { + case float64: + // for float64, everything fits in float64 without overflow. + // We are using a greater or equal because float cannot be compared easily because of precision loss. + greater = f >= float64(boundary) + case float32: + // everything fits in float32, except float64 greater than math.MaxFloat32. + // So, we must convert to float64 and check. + // We are using a greater or equal because float cannot be compared easily because of precision loss. + greater = float64(f) >= float64(boundary) + default: + // for all other integer types, it fits in an uint64 without overflow as we know value is positive. + greater = uint64(value) > boundary + } + + if greater { + return fmt.Errorf("%w: %v is greater than %v", ErrConversionIssue, value, boundary) + } + + return nil +} + +func checkLowerBoundary[T Type](value T, boundary int64) error { + if value >= 0 { + return nil + } + + var smaller bool + switch f := any(value).(type) { + case float64: + // everything fits in float64 without overflow. + // We are using a lower or equal because float cannot be compared easily because of precision loss. + smaller = f <= float64(boundary) + case float32: + // everything fits in float32, except float64 smaller than -math.MaxFloat32. + // So, we must convert to float64 and check. + // We are using a lower or equal because float cannot be compared easily because of precision loss. + smaller = float64(f) <= float64(boundary) + default: + // for all other integer types, it fits in an int64 without overflow as we know value is negative. + smaller = int64(value) < boundary + } + + if smaller { + return fmt.Errorf("%w: %v is less than %v", ErrConversionIssue, value, boundary) + } + + return nil +} diff --git a/conversion.go b/conversion.go index 307c11c..a893c10 100644 --- a/conversion.go +++ b/conversion.go @@ -4,17 +4,18 @@ package safecast -import ( - "fmt" - "math" -) +import "math" // ToInt attempts to convert any [Type] value to an int. // If the conversion results in a value outside the range of an int, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToInt[T Type](i T) (int, error) { - if i > 0 && uint64(i) > math.MaxInt { - return 0, fmt.Errorf("%w: %v is greater than math.MaxInt", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxInt); err != nil { + return 0, err + } + + if err := checkLowerBoundary(i, math.MinInt); err != nil { + return 0, err } return int(i), nil @@ -22,14 +23,14 @@ func ToInt[T Type](i T) (int, error) { // ToUint attempts to convert any [Type] value to an uint. // If the conversion results in a value outside the range of an uint, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToUint[T Type](i T) (uint, error) { if err := assertNotNegative(i); err != nil { return 0, err } - if float64(i) > math.MaxUint64 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxUint64", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxUint64); err != nil { + return 0, err } return uint(i), nil @@ -37,14 +38,14 @@ func ToUint[T Type](i T) (uint, error) { // ToInt8 attempts to convert any [Type] value to an int8. // If the conversion results in a value outside the range of an int8, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToInt8[T Type](i T) (int8, error) { - if i > math.MaxInt8 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxInt8", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxInt8); err != nil { + return 0, err } - if i < 0 && uint64(-i) > -math.MinInt8 { - return 0, fmt.Errorf("%w: %v is less than math.MinInt8", ErrOutOfRange, i) + if err := checkLowerBoundary(i, math.MinInt8); err != nil { + return 0, err } return int8(i), nil @@ -52,14 +53,14 @@ func ToInt8[T Type](i T) (int8, error) { // ToUint8 attempts to convert any [Type] value to an uint8. // If the conversion results in a value outside the range of an uint8, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToUint8[T Type](i T) (uint8, error) { if err := assertNotNegative(i); err != nil { return 0, err } - if uint64(i) > math.MaxUint8 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxUint8", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxUint8); err != nil { + return 0, err } return uint8(i), nil @@ -67,14 +68,14 @@ func ToUint8[T Type](i T) (uint8, error) { // ToInt16 attempts to convert any [Type] value to an int16. // If the conversion results in a value outside the range of an int16, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToInt16[T Type](i T) (int16, error) { - if i > 0 && uint64(i) > math.MaxInt16 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxInt16", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxInt16); err != nil { + return 0, err } - if i < 0 && uint64(-i) > -math.MinInt16 { - return 0, fmt.Errorf("%w: %v is less than math.MinInt16", ErrOutOfRange, i) + if err := checkLowerBoundary(i, math.MinInt16); err != nil { + return 0, err } return int16(i), nil @@ -82,14 +83,14 @@ func ToInt16[T Type](i T) (int16, error) { // ToUint16 attempts to convert any [Type] value to an uint16. // If the conversion results in a value outside the range of an uint16, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToUint16[T Type](i T) (uint16, error) { if err := assertNotNegative(i); err != nil { return 0, err } - if uint64(i) > math.MaxUint16 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxUint16", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxUint16); err != nil { + return 0, err } return uint16(i), nil @@ -97,14 +98,14 @@ func ToUint16[T Type](i T) (uint16, error) { // ToInt32 attempts to convert any [Type] value to an int32. // If the conversion results in a value outside the range of an int32, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToInt32[T Type](i T) (int32, error) { - if i > 0 && uint64(i) > math.MaxInt32 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxInt32", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxInt32); err != nil { + return 0, err } - if i < 0 && uint64(-i) > -math.MinInt32 { - return 0, fmt.Errorf("%w: %v is less than math.MinInt32", ErrOutOfRange, i) + if err := checkLowerBoundary(i, math.MinInt32); err != nil { + return 0, err } return int32(i), nil @@ -112,14 +113,14 @@ func ToInt32[T Type](i T) (int32, error) { // ToUint32 attempts to convert any [Type] value to an uint32. // If the conversion results in a value outside the range of an uint32, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToUint32[T Type](i T) (uint32, error) { if err := assertNotNegative(i); err != nil { return 0, err } - if uint64(i) > math.MaxUint32 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxUint32", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxUint32); err != nil { + return 0, err } return uint32(i), nil @@ -127,10 +128,10 @@ func ToUint32[T Type](i T) (uint32, error) { // ToInt64 attempts to convert any [Type] value to an int64. // If the conversion results in a value outside the range of an int64, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToInt64[T Type](i T) (int64, error) { - if i > 0 && uint64(i) > math.MaxInt64 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxInt64", ErrOutOfRange, i) + if err := checkUpperBoundary(i, math.MaxInt64); err != nil { + return 0, err } return int64(i), nil @@ -138,14 +139,14 @@ func ToInt64[T Type](i T) (int64, error) { // ToUint64 attempts to convert any [Type] value to an uint64. // If the conversion results in a value outside the range of an uint64, -// an ErrOutOfRange error is returned. +// an ErrConversionOverflow error is returned. func ToUint64[T Type](i T) (uint64, error) { if err := assertNotNegative(i); err != nil { return 0, err } - if float64(i) > math.MaxUint64 { - return 0, fmt.Errorf("%w: %v is greater than math.MaxUint64", ErrOutOfRange, i) + if err := checkUpperBoundary(i, uint64(math.MaxUint64)); err != nil { + return 0, err } return uint64(i), nil diff --git a/conversion_test.go b/conversion_test.go index ecffa43..475b528 100644 --- a/conversion_test.go +++ b/conversion_test.go @@ -27,7 +27,7 @@ func requireError(t *testing.T, err error) { t.Fatal("expected error") } - if !errors.Is(err, safecast.ErrOutOfRange) { + if !errors.Is(err, safecast.ErrConversionIssue) { t.Errorf("expected out of range error, got %v", err) } } @@ -1272,6 +1272,8 @@ func TestToUint64(t *testing.T) { }) assertUint64Error(t, []caseUint64[float32]{ + {name: "negative value", input: -1}, + {name: "out of range max uint64", input: math.MaxUint64}, {name: "out of range max float32", input: math.MaxFloat32}, }) }) @@ -1284,6 +1286,8 @@ func TestToUint64(t *testing.T) { }) assertUint64Error(t, []caseUint64[float64]{ + {name: "negative value", input: -1}, + {name: "out of range max uint64", input: math.MaxUint64}, {name: "out of range max float32", input: math.MaxFloat32}, {name: "out of range max float64", input: math.MaxFloat64}, }) @@ -1412,7 +1416,8 @@ func TestToInt(t *testing.T) { }) assertIntError(t, []caseInt[float32]{ - {name: "positive out of range", input: math.MaxFloat32 + 1}, + {name: "positive out of range", input: math.MaxFloat32}, + {name: "negative out of range", input: -math.MaxFloat32}, }) }) @@ -1423,8 +1428,9 @@ func TestToInt(t *testing.T) { {name: "positive within range", input: 10000.9, want: 10000}, }) - assertIntError(t, []caseInt[float32]{ - {name: "positive out of range", input: math.MaxFloat32 + 1}, + assertIntError(t, []caseInt[float64]{ + {name: "positive out of range", input: math.MaxFloat32}, + {name: "negative out of range", input: -math.MaxFloat32}, }) }) } @@ -1558,7 +1564,9 @@ func TestToUint(t *testing.T) { }) assertUint64Error(t, []caseUint64[float32]{ - {name: "out of range", input: math.MaxFloat32}, + {name: "negative value", input: -1}, + {name: "out of range max uint64", input: math.MaxUint64}, + {name: "out of range max float32", input: math.MaxFloat32}, }) }) @@ -1570,6 +1578,8 @@ func TestToUint(t *testing.T) { }) assertUintError(t, []caseUint[float64]{ + {name: "negative value", input: -1}, + {name: "out of range max uint64", input: math.MaxUint64}, {name: "out of range max float32", input: math.MaxFloat32}, {name: "out of range max float64", input: math.MaxFloat64}, }) diff --git a/errors.go b/errors.go index 1f50029..4696535 100644 --- a/errors.go +++ b/errors.go @@ -2,4 +2,4 @@ package safecast import "errors" -var ErrOutOfRange = errors.New("out of range") +var ErrConversionIssue = errors.New("conversion issue")