From 60f42ea61cb7e1de8d54432d8fb9ab028b8a575d Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Fri, 14 Aug 2020 22:58:49 +1200 Subject: strconv: fix incorrect bit size in ParseComplex; add tests In ParseComplex, the "size" passed to parseFloatPrefix should be 64 for complex128, not 128. It still works because of how parseFloatPrefix is forgiving about the size if it's not 32, but worth fixing anyway. Make ParseComplex and ParseFloat return a bit size error for anything other than 128 or 64 (for ParseComplex), or 64 or 32 (for ParseFloat). Add "InvalidBitSize" tests for these cases. Add tests for ParseComplex with bitSize==64: this is done in a similar way to how the ParseFloat 32-bit tests work, re-using the tests for the larger bit size. Add tests for FormatComplex -- there were none before. Fixes #40706 Change-Id: I16ddd546e5237207cc3b8c2181dd708eca42b04f Reviewed-on: https://go-review.googlesource.com/c/go/+/248219 Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Trust: Minux Ma Trust: Robert Griesemer Reviewed-by: Robert Griesemer --- src/strconv/atoc.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/strconv/atoc.go') diff --git a/src/strconv/atoc.go b/src/strconv/atoc.go index 55b7c23ee7..52cb5908b2 100644 --- a/src/strconv/atoc.go +++ b/src/strconv/atoc.go @@ -40,10 +40,10 @@ func convErr(err error, s string) (syntax, range_ error) { // away from the largest floating point number of the given component's size, // ParseComplex returns err.Err = ErrRange and c = ±Inf for the respective component. func ParseComplex(s string, bitSize int) (complex128, error) { - size := 128 - if bitSize == 64 { - size = 32 // complex64 uses float32 parts + if bitSize != 64 && bitSize != 128 { + return 0, bitSizeError(fnParseComplex, s, bitSize) } + size := bitSize >> 1 orig := s -- cgit v1.3-5-g9baa From e1b305af028544e00a22c905e68049c98c10a1cc Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Wed, 4 Nov 2020 10:13:42 +1300 Subject: strconv: revert ParseFloat/ParseComplex error on incorrect bitSize This is a partial revert of https://go-review.googlesource.com/c/go/+/248219 because we found that a non-trivial amount of code erroneously calls ParseFloat(s, 10) or even ParseFloat(s, 0) and expects it to work -- before that change was merged, ParseFloat accepted a bitSize of anything other than 32 or 64 to mean 64 (and ParseComplex was similar). So revert that behavior to avoid breaking people's code, and add tests for this. I may add a vet check to flag ParseFloat(s, not_32_or_64) in a later change. See #42297 for more details. Change-Id: I4bc0156bd74f67a39d5561b6e5fde3f2d20bd622 Reviewed-on: https://go-review.googlesource.com/c/go/+/267319 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Trust: Robert Griesemer Reviewed-by: Ian Lance Taylor --- src/strconv/atoc.go | 6 +++--- src/strconv/atoc_test.go | 21 +++++++++++++-------- src/strconv/atof.go | 3 --- src/strconv/atof_test.go | 22 ++++++++++++++-------- 4 files changed, 30 insertions(+), 22 deletions(-) (limited to 'src/strconv/atoc.go') diff --git a/src/strconv/atoc.go b/src/strconv/atoc.go index 52cb5908b2..85c7bafefa 100644 --- a/src/strconv/atoc.go +++ b/src/strconv/atoc.go @@ -40,10 +40,10 @@ func convErr(err error, s string) (syntax, range_ error) { // away from the largest floating point number of the given component's size, // ParseComplex returns err.Err = ErrRange and c = ±Inf for the respective component. func ParseComplex(s string, bitSize int) (complex128, error) { - if bitSize != 64 && bitSize != 128 { - return 0, bitSizeError(fnParseComplex, s, bitSize) + size := 64 + if bitSize == 64 { + size = 32 // complex64 uses float32 parts } - size := bitSize >> 1 orig := s diff --git a/src/strconv/atoc_test.go b/src/strconv/atoc_test.go index aecc09d247..4c1aad0900 100644 --- a/src/strconv/atoc_test.go +++ b/src/strconv/atoc_test.go @@ -212,13 +212,18 @@ func TestParseComplex(t *testing.T) { } } -func TestParseComplexInvalidBitSize(t *testing.T) { - _, err := ParseComplex("1+2i", 100) - const want = `strconv.ParseComplex: parsing "1+2i": invalid bit size 100` - if err == nil { - t.Fatalf("got nil error, want %q", want) - } - if err.Error() != want { - t.Fatalf("got error %q, want %q", err, want) +// Issue 42297: allow ParseComplex(s, not_32_or_64) for legacy reasons +func TestParseComplexIncorrectBitSize(t *testing.T) { + const s = "1.5e308+1.0e307i" + const want = 1.5e308 + 1.0e307i + + for _, bitSize := range []int{0, 10, 100, 256} { + c, err := ParseComplex(s, bitSize) + if err != nil { + t.Fatalf("ParseComplex(%q, %d) gave error %s", s, bitSize, err) + } + if c != want { + t.Fatalf("ParseComplex(%q, %d) = %g (expected %g)", s, bitSize, c, want) + } } } diff --git a/src/strconv/atof.go b/src/strconv/atof.go index a04f5621f6..9010a66ca8 100644 --- a/src/strconv/atof.go +++ b/src/strconv/atof.go @@ -688,9 +688,6 @@ func atof64(s string) (f float64, n int, err error) { // ParseFloat recognizes the strings "NaN", and the (possibly signed) strings "Inf" and "Infinity" // as their respective special floating point values. It ignores case when matching. func ParseFloat(s string, bitSize int) (float64, error) { - if bitSize != 32 && bitSize != 64 { - return 0, bitSizeError(fnParseFloat, s, bitSize) - } f, n, err := parseFloatPrefix(s, bitSize) if err == nil && n != len(s) { return 0, syntaxError(fnParseFloat, s) diff --git a/src/strconv/atof_test.go b/src/strconv/atof_test.go index cf43903506..3c058b9be5 100644 --- a/src/strconv/atof_test.go +++ b/src/strconv/atof_test.go @@ -634,14 +634,20 @@ func TestRoundTrip32(t *testing.T) { t.Logf("tested %d float32's", count) } -func TestParseFloatInvalidBitSize(t *testing.T) { - _, err := ParseFloat("3.14", 100) - const want = `strconv.ParseFloat: parsing "3.14": invalid bit size 100` - if err == nil { - t.Fatalf("got nil error, want %q", want) - } - if err.Error() != want { - t.Fatalf("got error %q, want %q", err, want) +// Issue 42297: a lot of code in the wild accidentally calls ParseFloat(s, 10) +// or ParseFloat(s, 0), so allow bitSize values other than 32 and 64. +func TestParseFloatIncorrectBitSize(t *testing.T) { + const s = "1.5e308" + const want = 1.5e308 + + for _, bitSize := range []int{0, 10, 100, 128} { + f, err := ParseFloat(s, bitSize) + if err != nil { + t.Fatalf("ParseFloat(%q, %d) gave error %s", s, bitSize, err) + } + if f != want { + t.Fatalf("ParseFloat(%q, %d) = %g (expected %g)", s, bitSize, f, want) + } } } -- cgit v1.3-5-g9baa