runtime: stop using nil ponters with Unmarshal (grpc-ecosystem#1028)

* runtime: stop using nil ponters with Unmarshal

Fixes grpc-ecosystem#1025

runtime/BUILD.bazel

@@ -48,6 +48,7 @@ go_test(
     size = "small",
     srcs = [
         "context_test.go",
+        "convert_test.go",
         "errors_test.go",
         "fieldmask_test.go",
         "handler_test.go",

runtime/convert.go

@@ -206,16 +206,22 @@ func BytesSlice(val, sep string) ([][]byte, error) {
 
 // Timestamp converts the given RFC3339 formatted string into a timestamp.Timestamp.
 func Timestamp(val string) (*timestamp.Timestamp, error) {
-	var r *timestamp.Timestamp
-	err := jsonpb.UnmarshalString(val, r)
-	return r, err
+	var r timestamp.Timestamp
+	err := jsonpb.UnmarshalString(val, &r)
+	if err != nil {
+		return nil, err
+	}
+	return &r, nil
 }
 
 // Duration converts the given string into a timestamp.Duration.
 func Duration(val string) (*duration.Duration, error) {
-	var r *duration.Duration
-	err := jsonpb.UnmarshalString(val, r)
-	return r, err
+	var r duration.Duration
+	err := jsonpb.UnmarshalString(val, &r)
+	if err != nil {
+		return nil, err
+	}
+	return &r, nil
 }
 
 // Enum converts the given string into an int32 that should be type casted into the

runtime/convert_test.go

@@ -0,0 +1,143 @@
+package runtime_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/golang/protobuf/proto"
+	"github.com/golang/protobuf/ptypes/duration"
+	"github.com/golang/protobuf/ptypes/timestamp"
+	"github.com/grpc-ecosystem/grpc-gateway/runtime"
+)
+
+func TestConvertTimestamp(t *testing.T) {
+	specs := []struct {
+		name    string
+		input   string
+		output  *timestamp.Timestamp
+		wanterr error
+	}{
+		{
+			name:  "a valid RFC3339 timestamp",
+			input: `"2016-05-10T10:19:13.123Z"`,
+			output: &timestamp.Timestamp{
+				Seconds: 1462875553,
+				Nanos:   123000000,
+			},
+			wanterr: nil,
+		},
+		{
+			name:    "invalid timestamp",
+			input:   `"05-10-2016T10:19:13.123Z"`,
+			output:  nil,
+			wanterr: fmt.Errorf(`bad Timestamp: parsing time "05-10-2016T10:19:13.123Z" as "2006-01-02T15:04:05.999999999Z07:00": cannot parse "0-2016T10:19:13.123Z" as "2006"`),
+		},
+		{
+			name:   "JSON number",
+			input:  "123",
+			output: nil,
+			wanterr: &json.UnmarshalTypeError{
+				Value:  "number",
+				Type:   reflect.TypeOf("123"),
+				Offset: 3,
+			},
+		},
+		{
+			name:   "JSON bool",
+			input:  "true",
+			output: nil,
+			wanterr: &json.UnmarshalTypeError{
+				Value:  "bool",
+				Type:   reflect.TypeOf("123"),
+				Offset: 4,
+			},
+		},
+	}
+
+	for _, spec := range specs {
+		t.Run(spec.name, func(t *testing.T) {
+			ts, err := runtime.Timestamp(spec.input)
+			if spec.wanterr != nil {
+				if !reflect.DeepEqual(err, spec.wanterr) {
+					t.Errorf("got unexpected error\n%#v\nexpected\n%#v", err, spec.wanterr)
+				}
+				return
+			}
+			if !proto.Equal(ts, spec.output) {
+				t.Errorf(
+					"when testing %s; got\n%#v\nexpected\n%#v",
+					spec.name,
+					ts,
+					spec.output,
+				)
+			}
+		})
+	}
+}
+
+func TestConvertDuration(t *testing.T) {
+	specs := []struct {
+		name    string
+		input   string
+		output  *duration.Duration
+		wanterr error
+	}{
+		{
+			name:  "a valid duration",
+			input: `"123.456s"`,
+			output: &duration.Duration{
+				Seconds: 123,
+				Nanos:   456000000,
+			},
+			wanterr: nil,
+		},
+		{
+			name:    "invalid duration",
+			input:   `"123years"`,
+			output:  nil,
+			wanterr: fmt.Errorf(`bad Duration: time: unknown unit years in duration 123years`),
+		},
+		{
+			name:   "JSON number",
+			input:  "123",
+			output: nil,
+			wanterr: &json.UnmarshalTypeError{
+				Value:  "number",
+				Type:   reflect.TypeOf("123"),
+				Offset: 3,
+			},
+		},
+		{
+			name:   "JSON bool",
+			input:  "true",
+			output: nil,
+			wanterr: &json.UnmarshalTypeError{
+				Value:  "bool",
+				Type:   reflect.TypeOf("123"),
+				Offset: 4,
+			},
+		},
+	}
+
+	for _, spec := range specs {
+		t.Run(spec.name, func(t *testing.T) {
+			ts, err := runtime.Duration(spec.input)
+			if spec.wanterr != nil {
+				if !reflect.DeepEqual(err, spec.wanterr) {
+					t.Errorf("got unexpected error\n%#v\nexpected\n%#v", err, spec.wanterr)
+				}
+				return
+			}
+			if !proto.Equal(ts, spec.output) {
+				t.Errorf(
+					"when testing %s; got\n%#v\nexpected\n%#v",
+					spec.name,
+					ts,
+					spec.output,
+				)
+			}
+		})
+	}
+}