From 1d8c75202a514dde4c56727fa1ca63933af2727e Mon Sep 17 00:00:00 2001 From: Quang Date: Sun, 27 Oct 2024 15:39:26 +0700 Subject: [PATCH] fix: magic number in parsing (#18) --- .golangci.yaml | 1 + bint.go | 43 +++++++++++++++++++++---------------------- codec.go | 4 +--- decimal.go | 22 ++++++++++------------ decimal_test.go | 10 ---------- u128.go | 8 +++++++- u256.go | 2 +- 7 files changed, 41 insertions(+), 49 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index abcad3f..f0aee82 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -3,6 +3,7 @@ run: linters: enable: + - nolintlint - errcheck - gosimple - goimports diff --git a/bint.go b/bint.go index 8854a91..96499f4 100644 --- a/bint.go +++ b/bint.go @@ -1,11 +1,18 @@ package udecimal import ( + "bytes" "fmt" "math/big" "strings" ) +const ( + // maxDigitU64 is the maximum digits of a number + // that can be safely stored in a uint64. + maxDigitU64 = 19 +) + var ( bigZero = big.NewInt(0) bigOne = big.NewInt(1) @@ -171,7 +178,7 @@ func parseBint(s []byte) (bool, bint, uint8, error) { return false, bint{}, 0, errInvalidFormat(s) } - // nolint: gosec + //nolint:gosec return neg, bintFromBigInt(dValue), uint8(prec), nil } @@ -211,7 +218,7 @@ func parseBintFromU128(s []byte) (bool, bint, uint8, error) { prec uint8 ) - if len(s[pos:]) <= 19 { + if len(s[pos:]) <= maxDigitU64 { coef, prec, err = parseSmallToU128(s[pos:]) } else { coef, prec, err = parseLargeToU128(s[pos:]) @@ -237,7 +244,7 @@ func parseSmallToU128(s []byte) (u128, uint8, error) { return u128{}, 0, ErrInvalidFormat } - // nolint: gosec + //nolint:gosec prec = uint8(len(s) - i - 1) // prevent "123." or "-123." @@ -269,27 +276,12 @@ func parseSmallToU128(s []byte) (u128, uint8, error) { func parseLargeToU128(s []byte) (u128, uint8, error) { // find '.' position l := len(s) - pos := -1 - - for i := 0; i < len(s); i++ { - if s[i] == '.' { - pos = i - } - } - + pos := bytes.IndexByte(s, '.') if pos == 0 || pos == l-1 { + // prevent ".123" or "123." return u128{}, 0, ErrInvalidFormat } - var prec uint8 - if pos != -1 { - // nolint: gosec - prec = uint8(l - pos - 1) - if prec > defaultPrec { - return u128{}, 0, ErrPrecOutOfRange - } - } - if pos == -1 { // no decimal point coef, err := digitToU128(s) @@ -300,6 +292,13 @@ func parseLargeToU128(s []byte) (u128, uint8, error) { return coef, 0, nil } + // now 0 < pos < l-1 + //nolint:gosec + prec := uint8(l - pos - 1) + if prec > defaultPrec { + return u128{}, 0, ErrPrecOutOfRange + } + // number has a decimal point, split into 2 parts: integer and fraction intPart, err := digitToU128(s[:pos]) if err != nil { @@ -307,7 +306,7 @@ func parseLargeToU128(s []byte) (u128, uint8, error) { } // because max prec is 19, - // factionPart can't be larger than 10%20-1 and will fit into uint64 (fractionPart.hi == 0) + // factionPart can't be larger than 10^19-1 and will fit into uint64 (fractionPart.hi == 0) fractionPart, err := digitToU128(s[pos+1:]) if err != nil { return u128{}, 0, err @@ -328,7 +327,7 @@ func parseLargeToU128(s []byte) (u128, uint8, error) { } func digitToU128(s []byte) (u128, error) { - if len(s) <= 19 { + if len(s) <= maxDigitU64 { var u uint64 for i := 0; i < len(s); i++ { if s[i] < '0' || s[i] > '9' { diff --git a/codec.go b/codec.go index 08bf4b3..5f3c94e 100644 --- a/codec.go +++ b/codec.go @@ -166,7 +166,6 @@ func (d Decimal) fillBuffer(buf []byte, trimTrailingZeros bool) int { for ; rem != 0; rem /= 10 { n++ - // nolint: gosec buf[l-n] = byte(rem%10) + '0' } @@ -188,8 +187,7 @@ func (d Decimal) fillBuffer(buf []byte, trimTrailingZeros bool) int { q, r := quoRem64(quo, 10) n++ - // nolint: gosec - buf[l-n] = uint8(r%10) + '0' + buf[l-n] = byte(r%10) + '0' if q.IsZero() { break } diff --git a/decimal.go b/decimal.go index eeb660c..57da3a8 100644 --- a/decimal.go +++ b/decimal.go @@ -99,8 +99,9 @@ var ( ErrEmptyString = fmt.Errorf("parse empty string") // ErrMaxStrLen is returned when the input string exceeds the maximum length - // This limitation is set to prevent large string input which can cause performance issue - // Maximum length is set to 200 + // Maximum length is arbitrarily set to 200 so string length value can fit in 1 byte (for MarshalBinary). + // Also such that big number (more than 200 digits) is unrealistic in financial system + // which this library is mainly designed for. ErrMaxStrLen = fmt.Errorf("string input exceeds maximum length %d", maxStrLen) // ErrInvalidFormat is returned when the input string is not in the correct format @@ -213,7 +214,7 @@ func NewFromInt64(coef int64, prec uint8) (Decimal, error) { return Decimal{}, ErrPrecOutOfRange } - // nolint: gosec + //nolint:gosec return newDecimal(neg, bintFromU64(uint64(coef)), prec), nil } @@ -278,6 +279,7 @@ func (d Decimal) InexactFloat64() float64 { // Returns error if: // 1. empty/invalid string // 2. the number has more than 19 digits after the decimal point +// 3. string length exceeds maxStrLen (which is 200 characters. See [ErrMaxStrLen] for more details) func Parse(s string) (Decimal, error) { return parseBytes(unsafeStringToBytes(s)) } @@ -429,10 +431,6 @@ func (d Decimal) Sub64(e uint64) Decimal { // Mul returns d * e. // The result will have at most defaultPrec digits after the decimal point. func (d Decimal) Mul(e Decimal) Decimal { - if e.coef.IsZero() { - return Decimal{} - } - prec := d.prec + e.prec neg := d.neg != e.neg @@ -1149,7 +1147,7 @@ func (d Decimal) PowInt(e int) Decimal { neg = false } - // nolint: gosec + //nolint:gosec return newDecimal(neg, bintFromBigInt(qBig), uint8(powPrecision)) } @@ -1215,7 +1213,7 @@ func (d Decimal) tryPowIntU128(e int) (Decimal, error) { return Decimal{}, errOverflow } - // nolint: gosec + //nolint:gosec return newDecimal(neg, bintFromU128(u128{hi: result.hi, lo: result.lo}), uint8(powPrecision)), nil } @@ -1269,7 +1267,7 @@ func (d Decimal) tryInversePowIntU128(e int) (Decimal, error) { return Decimal{}, errOverflow } - // nolint: gosec + //nolint:gosec a256 := one128.MulToU256(pow10[defaultPrec+uint8(powPrecision)]) q, err := a256.fastQuo(u128{hi: result.hi, lo: result.lo}) @@ -1288,7 +1286,7 @@ func (d Decimal) tryInversePowIntU128(e int) (Decimal, error) { } // a256 = 10^(powPrecision + factor + defaultPrec) - // nolint: gosec + //nolint:gosec a256 := pow10[factor].MulToU256(pow10[defaultPrec+uint8(powPrecision)]) q, err := a256.fastQuo(u128{hi: result.hi, lo: result.lo}) if err != nil { @@ -1341,7 +1339,7 @@ func (d Decimal) sqrtU128() (Decimal, error) { return Decimal{}, errOverflow } - // nolint: gosec + //nolint:gosec bitLen := uint(coef.bitLen()) // bitLen < 192 // initial guess = 2^((bitLen + 1) / 2) ≥ √coef diff --git a/decimal_test.go b/decimal_test.go index 63be7c2..185b264 100644 --- a/decimal_test.go +++ b/decimal_test.go @@ -564,7 +564,6 @@ func TestAdd(t *testing.T) { aa := decimal.RequireFromString(tc.a) bb := decimal.RequireFromString(tc.b) - // nolint: gosec prec := int32(c.Prec()) cc := aa.Add(bb).Truncate(prec) @@ -2339,12 +2338,3 @@ func TestPrecUint(t *testing.T) { }) } } - -func BenchmarkDiv(b *testing.B) { - a := MustParse("22773757910726981402256170801141121114") - bb := MustParse("811656739243220271.159") - - for i := 0; i < b.N; i++ { - _, _ = a.Div(bb) - } -} diff --git a/u128.go b/u128.go index 729e4cc..7d035ab 100644 --- a/u128.go +++ b/u128.go @@ -137,6 +137,12 @@ func (u u128) Mul(v u128) (u128, error) { // MulToU256 returns u*v and carry. // The whole result will be stored in a 256-bits unsigned integer. func (u u128) MulToU256(v u128) u256 { + // short path for small numbers + if u.hi == 0 && v.hi == 0 { + hi, lo := bits.Mul64(u.lo, v.lo) + return u256{lo: lo, hi: hi} + } + hi, lo := bits.Mul64(u.lo, v.lo) p0, p1 := bits.Mul64(u.hi, v.lo) p2, p3 := bits.Mul64(u.lo, v.hi) @@ -183,7 +189,7 @@ func (u u128) QuoRem(v u128) (q, r u128, err error) { // generate a "trial quotient," guaranteed to be within 1 of the actual // quotient, then adjust. - // nolint: gosec + //nolint:gosec n := uint(bits.LeadingZeros64(v.hi)) v1 := v.Lsh(n) u1 := u.Rsh(1) diff --git a/u256.go b/u256.go index 046796c..06c5301 100644 --- a/u256.go +++ b/u256.go @@ -165,7 +165,7 @@ func (u u256) div256by128(v u128) u128 { // normalize v n := bits.LeadingZeros64(v.hi) - // nolint: gosec + //nolint:gosec v = v.Lsh(uint(n)) // shift u to the left by n bits (n < 64)