configstruct: fix parsing of invalid booleans in the config

Apparently fmt.Sscanln doesn't parse bool's properly and this isn't
likely to be fixed by the Go team who regard sscanf as a mistake.

This only uses sscan for integers and uses the correct routine for
everything else.

This also implements parsing time.Duration

See: https://github.com/golang/go/issues/43306

fs/config/configstruct/configstruct.go

@@ -6,7 +6,9 @@ import (
 	"fmt"
 	"reflect"
 	"regexp"
+	"strconv"
 	"strings"
+	"time"
 
 	"github.com/rclone/rclone/fs/config/configmap"
 )
@@ -24,22 +26,39 @@ func camelToSnake(in string) string {
 // StringToInterface turns in into an interface{} the same type as def
 func StringToInterface(def interface{}, in string) (newValue interface{}, err error) {
 	typ := reflect.TypeOf(def)
-	if typ.Kind() == reflect.String && typ.Name() == "string" {
-		// Pass strings unmodified
-		return in, nil
-	}
-	// Otherwise parse with Sscanln
-	//
-	// This means any types we use here must implement fmt.Scanner
 	o := reflect.New(typ)
-	n, err := fmt.Sscanln(in, o.Interface())
+	switch def.(type) {
+	case string:
+		// return strings unmodified
+		newValue = in
+	case int, int8, int16, int32, int64,
+		uint, uint8, uint16, uint32, uint64, uintptr,
+		float32, float64:
+		// As per Rob Pike's advice in https://github.com/golang/go/issues/43306
+		// we only use Sscan for numbers
+		var n int
+		n, err = fmt.Sscanln(in, o.Interface())
+		if err == nil && n != 1 {
+			err = errors.New("no items parsed")
+		}
+		newValue = o.Elem().Interface()
+	case bool:
+		newValue, err = strconv.ParseBool(in)
+	case time.Duration:
+		newValue, err = time.ParseDuration(in)
+	default:
+		// Try using a Set method
+		if do, ok := o.Interface().(interface{ Set(s string) error }); ok {
+			err = do.Set(in)
+		} else {
+			err = errors.New("don't know how to parse this type")
+		}
+		newValue = o.Elem().Interface()
+	}
 	if err != nil {
-		return newValue, fmt.Errorf("parsing %q as %T failed: %w", in, def, err)
+		return nil, fmt.Errorf("parsing %q as %T failed: %w", in, def, err)
 	}
-	if n != 1 {
+	return newValue, nil
-		return newValue, errors.New("no items parsed")
-	}
-	return o.Elem().Interface(), nil
 }
 
 // Item describes a single entry in the options structure

fs/config/configstruct/configstruct_test.go

@@ -1,6 +1,7 @@
 package configstruct_test
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -114,3 +115,53 @@ func TestSetFull(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, want, in)
 }
+
+func TestStringToInterface(t *testing.T) {
+	item := struct{ A int }{2}
+	for _, test := range []struct {
+		in   string
+		def  interface{}
+		want interface{}
+		err  string
+	}{
+		{"", string(""), "", ""},
+		{"   string   ", string(""), "   string   ", ""},
+		{"123", int(0), int(123), ""},
+		{"0x123", int(0), int(0x123), ""},
+		{"   0x123   ", int(0), int(0x123), ""},
+		{"-123", int(0), int(-123), ""},
+		{"0", false, false, ""},
+		{"1", false, true, ""},
+		{"7", false, true, `parsing "7" as bool failed: strconv.ParseBool: parsing "7": invalid syntax`},
+		{"FALSE", false, false, ""},
+		{"true", false, true, ""},
+		{"123", uint(0), uint(123), ""},
+		{"123", int64(0), int64(123), ""},
+		{"123x", int64(0), nil, "parsing \"123x\" as int64 failed: expected newline"},
+		{"truth", false, nil, "parsing \"truth\" as bool failed: strconv.ParseBool: parsing \"truth\": invalid syntax"},
+		{"struct", item, nil, "parsing \"struct\" as struct { A int } failed: don't know how to parse this type"},
+		{"1s", fs.Duration(0), fs.Duration(time.Second), ""},
+		{"1m1s", fs.Duration(0), fs.Duration(61 * time.Second), ""},
+		{"1potato", fs.Duration(0), nil, `parsing "1potato" as fs.Duration failed: parsing time "1potato" as "2006-01-02": cannot parse "1potato" as "2006"`},
+		{``, []string{}, []string{}, ""},
+		{`[]`, []string(nil), []string{}, ""},
+		{`["hello"]`, []string{}, []string{"hello"}, ""},
+		{`["hello","world!"]`, []string(nil), []string{"hello", "world!"}, ""},
+		{"1s", time.Duration(0), time.Second, ""},
+		{"1m1s", time.Duration(0), 61 * time.Second, ""},
+		{"1potato", time.Duration(0), nil, `parsing "1potato" as time.Duration failed: time: unknown unit "potato" in duration "1potato"`},
+		{"1M", fs.SizeSuffix(0), fs.Mebi, ""},
+		{"1G", fs.SizeSuffix(0), fs.Gibi, ""},
+		{"1potato", fs.SizeSuffix(0), nil, `parsing "1potato" as fs.SizeSuffix failed: bad suffix 'o'`},
+	} {
+		what := fmt.Sprintf("parse %q as %T", test.in, test.def)
+		got, err := configstruct.StringToInterface(test.def, test.in)
+		if test.err == "" {
+			require.NoError(t, err, what)
+			assert.Equal(t, test.want, got, what)
+		} else {
+			assert.Nil(t, got, what)
+			assert.EqualError(t, err, test.err, what)
+		}
+	}
+}

fs/config/configstruct/internal_test.go

@@ -1,11 +1,9 @@
 package configstruct
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestCamelToSnake(t *testing.T) {
@@ -22,39 +20,3 @@ func TestCamelToSnake(t *testing.T) {
 		assert.Equal(t, test.want, got, test.in)
 	}
 }
-
-func TestStringToInterface(t *testing.T) {
-	item := struct{ A int }{2}
-	for _, test := range []struct {
-		in   string
-		def  interface{}
-		want interface{}
-		err  string
-	}{
-		{"", string(""), "", ""},
-		{"   string   ", string(""), "   string   ", ""},
-		{"123", int(0), int(123), ""},
-		{"0x123", int(0), int(0x123), ""},
-		{"   0x123   ", int(0), int(0x123), ""},
-		{"-123", int(0), int(-123), ""},
-		{"0", false, false, ""},
-		{"1", false, true, ""},
-		{"FALSE", false, false, ""},
-		{"true", false, true, ""},
-		{"123", uint(0), uint(123), ""},
-		{"123", int64(0), int64(123), ""},
-		{"123x", int64(0), nil, "parsing \"123x\" as int64 failed: expected newline"},
-		{"truth", false, nil, "parsing \"truth\" as bool failed: syntax error scanning boolean"},
-		{"struct", item, nil, "parsing \"struct\" as struct { A int } failed: can't scan type: *struct { A int }"},
-	} {
-		what := fmt.Sprintf("parse %q as %T", test.in, test.def)
-		got, err := StringToInterface(test.def, test.in)
-		if test.err == "" {
-			require.NoError(t, err, what)
-			assert.Equal(t, test.want, got, what)
-		} else {
-			assert.Nil(t, got)
-			assert.EqualError(t, err, test.err, what)
-		}
-	}
-}