Change parseTimeout to not handle non-second durations (grpc#1706)

Author: dfawley

service_config.go

@@ -20,6 +20,9 @@ package grpc
 
 import (
 	"encoding/json"
+	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	"google.golang.org/grpc/grpclog"
@@ -70,12 +73,48 @@ type ServiceConfig struct {
 	Methods map[string]MethodConfig
 }
 
-func parseTimeout(t *string) (*time.Duration, error) {
-	if t == nil {
+func parseDuration(s *string) (*time.Duration, error) {
+	if s == nil {
 		return nil, nil
 	}
-	d, err := time.ParseDuration(*t)
-	return &d, err
+	if !strings.HasSuffix(*s, "s") {
+		return nil, fmt.Errorf("malformed duration %q", *s)
+	}
+	ss := strings.SplitN((*s)[:len(*s)-1], ".", 3)
+	if len(ss) > 2 {
+		return nil, fmt.Errorf("malformed duration %q", *s)
+	}
+	// hasDigits is set if either the whole or fractional part of the number is
+	// present, since both are optional but one is required.
+	hasDigits := false
+	var d time.Duration
+	if len(ss[0]) > 0 {
+		i, err := strconv.ParseInt(ss[0], 10, 32)
+		if err != nil {
+			return nil, fmt.Errorf("malformed duration %q: %v", *s, err)
+		}
+		d = time.Duration(i) * time.Second
+		hasDigits = true
+	}
+	if len(ss) == 2 && len(ss[1]) > 0 {
+		if len(ss[1]) > 9 {
+			return nil, fmt.Errorf("malformed duration %q", *s)
+		}
+		f, err := strconv.ParseInt(ss[1], 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("malformed duration %q: %v", *s, err)
+		}
+		for i := 9; i > len(ss[1]); i-- {
+			f *= 10
+		}
+		d += time.Duration(f)
+		hasDigits = true
+	}
+	if !hasDigits {
+		return nil, fmt.Errorf("malformed duration %q", *s)
+	}
+
+	return &d, nil
 }
 
 type jsonName struct {
@@ -128,7 +167,7 @@ func parseServiceConfig(js string) (ServiceConfig, error) {
 		if m.Name == nil {
 			continue
 		}
-		d, err := parseTimeout(m.Timeout)
+		d, err := parseDuration(m.Timeout)
 		if err != nil {
 			grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to %v", js, err)
 			return ServiceConfig{}, err
@@ -182,18 +221,6 @@ func getMaxSize(mcMax, doptMax *int, defaultVal int) *int {
 	return doptMax
 }
 
-func newBool(b bool) *bool {
-	return &b
-}
-
 func newInt(b int) *int {
 	return &b
 }
-
-func newDuration(b time.Duration) *time.Duration {
-	return &b
-}
-
-func newString(b string) *string {
-	return &b
-}

service_config_test.go

@@ -19,6 +19,8 @@
 package grpc
 
 import (
+	"fmt"
+	"math"
 	"reflect"
 	"testing"
 	"time"
@@ -321,3 +323,64 @@ func TestPraseMsgSize(t *testing.T) {
 		}
 	}
 }
+
+func TestParseDuration(t *testing.T) {
+	testCases := []struct {
+		s    *string
+		want *time.Duration
+		err  bool
+	}{
+		{s: nil, want: nil},
+		{s: newString("1s"), want: newDuration(time.Second)},
+		{s: newString("-1s"), want: newDuration(-time.Second)},
+		{s: newString("1.1s"), want: newDuration(1100 * time.Millisecond)},
+		{s: newString("1.s"), want: newDuration(time.Second)},
+		{s: newString("1.0s"), want: newDuration(time.Second)},
+		{s: newString(".002s"), want: newDuration(2 * time.Millisecond)},
+		{s: newString(".002000s"), want: newDuration(2 * time.Millisecond)},
+		{s: newString("0.003s"), want: newDuration(3 * time.Millisecond)},
+		{s: newString("0.000004s"), want: newDuration(4 * time.Microsecond)},
+		{s: newString("5000.000000009s"), want: newDuration(5000*time.Second + 9*time.Nanosecond)},
+		{s: newString("4999.999999999s"), want: newDuration(5000*time.Second - time.Nanosecond)},
+		{s: newString("1"), err: true},
+		{s: newString("s"), err: true},
+		{s: newString(".s"), err: true},
+		{s: newString("1 s"), err: true},
+		{s: newString(" 1s"), err: true},
+		{s: newString("1ms"), err: true},
+		{s: newString("1.1.1s"), err: true},
+		{s: newString("Xs"), err: true},
+		{s: newString("as"), err: true},
+		{s: newString(".0000000001s"), err: true},
+		{s: newString(fmt.Sprint(math.MaxInt32) + "s"), want: newDuration(math.MaxInt32 * time.Second)},
+		{s: newString(fmt.Sprint(int64(math.MaxInt32)+1) + "s"), err: true},
+	}
+	for _, tc := range testCases {
+		got, err := parseDuration(tc.s)
+		if tc.err != (err != nil) ||
+			(got == nil) != (tc.want == nil) ||
+			(got != nil && *got != *tc.want) {
+			wantErr := "<nil>"
+			if tc.err {
+				wantErr = "<non-nil error>"
+			}
+			s := "<nil>"
+			if tc.s != nil {
+				s = `&"` + *tc.s + `"`
+			}
+			t.Errorf("parseDuration(%v) = %v, %v; want %v, %v", s, got, err, tc.want, wantErr)
+		}
+	}
+}
+
+func newBool(b bool) *bool {
+	return &b
+}
+
+func newDuration(b time.Duration) *time.Duration {
+	return &b
+}
+
+func newString(b string) *string {
+	return &b
+}

test/end2end_test.go

@@ -1304,7 +1304,7 @@ func TestGetMethodConfig(t *testing.T) {
                 }
             ],
             "waitForReady": true,
-            "timeout": "1ms"
+            "timeout": ".001s"
         },
         {
             "name": [
@@ -1343,7 +1343,7 @@ func TestGetMethodConfig(t *testing.T) {
                 }
             ],
             "waitForReady": true,
-            "timeout": "1ms"
+            "timeout": ".001s"
         },
         {
             "name": [
@@ -1393,7 +1393,7 @@ func TestServiceConfigWaitForReady(t *testing.T) {
                 }
             ],
             "waitForReady": false,
-            "timeout": "1ms"
+            "timeout": ".001s"
         }
     ]
 }`)
@@ -1433,7 +1433,7 @@ func TestServiceConfigWaitForReady(t *testing.T) {
                 }
             ],
             "waitForReady": true,
-            "timeout": "1ms"
+            "timeout": ".001s"
         }
     ]
 }`)
@@ -1478,7 +1478,7 @@ func TestServiceConfigTimeout(t *testing.T) {
                 }
             ],
             "waitForReady": true,
-            "timeout": "1h"
+            "timeout": "3600s"
         }
     ]
 }`)
@@ -1523,7 +1523,7 @@ func TestServiceConfigTimeout(t *testing.T) {
                 }
             ],
             "waitForReady": true,
-            "timeout": "1ns"
+            "timeout": ".000000001s"
         }
     ]
 }`)