From 605f1897232a643d1e15cbc18399099758a71fd0 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Mon, 27 Sep 2021 18:39:04 +0100
Subject: [PATCH] check presence and type of transport constructor options

---
 config/reflection_magic.go | 38 +++++++++++++++++++++++++++++++-------
 config/transport_test.go   | 16 +++++++++++++++-
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/config/reflection_magic.go b/config/reflection_magic.go
index 2aee81dc3e..c462571d2d 100644
--- a/config/reflection_magic.go
+++ b/config/reflection_magic.go
@@ -98,6 +98,32 @@ func makeArgumentConstructors(fnType reflect.Type, argTypes map[reflect.Type]con
 	return out, nil
 }
 
+func getConstructorOpts(t reflect.Type, opts ...interface{}) ([]reflect.Value, error) {
+	if !t.IsVariadic() {
+		if len(opts) > 0 {
+			return nil, errors.New("constructor doesn't accept any options")
+		}
+		return nil, nil
+	}
+	if len(opts) == 0 {
+		return nil, nil
+	}
+	// variadic parameters always go last
+	wantType := t.In(t.NumIn() - 1).Elem()
+	values := make([]reflect.Value, 0, len(opts))
+	for _, opt := range opts {
+		val := reflect.ValueOf(opt)
+		if opt == nil {
+			return nil, errors.New("expected a transport option, got nil")
+		}
+		if val.Type() != wantType {
+			return nil, fmt.Errorf("expected option of type %s, got %s", wantType, reflect.TypeOf(opt))
+		}
+		values = append(values, val.Convert(wantType))
+	}
+	return values, nil
+}
+
 // makes a transport constructor.
 func makeConstructor(
 	tpt interface{},
@@ -123,6 +149,10 @@ func makeConstructor(
 	if err != nil {
 		return nil, err
 	}
+	optValues, err := getConstructorOpts(t, opts...)
+	if err != nil {
+		return nil, err
+	}
 
 	return func(h host.Host, u *tptu.Upgrader, cg connmgr.ConnectionGater) (interface{}, error) {
 		arguments := make([]reflect.Value, 0, len(argConstructors)+len(opts))
@@ -136,13 +166,7 @@ func makeConstructor(
 				arguments = append(arguments, reflect.Zero(t.In(i)))
 			}
 		}
-		for _, opt := range opts {
-			// don't panic on nil options
-			if opt == nil {
-				return nil, errors.New("expected a transport option, got nil")
-			}
-			arguments = append(arguments, reflect.ValueOf(opt))
-		}
+		arguments = append(arguments, optValues...)
 		return callConstructor(v, arguments)
 	}, nil
 }
diff --git a/config/transport_test.go b/config/transport_test.go
index d86e031a09..bca587b9a7 100644
--- a/config/transport_test.go
+++ b/config/transport_test.go
@@ -16,6 +16,20 @@ func TestTransportVariadicOptions(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestConstructorWithoutOptsCalledWithOpts(t *testing.T) {
+	_, err := TransportConstructor(func(_ *tptu.Upgrader) transport.Transport {
+		return nil
+	}, 42)
+	require.EqualError(t, err, "constructor doesn't accept any options")
+}
+
+func TestConstructorWithOptsTypeMismatch(t *testing.T) {
+	_, err := TransportConstructor(func(_ *tptu.Upgrader, opts ...int) transport.Transport {
+		return nil
+	}, 42, "foo")
+	require.EqualError(t, err, "expected option of type int, got string")
+}
+
 func TestConstructorWithOpts(t *testing.T) {
 	var options []int
 	c, err := TransportConstructor(func(_ *tptu.Upgrader, opts ...int) transport.Transport {
@@ -25,5 +39,5 @@ func TestConstructorWithOpts(t *testing.T) {
 	require.NoError(t, err)
 	_, err = c(nil, nil, nil)
 	require.NoError(t, err)
-	require.Equal(t, options, []int{42, 1337})
+	require.Equal(t, []int{42, 1337}, options)
 }