Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

types: fix overflow check of types/convert.go::floatStrToIntStr #11114

Merged
merged 22 commits into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f472fab
types: fix overflow check of floatStrToIntStr
H-ZeX Jul 6, 2019
a26272d
fix overflow check
H-ZeX Jul 6, 2019
81d6cf9
improve test converage
H-ZeX Jul 8, 2019
26f52f3
Merge branch 'master' into fix_overflow_check
H-ZeX Jul 8, 2019
ab82a67
improve test converage
H-ZeX Jul 9, 2019
9c27a8d
Merge branch 'fix_overflow_check' of github.com:pingcap/tidb into fix…
H-ZeX Jul 9, 2019
1d2ba4b
fix unit test
H-ZeX Jul 9, 2019
15ca9d6
Merge branch 'master' into fix_overflow_check
H-ZeX Jul 10, 2019
4b8109e
Merge branch 'master' into fix_overflow_check
lonng Jul 10, 2019
66e19e3
remove isOverflowInt64 check
H-ZeX Jul 10, 2019
4edb2ee
Merge branch 'fix_overflow_check' of github.com:pingcap/tidb into fix…
H-ZeX Jul 10, 2019
9062ef4
Merge branch 'master' into fix_overflow_check
lonng Jul 11, 2019
06c0fb7
Merge branch 'master' into fix_overflow_check
lonng Jul 11, 2019
627a573
fix a bug
H-ZeX Jul 11, 2019
14467f1
Merge branch 'fix_overflow_check' of github.com:pingcap/tidb into fix…
H-ZeX Jul 11, 2019
b2e81af
Merge branch 'master' into fix_overflow_check
lonng Jul 11, 2019
7bffb9c
fix a bug
H-ZeX Jul 11, 2019
f6a30d4
Merge branch 'fix_overflow_check' of github.com:pingcap/tidb into fix…
H-ZeX Jul 11, 2019
4d65064
Merge branch 'master' into fix_overflow_check
lonng Jul 11, 2019
5cd0090
Merge branch 'master' into fix_overflow_check
lonng Jul 12, 2019
d611ddf
Merge branch 'master' into fix_overflow_check
lonng Jul 16, 2019
8aa2827
Merge branch 'master' into fix_overflow_check
lonng Jul 16, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions types/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ func roundIntStr(numNextDot byte, intStr string) string {
// strconv.ParseInt, we can't parse float first then convert it to string because precision will
// be lost. For example, the string value "18446744073709551615" which is the max number of unsigned
// int will cause some precision to lose. intStr[0] may be a positive and negative sign like '+' or '-'.
//
// This func will find serious overflow such as the len of intStr > 20 (without prefix `+/-`)
// however, it will not check whether the intStr overflow BIGINT.
func floatStrToIntStr(sc *stmtctx.StatementContext, validFloat string, oriStr string) (intStr string, _ error) {
var dotIdx = -1
var eIdx = -1
Expand Down Expand Up @@ -443,12 +446,15 @@ func floatStrToIntStr(sc *stmtctx.StatementContext, validFloat string, oriStr st
if err != nil {
return validFloat, errors.Trace(err)
}
if exp > 0 && int64(intCnt) > (math.MaxInt64-int64(exp)) {
// (exp + incCnt) overflows MaxInt64.
intCnt += exp
if exp >= 0 && (intCnt > 21 || intCnt < 0) {
// MaxInt64 has 19 decimal digits.
// MaxUint64 has 20 decimal digits.
// And the intCnt may contain the len of `+/-`,
// so I use 21 here as the early detection.
sc.AppendWarning(ErrOverflow.GenWithStackByArgs("BIGINT", oriStr))
return validFloat[:eIdx], nil
}
intCnt += exp
if intCnt <= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intCnt will never be negative according to the L448.
Prefer to:

if intCnt > 0 {
	extraZeroCount := ...
} else {
	...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intCnt>=0, exp>=0, however, if exp=I64::MAX, intCnt>0, then their sum will overflow and get negative.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intCnt>=0, exp>=0, however, if exp=I64::MAX, intCnt>0, then their sum will overflow and get negative.

If intCnt < 0 will early return in L451.

intStr = "0"
if intCnt == 0 && len(digits) > 0 {
Expand All @@ -474,11 +480,6 @@ func floatStrToIntStr(sc *stmtctx.StatementContext, validFloat string, oriStr st
} else {
// convert scientific notation decimal number
extraZeroCount := intCnt - len(digits)
if extraZeroCount > 20 {
// Append overflow warning and return to avoid allocating too much memory.
sc.AppendWarning(ErrOverflow.GenWithStackByArgs("BIGINT", oriStr))
return validFloat[:eIdx], nil
}
intStr = string(digits) + strings.Repeat("0", extraZeroCount)
}
return intStr, nil
Expand Down
1 change: 1 addition & 0 deletions types/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ func (s *testTypeConvertSuite) TestGetValidFloat(c *C) {
{".5e0", "1"},
{"+.5e0", "+1"},
{"-.5e0", "-1"},
{".5", "1"},
{"123.456789e5", "12345679"},
{"123.456784e5", "12345678"},
}
Expand Down