From 99e2770ac2b0ec7c9b9289f6d91997d68b55c1f7 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Mon, 10 Aug 2020 16:15:29 -0700 Subject: [PATCH 1/5] private/protocol/json/jsonutil: Fix timestamp truncation --- private/protocol/json/jsonutil/unmarshal.go | 5 +- .../protocol/json/jsonutil/unmarshal_test.go | 73 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 private/protocol/json/jsonutil/unmarshal_test.go diff --git a/private/protocol/json/jsonutil/unmarshal.go b/private/protocol/json/jsonutil/unmarshal.go index 5e9499699ba..7355c86136c 100644 --- a/private/protocol/json/jsonutil/unmarshal.go +++ b/private/protocol/json/jsonutil/unmarshal.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "math" "reflect" "strings" "time" @@ -263,7 +264,9 @@ func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag value.Set(reflect.ValueOf(&d)) case *time.Time: // Time unmarshaled from a float64 can only be epoch seconds - t := time.Unix(int64(d), 0).UTC() + seconds, frac := math.Modf(d) + ms := math.Round(frac * math.Pow10(3)) // Convert fraction to milliseconds and the round the value + t := time.Unix(int64(seconds), int64(ms*math.Pow10(6))).UTC() value.Set(reflect.ValueOf(&t)) default: return fmt.Errorf("unsupported value: %v (%s)", value.Interface(), value.Type()) diff --git a/private/protocol/json/jsonutil/unmarshal_test.go b/private/protocol/json/jsonutil/unmarshal_test.go new file mode 100644 index 00000000000..e1929cfc2c6 --- /dev/null +++ b/private/protocol/json/jsonutil/unmarshal_test.go @@ -0,0 +1,73 @@ +// +build go1.7 + +package jsonutil_test + +import ( + "bytes" + "reflect" + "testing" + "time" + + "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" +) + +func TestUnmarshalJSON_Time(t *testing.T) { + type input struct { + TimeField *time.Time `locationName:"timeField"` + } + + cases := map[string]struct { + JSON string + Value input + Expected input + }{ + "seconds precision": { + JSON: `{"timeField":1597094942}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, 00, time.UTC) + return &dt + }(), + }, + }, + "exact milliseconds precision": { + JSON: `{"timeField":1597094942.123}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, + "milliseconds precision round down": { + JSON: `{"timeField":1597094942.1234}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, + "milliseconds precision round up": { + JSON: `{"timeField":1597094942.1235}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(124*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + err := jsonutil.UnmarshalJSON(&tt.Value, bytes.NewReader([]byte(tt.JSON))) + if err != nil { + t.Errorf("expect no error, got %v", err) + } + if e, a := tt.Expected, tt.Value; !reflect.DeepEqual(e, a) { + t.Errorf("expect %v, got %v", e, a) + } + }) + } +} From 9d7f162dc499f50872f24b83e0c66a5aff944788 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 11 Aug 2020 12:50:28 -0700 Subject: [PATCH 2/5] Improvements to parsing float representations of time. --- private/protocol/json/jsonutil/unmarshal.go | 39 ++++++++++++++----- .../protocol/json/jsonutil/unmarshal_test.go | 35 +++++++++++++---- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/private/protocol/json/jsonutil/unmarshal.go b/private/protocol/json/jsonutil/unmarshal.go index 7355c86136c..d9136c1b879 100644 --- a/private/protocol/json/jsonutil/unmarshal.go +++ b/private/protocol/json/jsonutil/unmarshal.go @@ -6,7 +6,7 @@ import ( "encoding/json" "fmt" "io" - "math" + "math/big" "reflect" "strings" "time" @@ -16,6 +16,8 @@ import ( "github.com/aws/aws-sdk-go/private/protocol" ) +var millisecondsFloat = new(big.Float).SetInt64(1e3) + // UnmarshalJSONError unmarshal's the reader's JSON document into the passed in // type. The value to unmarshal the json document into must be a pointer to the // type. @@ -40,7 +42,9 @@ func UnmarshalJSONError(v interface{}, stream io.Reader) error { func UnmarshalJSON(v interface{}, stream io.Reader) error { var out interface{} - err := json.NewDecoder(stream).Decode(&out) + decoder := json.NewDecoder(stream) + decoder.UseNumber() + err := decoder.Decode(&out) if err == io.EOF { return nil } else if err != nil { @@ -55,7 +59,9 @@ func UnmarshalJSON(v interface{}, stream io.Reader) error { func UnmarshalJSONCaseInsensitive(v interface{}, stream io.Reader) error { var out interface{} - err := json.NewDecoder(stream).Decode(&out) + decoder := json.NewDecoder(stream) + decoder.UseNumber() + err := decoder.Decode(&out) if err == io.EOF { return nil } else if err != nil { @@ -255,18 +261,31 @@ func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag default: return fmt.Errorf("unsupported value: %v (%s)", value.Interface(), value.Type()) } - case float64: + case json.Number: switch value.Interface().(type) { case *int64: - di := int64(d) + // Retain the old behavior where we would just truncate the float64 + // calling d.Int64() here would cause an invalid sytnax error due to the usage of strconv.ParseInt + f, err := d.Float64() + if err != nil { + return err + } + di := int64(f) value.Set(reflect.ValueOf(&di)) case *float64: - value.Set(reflect.ValueOf(&d)) + f, err := d.Float64() + if err != nil { + return err + } + value.Set(reflect.ValueOf(&f)) case *time.Time: - // Time unmarshaled from a float64 can only be epoch seconds - seconds, frac := math.Modf(d) - ms := math.Round(frac * math.Pow10(3)) // Convert fraction to milliseconds and the round the value - t := time.Unix(int64(seconds), int64(ms*math.Pow10(6))).UTC() + float, ok := new(big.Float).SetString(d.String()) + if !ok { + return fmt.Errorf("unsupported time value: %v (%s)", value.Interface(), value.Type()) + } + float = float.Mul(float, millisecondsFloat) + ns, _ := float.Int64() + t := time.Unix(0, ns*1e6).UTC() value.Set(reflect.ValueOf(&t)) default: return fmt.Errorf("unsupported value: %v (%s)", value.Interface(), value.Type()) diff --git a/private/protocol/json/jsonutil/unmarshal_test.go b/private/protocol/json/jsonutil/unmarshal_test.go index e1929cfc2c6..31953dd8fcc 100644 --- a/private/protocol/json/jsonutil/unmarshal_test.go +++ b/private/protocol/json/jsonutil/unmarshal_test.go @@ -8,12 +8,15 @@ import ( "testing" "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" ) -func TestUnmarshalJSON_Time(t *testing.T) { +func TestUnmarshalJSON_JSONNumber(t *testing.T) { type input struct { - TimeField *time.Time `locationName:"timeField"` + TimeField *time.Time `locationName:"timeField"` + IntField *int64 `locationName:"intField"` + FloatField *float64 `locationName:"floatField"` } cases := map[string]struct { @@ -39,8 +42,8 @@ func TestUnmarshalJSON_Time(t *testing.T) { }(), }, }, - "milliseconds precision round down": { - JSON: `{"timeField":1597094942.1234}`, + "microsecond precision truncated": { + JSON: `{"timeField":1597094942.1235}`, Expected: input{ TimeField: func() *time.Time { dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) @@ -48,15 +51,33 @@ func TestUnmarshalJSON_Time(t *testing.T) { }(), }, }, - "milliseconds precision round up": { - JSON: `{"timeField":1597094942.1235}`, + "nanosecond precision truncated": { + JSON: `{"timeField":1597094942.123456789}`, Expected: input{ TimeField: func() *time.Time { - dt := time.Date(2020, 8, 10, 21, 29, 02, int(124*time.Millisecond), time.UTC) + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) return &dt }(), }, }, + "integer field": { + JSON: `{"intField":123456789}`, + Expected: input{ + IntField: aws.Int64(123456789), + }, + }, + "integer field truncated": { + JSON: `{"intField":123456789.123}`, + Expected: input{ + IntField: aws.Int64(123456789), + }, + }, + "float64 field": { + JSON: `{"floatField":123456789.123}`, + Expected: input{ + FloatField: aws.Float64(123456789.123), + }, + }, } for name, tt := range cases { From 2108cfd6d6354b15b961f3468a2ad804642c0d09 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 11 Aug 2020 13:17:07 -0700 Subject: [PATCH 3/5] Additional test cases --- private/protocol/json/jsonutil/unmarshal.go | 4 +-- .../protocol/json/jsonutil/unmarshal_test.go | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/private/protocol/json/jsonutil/unmarshal.go b/private/protocol/json/jsonutil/unmarshal.go index d9136c1b879..97b19b86dae 100644 --- a/private/protocol/json/jsonutil/unmarshal.go +++ b/private/protocol/json/jsonutil/unmarshal.go @@ -265,7 +265,7 @@ func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag switch value.Interface().(type) { case *int64: // Retain the old behavior where we would just truncate the float64 - // calling d.Int64() here would cause an invalid sytnax error due to the usage of strconv.ParseInt + // calling d.Int64() here could cause an invalid syntax error due to the usage of strconv.ParseInt f, err := d.Float64() if err != nil { return err @@ -281,7 +281,7 @@ func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag case *time.Time: float, ok := new(big.Float).SetString(d.String()) if !ok { - return fmt.Errorf("unsupported time value: %v (%s)", value.Interface(), value.Type()) + return fmt.Errorf("unsupported float time representation: %v", d.String()) } float = float.Mul(float, millisecondsFloat) ns, _ := float.Int64() diff --git a/private/protocol/json/jsonutil/unmarshal_test.go b/private/protocol/json/jsonutil/unmarshal_test.go index 31953dd8fcc..b3ac068b314 100644 --- a/private/protocol/json/jsonutil/unmarshal_test.go +++ b/private/protocol/json/jsonutil/unmarshal_test.go @@ -60,6 +60,33 @@ func TestUnmarshalJSON_JSONNumber(t *testing.T) { }(), }, }, + "milliseconds precision as small exponent": { + JSON: `{"timeField":1.597094942123e9}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, + "milliseconds precision as large exponent": { + JSON: `{"timeField":1.597094942123E9}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, + "milliseconds precision as exponent with sign": { + JSON: `{"timeField":1.597094942123e+9}`, + Expected: input{ + TimeField: func() *time.Time { + dt := time.Date(2020, 8, 10, 21, 29, 02, int(123*time.Millisecond), time.UTC) + return &dt + }(), + }, + }, "integer field": { JSON: `{"intField":123456789}`, Expected: input{ From 9edd8975100d3b6ae24ddb344d138a0a4654c454 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 11 Aug 2020 13:22:24 -0700 Subject: [PATCH 4/5] Add CHANGELOG_PENDING.md entry --- CHANGELOG_PENDING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a1927a39ca..4f2c96ed882 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,3 +3,6 @@ ### SDK Enhancements ### SDK Bugs +* `private/protocol/json/jsonutil`: Fixes a bug that truncated millisecond precision time in API response to seconds. ([#3474](https://github.com/aws/aws-sdk-go/pull/3474)) + * Fixes [#3464](https://github.com/aws/aws-sdk-go/issues/3464) + * Fixes [#3410](https://github.com/aws/aws-sdk-go/issues/3410) From 008a547f9b437172754422fb087fae0503e7b635 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 11 Aug 2020 13:28:29 -0700 Subject: [PATCH 5/5] Fix misleading variable name --- private/protocol/json/jsonutil/unmarshal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/private/protocol/json/jsonutil/unmarshal.go b/private/protocol/json/jsonutil/unmarshal.go index 97b19b86dae..8b2c9bbeba0 100644 --- a/private/protocol/json/jsonutil/unmarshal.go +++ b/private/protocol/json/jsonutil/unmarshal.go @@ -284,8 +284,8 @@ func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag return fmt.Errorf("unsupported float time representation: %v", d.String()) } float = float.Mul(float, millisecondsFloat) - ns, _ := float.Int64() - t := time.Unix(0, ns*1e6).UTC() + ms, _ := float.Int64() + t := time.Unix(0, ms*1e6).UTC() value.Set(reflect.ValueOf(&t)) default: return fmt.Errorf("unsupported value: %v (%s)", value.Interface(), value.Type())