From 6724d42e2d876862ef76cbc67920db42a135c8a5 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Fri, 13 Apr 2018 11:44:15 -0700 Subject: [PATCH 1/3] Fix XML unmarshaler not correctly unmarshaling list of timestamp values Fixes a bug in the XML unmarshaler that would incorrectly try to unmarshal "time.Time" parameters that did not have the struct tag type on them. This would occur for nested lists like CloudWatch's GetMetricDataResponse MetricDataResults timestamp parameters. Adds protocol tests for XML unmarshaling with list of timestamps. Related to aws/aws-sdk-go#1894 Fix #1892 --- models/protocol_tests/output/rest-xml.json | 34 +++++ private/protocol/restxml/unmarshal_test.go | 145 +++++++++++++++++++-- private/protocol/xml/xmlutil/unmarshal.go | 41 ++++-- 3 files changed, 196 insertions(+), 24 deletions(-) diff --git a/models/protocol_tests/output/rest-xml.json b/models/protocol_tests/output/rest-xml.json index b00ee3b1410..3e24d17ea8b 100644 --- a/models/protocol_tests/output/rest-xml.json +++ b/models/protocol_tests/output/rest-xml.json @@ -42,6 +42,12 @@ }, "Timestamp": { "shape": "TimestampType" + }, + "Blobs": { + "shape": "Blobs" + }, + "Timestamps": { + "shape": "Timestamps" } } }, @@ -76,6 +82,17 @@ }, "TimestampType": { "type": "timestamp" + }, + "BlobType": { + "type": "blob" + }, + "Blobs": { + "type":"list", + "member":{"shape":"BlobType"} + }, + "Timestamps":{ + "type":"list", + "member":{"shape":"TimestampType"} } }, "cases": [ @@ -136,6 +153,23 @@ }, "body": "123falsetrue1.21.3200a2015-01-25T08:00:00Z" } + }, + { + "given": { + "output": { + "shape": "OutputShape" + }, + "name": "OperationName" + }, + "result": { + "Blobs": ["value", "value2"], + "Timestamps": [1422172800, 1422172801] + }, + "response": { + "status_code": 200, + "headers": {}, + "body": "dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z" + } } ] }, diff --git a/private/protocol/restxml/unmarshal_test.go b/private/protocol/restxml/unmarshal_test.go index 7de81e5d489..018853267a7 100644 --- a/private/protocol/restxml/unmarshal_test.go +++ b/private/protocol/restxml/unmarshal_test.go @@ -100,13 +100,13 @@ type OutputService1TestCaseOperation1Request struct { } // Send marshals and sends the OutputService1TestCaseOperation1 API request. -func (r OutputService1TestCaseOperation1Request) Send() (*OutputService1TestShapeOutputService1TestCaseOperation2Output, error) { +func (r OutputService1TestCaseOperation1Request) Send() (*OutputService1TestShapeOutputService1TestCaseOperation3Output, error) { err := r.Request.Send() if err != nil { return nil, err } - return r.Request.Data.(*OutputService1TestShapeOutputService1TestCaseOperation2Output), nil + return r.Request.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output), nil } // OutputService1TestCaseOperation1Request returns a request value for making API operation for @@ -129,7 +129,7 @@ func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation1Request(inp input = &OutputService1TestShapeOutputService1TestCaseOperation1Input{} } - output := &OutputService1TestShapeOutputService1TestCaseOperation2Output{} + output := &OutputService1TestShapeOutputService1TestCaseOperation3Output{} req := c.newRequest(op, input, output) output.responseMetadata = aws.Response{Request: req} @@ -146,13 +146,13 @@ type OutputService1TestCaseOperation2Request struct { } // Send marshals and sends the OutputService1TestCaseOperation2 API request. -func (r OutputService1TestCaseOperation2Request) Send() (*OutputService1TestShapeOutputService1TestCaseOperation2Output, error) { +func (r OutputService1TestCaseOperation2Request) Send() (*OutputService1TestShapeOutputService1TestCaseOperation3Output, error) { err := r.Request.Send() if err != nil { return nil, err } - return r.Request.Data.(*OutputService1TestShapeOutputService1TestCaseOperation2Output), nil + return r.Request.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output), nil } // OutputService1TestCaseOperation2Request returns a request value for making API operation for @@ -175,13 +175,59 @@ func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation2Request(inp input = &OutputService1TestShapeOutputService1TestCaseOperation2Input{} } - output := &OutputService1TestShapeOutputService1TestCaseOperation2Output{} + output := &OutputService1TestShapeOutputService1TestCaseOperation3Output{} req := c.newRequest(op, input, output) output.responseMetadata = aws.Response{Request: req} return OutputService1TestCaseOperation2Request{Request: req, Input: input, Copy: c.OutputService1TestCaseOperation2Request} } +const opOutputService1TestCaseOperation3 = "OperationName" + +// OutputService1TestCaseOperation3Request is a API request type for the OutputService1TestCaseOperation3 API operation. +type OutputService1TestCaseOperation3Request struct { + *aws.Request + Input *OutputService1TestShapeOutputService1TestCaseOperation3Input + Copy func(*OutputService1TestShapeOutputService1TestCaseOperation3Input) OutputService1TestCaseOperation3Request +} + +// Send marshals and sends the OutputService1TestCaseOperation3 API request. +func (r OutputService1TestCaseOperation3Request) Send() (*OutputService1TestShapeOutputService1TestCaseOperation3Output, error) { + err := r.Request.Send() + if err != nil { + return nil, err + } + + return r.Request.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output), nil +} + +// OutputService1TestCaseOperation3Request returns a request value for making API operation for +// . +// +// // Example sending a request using the OutputService1TestCaseOperation3Request method. +// req := client.OutputService1TestCaseOperation3Request(params) +// resp, err := req.Send() +// if err == nil { +// fmt.Println(resp) +// } +func (c *OutputService1ProtocolTest) OutputService1TestCaseOperation3Request(input *OutputService1TestShapeOutputService1TestCaseOperation3Input) OutputService1TestCaseOperation3Request { + op := &aws.Operation{ + Name: opOutputService1TestCaseOperation3, + + HTTPPath: "/", + } + + if input == nil { + input = &OutputService1TestShapeOutputService1TestCaseOperation3Input{} + } + + output := &OutputService1TestShapeOutputService1TestCaseOperation3Output{} + req := c.newRequest(op, input, output) + output.responseMetadata = aws.Response{Request: req} + + return OutputService1TestCaseOperation3Request{Request: req, Input: input, Copy: c.OutputService1TestCaseOperation3Request} +} + type OutputService1TestShapeOutputService1TestCaseOperation1Input struct { _ struct{} `type:"structure"` } @@ -202,11 +248,23 @@ func (s OutputService1TestShapeOutputService1TestCaseOperation2Input) MarshalFie return nil } -type OutputService1TestShapeOutputService1TestCaseOperation2Output struct { +type OutputService1TestShapeOutputService1TestCaseOperation3Input struct { + _ struct{} `type:"structure"` +} + +// MarshalFields encodes the AWS API shape using the passed in protocol encoder. +func (s OutputService1TestShapeOutputService1TestCaseOperation3Input) MarshalFields(e protocol.FieldEncoder) error { + + return nil +} + +type OutputService1TestShapeOutputService1TestCaseOperation3Output struct { _ struct{} `type:"structure"` responseMetadata aws.Response + Blobs [][]byte `type:"list"` + Char *string `type:"character"` Double *float64 `type:"double"` @@ -227,16 +285,30 @@ type OutputService1TestShapeOutputService1TestCaseOperation2Output struct { Timestamp *time.Time `type:"timestamp" timestampFormat:"iso8601"` + Timestamps []time.Time `type:"list"` + TrueBool *bool `type:"boolean"` } // SDKResponseMetdata return sthe response metadata for the API. -func (s OutputService1TestShapeOutputService1TestCaseOperation2Output) SDKResponseMetadata() aws.Response { +func (s OutputService1TestShapeOutputService1TestCaseOperation3Output) SDKResponseMetadata() aws.Response { return s.responseMetadata } // MarshalFields encodes the AWS API shape using the passed in protocol encoder. -func (s OutputService1TestShapeOutputService1TestCaseOperation2Output) MarshalFields(e protocol.FieldEncoder) error { +func (s OutputService1TestShapeOutputService1TestCaseOperation3Output) MarshalFields(e protocol.FieldEncoder) error { + if len(s.Blobs) > 0 { + v := s.Blobs + + metadata := protocol.Metadata{} + ls0 := e.List(protocol.BodyTarget, "Blobs", metadata) + ls0.Start() + for _, v1 := range v { + ls0.ListAddValue(protocol.BytesValue(v1)) + } + ls0.End() + + } if s.Char != nil { v := *s.Char @@ -285,6 +357,18 @@ func (s OutputService1TestShapeOutputService1TestCaseOperation2Output) MarshalFi metadata := protocol.Metadata{} e.SetValue(protocol.BodyTarget, "Timestamp", protocol.TimeValue{V: v, Format: protocol.ISO8601TimeFormat}, metadata) } + if len(s.Timestamps) > 0 { + v := s.Timestamps + + metadata := protocol.Metadata{} + ls0 := e.List(protocol.BodyTarget, "Timestamps", metadata) + ls0.Start() + for _, v1 := range v { + ls0.ListAddValue(protocol.TimeValue{V: v1}) + } + ls0.End() + + } if s.TrueBool != nil { v := *s.TrueBool @@ -2325,7 +2409,7 @@ func TestOutputService1ProtocolTestScalarMembersCase1(t *testing.T) { t.Errorf("expect not error, got %v", req.Error) } - out := req.Data.(*OutputService1TestShapeOutputService1TestCaseOperation2Output) + out := req.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output) // assert response if out == nil { t.Errorf("expect not to be nil") @@ -2387,7 +2471,7 @@ func TestOutputService1ProtocolTestScalarMembersCase2(t *testing.T) { t.Errorf("expect not error, got %v", req.Error) } - out := req.Data.(*OutputService1TestShapeOutputService1TestCaseOperation2Output) + out := req.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output) // assert response if out == nil { t.Errorf("expect not to be nil") @@ -2428,6 +2512,45 @@ func TestOutputService1ProtocolTestScalarMembersCase2(t *testing.T) { } +func TestOutputService1ProtocolTestScalarMembersCase3(t *testing.T) { + cfg := unit.Config() + cfg.EndpointResolver = aws.ResolveWithEndpointURL("https://test") + + svc := NewOutputService1ProtocolTest(cfg) + + buf := bytes.NewReader([]byte("dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z")) + req := svc.OutputService1TestCaseOperation3Request(nil) + req.HTTPResponse = &http.Response{StatusCode: 200, Body: ioutil.NopCloser(buf), Header: http.Header{}} + + // set headers + + // unmarshal response + restxml.UnmarshalMeta(req.Request) + restxml.Unmarshal(req.Request) + if req.Error != nil { + t.Errorf("expect not error, got %v", req.Error) + } + + out := req.Data.(*OutputService1TestShapeOutputService1TestCaseOperation3Output) + // assert response + if out == nil { + t.Errorf("expect not to be nil") + } + if e, a := "value", string(out.Blobs[0]); e != a { + t.Errorf("expect %v, got %v", e, a) + } + if e, a := "value2", string(out.Blobs[1]); e != a { + t.Errorf("expect %v, got %v", e, a) + } + if e, a := time.Unix(1.4221728e+09, 0).UTC().String(), out.Timestamps[0].String(); e != a { + t.Errorf("expect %v, got %v", e, a) + } + if e, a := time.Unix(1.422172801e+09, 0).UTC().String(), out.Timestamps[1].String(); e != a { + t.Errorf("expect %v, got %v", e, a) + } + +} + func TestOutputService2ProtocolTestBlobCase1(t *testing.T) { cfg := unit.Config() cfg.EndpointResolver = aws.ResolveWithEndpointURL("https://test") diff --git a/private/protocol/xml/xmlutil/unmarshal.go b/private/protocol/xml/xmlutil/unmarshal.go index 3999a289249..88a3ecde401 100644 --- a/private/protocol/xml/xmlutil/unmarshal.go +++ b/private/protocol/xml/xmlutil/unmarshal.go @@ -52,14 +52,23 @@ func parse(r reflect.Value, node *XMLNode, tag reflect.StructTag) error { if t == "" { switch rtype.Kind() { case reflect.Struct: - t = "structure" + // also it can't be a time object + _, tok := r.Interface().(*time.Time) + if _, ok := r.Interface().(time.Time); !(ok || tok) { + t = "structure" + } case reflect.Slice: - t = "list" + // also it can't be a byte slice + if _, ok := r.Interface().([]byte); !ok { + t = "list" + } case reflect.Map: t = "map" } } + fmt.Println("parsing", node.Name, tag.Get("type"), t, r.Type()) + switch t { case "structure": if field, ok := rtype.FieldByName("_"); ok { @@ -223,41 +232,47 @@ func parseScalar(r reflect.Value, node *XMLNode, tag reflect.StructTag) error { return nil } + if r.Kind() == reflect.Ptr { + if r.IsNil() { + r.Set(reflect.New(r.Type().Elem())) + } + r = r.Elem() + } + switch r.Interface().(type) { - case *string: - r.Set(reflect.ValueOf(&node.Text)) - return nil + case string: + r.Set(reflect.ValueOf(node.Text)) case []byte: b, err := base64.StdEncoding.DecodeString(node.Text) if err != nil { return err } r.Set(reflect.ValueOf(b)) - case *bool: + case bool: v, err := strconv.ParseBool(node.Text) if err != nil { return err } - r.Set(reflect.ValueOf(&v)) - case *int64: + r.Set(reflect.ValueOf(v)) + case int64: v, err := strconv.ParseInt(node.Text, 10, 64) if err != nil { return err } - r.Set(reflect.ValueOf(&v)) - case *float64: + r.Set(reflect.ValueOf(v)) + case float64: v, err := strconv.ParseFloat(node.Text, 64) if err != nil { return err } - r.Set(reflect.ValueOf(&v)) - case *time.Time: + r.Set(reflect.ValueOf(v)) + case time.Time: const ISO8601UTC = "2006-01-02T15:04:05Z" t, err := time.Parse(ISO8601UTC, node.Text) if err != nil { return err } - r.Set(reflect.ValueOf(&t)) + r.Set(reflect.ValueOf(t)) default: return fmt.Errorf("unsupported value: %v (%s)", r.Interface(), r.Type()) } From 873fb9be21bed35196941fdcbd66b489414c8064 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Mon, 23 Apr 2018 14:10:57 -0700 Subject: [PATCH 2/3] remove debug line --- private/protocol/xml/xmlutil/unmarshal.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/private/protocol/xml/xmlutil/unmarshal.go b/private/protocol/xml/xmlutil/unmarshal.go index 88a3ecde401..2b95c35e668 100644 --- a/private/protocol/xml/xmlutil/unmarshal.go +++ b/private/protocol/xml/xmlutil/unmarshal.go @@ -67,8 +67,6 @@ func parse(r reflect.Value, node *XMLNode, tag reflect.StructTag) error { } } - fmt.Println("parsing", node.Name, tag.Get("type"), t, r.Type()) - switch t { case "structure": if field, ok := rtype.FieldByName("_"); ok { From 72da4ca803b4fd15db33b624b1b7023cbbd7a901 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Mon, 23 Apr 2018 14:11:09 -0700 Subject: [PATCH 3/3] Add protocol tests for list of values --- models/protocol_tests/output/rest-xml.json | 14 +++++++++++--- private/protocol/restxml/unmarshal_test.go | 16 +++++++++++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/models/protocol_tests/output/rest-xml.json b/models/protocol_tests/output/rest-xml.json index 3e24d17ea8b..ade7c2224e2 100644 --- a/models/protocol_tests/output/rest-xml.json +++ b/models/protocol_tests/output/rest-xml.json @@ -48,6 +48,9 @@ }, "Timestamps": { "shape": "Timestamps" + }, + "Float64s": { + "shape": "Float64s" } } }, @@ -93,6 +96,10 @@ "Timestamps":{ "type":"list", "member":{"shape":"TimestampType"} + }, + "Float64s": { + "type":"list", + "member":{"shape":"FloatType"} } }, "cases": [ @@ -162,13 +169,14 @@ "name": "OperationName" }, "result": { - "Blobs": ["value", "value2"], - "Timestamps": [1422172800, 1422172801] + "Float64s": [0.1, 0.2], + "Blobs": ["value", "value2"], + "Timestamps": [1422172800, 1422172801] }, "response": { "status_code": 200, "headers": {}, - "body": "dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z" + "body": "0.10.2dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z" } } ] diff --git a/private/protocol/restxml/unmarshal_test.go b/private/protocol/restxml/unmarshal_test.go index 018853267a7..a9c61f57a2f 100644 --- a/private/protocol/restxml/unmarshal_test.go +++ b/private/protocol/restxml/unmarshal_test.go @@ -273,6 +273,8 @@ type OutputService1TestShapeOutputService1TestCaseOperation3Output struct { Float *float64 `type:"float"` + Float64s []float64 `type:"list"` + ImaHeader *string `location:"header" type:"string"` ImaHeaderLocation *string `location:"header" locationName:"X-Foo" type:"string"` @@ -333,6 +335,18 @@ func (s OutputService1TestShapeOutputService1TestCaseOperation3Output) MarshalFi metadata := protocol.Metadata{} e.SetValue(protocol.BodyTarget, "Float", protocol.Float64Value(v), metadata) } + if len(s.Float64s) > 0 { + v := s.Float64s + + metadata := protocol.Metadata{} + ls0 := e.List(protocol.BodyTarget, "Float64s", metadata) + ls0.Start() + for _, v1 := range v { + ls0.ListAddValue(protocol.Float64Value(v1)) + } + ls0.End() + + } if s.Long != nil { v := *s.Long @@ -2518,7 +2532,7 @@ func TestOutputService1ProtocolTestScalarMembersCase3(t *testing.T) { svc := NewOutputService1ProtocolTest(cfg) - buf := bytes.NewReader([]byte("dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z")) + buf := bytes.NewReader([]byte("0.10.2dmFsdWU=dmFsdWUy2015-01-25T08:00:00Z2015-01-25T08:00:01Z")) req := svc.OutputService1TestCaseOperation3Request(nil) req.HTTPResponse = &http.Response{StatusCode: 200, Body: ioutil.NopCloser(buf), Header: http.Header{}}