diff --git a/.golangci.yml b/.golangci.yml index eeeb28be1..bd370b77e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,11 +26,11 @@ linters: enable: - asciicheck - bodyclose + - copyloopvar # - dogsled # - dupl - errcheck # - exhaustive - - exportloopref # - funlen # - gci # - gochecknoglobals diff --git a/coll/coll_test.go b/coll/coll_test.go index 02d3a9e2e..4f7da49fa 100644 --- a/coll/coll_test.go +++ b/coll/coll_test.go @@ -343,7 +343,6 @@ func TestLessThan(t *testing.T) { } for _, d := range data { - d := d t.Run(fmt.Sprintf(`LessThan("%s")(<%T>%#v,%#v)==%v`, d.key, d.left, d.left, d.right, d.out), func(t *testing.T) { assert.Equal(t, d.out, lessThan(d.key)(d.left, d.right)) }) @@ -462,7 +461,6 @@ func TestSort(t *testing.T) { } for _, d := range data { - d := d t.Run(fmt.Sprintf(`Sort("%s",<%T>)==%#v`, d.key, d.in, d.out), func(t *testing.T) { out, err := Sort(d.key, d.in) require.NoError(t, err) @@ -569,9 +567,7 @@ func BenchmarkFlatten(b *testing.B) { }, } for depth := -1; depth <= 2; depth++ { - depth := depth for _, d := range data { - d := d b.Run(fmt.Sprintf("depth%d %T(%v)", depth, d, d), func(b *testing.B) { for i := 0; i < b.N; i++ { Flatten(d, depth) diff --git a/coll/index.go b/coll/index.go index 6f145bbea..b3b2458b2 100644 --- a/coll/index.go +++ b/coll/index.go @@ -2,6 +2,7 @@ package coll import ( "fmt" + "math" "reflect" ) @@ -69,7 +70,12 @@ func indexArg(index reflect.Value, cap int) (int, error) { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: x = index.Int() case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: - x = int64(index.Uint()) + val := index.Uint() + if val > math.MaxInt64 { + return -1, fmt.Errorf("cannot index slice/array with %d (too large)", val) + } + + x = int64(val) case reflect.Invalid: return 0, fmt.Errorf("cannot index slice/array with nil") default: diff --git a/conv/conv.go b/conv/conv.go index 099f67803..fd455244c 100644 --- a/conv/conv.go +++ b/conv/conv.go @@ -194,12 +194,15 @@ func ToInt64(v interface{}) (int64, error) { case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int: return val.Int(), nil case reflect.Uint8, reflect.Uint16, reflect.Uint32: + //nolint:gosec // G115 isn't applicable, this is a Uint32 at most return int64(val.Uint()), nil case reflect.Uint, reflect.Uint64: tv := val.Uint() - // this can overflow and give -1, but IMO this is better than - // returning maxint64 + if tv > math.MaxInt64 { + return 0, fmt.Errorf("could not convert %d to int64, would overflow", tv) + } + return int64(tv), nil case reflect.Float32, reflect.Float64: return int64(val.Float()), nil diff --git a/conv/conv_test.go b/conv/conv_test.go index 6c4bd65f8..2ce0659a0 100644 --- a/conv/conv_test.go +++ b/conv/conv_test.go @@ -157,10 +157,6 @@ func TestToInt64(t *testing.T) { require.NoError(t, err) assert.Equal(t, int64(3), actual) - actual, err = ToInt64(uint64(math.MaxUint64)) - require.NoError(t, err) - assert.Equal(t, int64(-1), actual) - actual, err = ToInt64(uint8(math.MaxUint8)) require.NoError(t, err) assert.Equal(t, int64(0xFF), actual) @@ -198,6 +194,9 @@ func TestToInt64(t *testing.T) { _, err = ToInt64("foo") require.Error(t, err) + + _, err = ToInt64(uint64(math.MaxUint64)) + require.Error(t, err) }) } @@ -226,10 +225,6 @@ func TestToInt(t *testing.T) { require.NoError(t, err) assert.Equal(t, 42, actual) - actual, err = ToInt(uint64(math.MaxUint64)) - require.NoError(t, err) - assert.Equal(t, -1, actual) - actual, err = ToInt(uint8(math.MaxUint8)) require.NoError(t, err) assert.Equal(t, 0xFF, actual) @@ -267,6 +262,9 @@ func TestToInt(t *testing.T) { _, err = ToInt("foo") require.Error(t, err) + + _, err = ToInt(uint64(math.MaxUint64)) + require.Error(t, err) }) } @@ -408,7 +406,6 @@ func TestToString(t *testing.T) { } for _, d := range testdata { - d := d t.Run(fmt.Sprintf("%T/%#v == %s", d.in, d.in, d.out), func(t *testing.T) { out := ToString(d.in) assert.Equal(t, d.out, out) diff --git a/conv/evalargs.go b/conv/evalargs.go index b802e13de..f7df2ec1b 100644 --- a/conv/evalargs.go +++ b/conv/evalargs.go @@ -22,7 +22,7 @@ func printableValue(v reflect.Value) (interface{}, bool) { } if !v.Type().Implements(errorType) && !v.Type().Implements(fmtStringerType) { - if v.CanAddr() && (reflect.PtrTo(v.Type()).Implements(errorType) || reflect.PtrTo(v.Type()).Implements(fmtStringerType)) { + if v.CanAddr() && (reflect.PointerTo(v.Type()).Implements(errorType) || reflect.PointerTo(v.Type()).Implements(fmtStringerType)) { v = v.Addr() } else { switch v.Kind() { diff --git a/crypto/rsa_test.go b/crypto/rsa_test.go index 1d8d7d721..6df50ff65 100644 --- a/crypto/rsa_test.go +++ b/crypto/rsa_test.go @@ -65,7 +65,6 @@ func TestRSACrypt(t *testing.T) { } for _, d := range testdata { - d := d t.Run(d.name, func(t *testing.T) { t.Parallel() diff --git a/docs-src/content/functions/conv.yml b/docs-src/content/functions/conv.yml index 31fca92f8..76337b5e3 100644 --- a/docs-src/content/functions/conv.yml +++ b/docs-src/content/functions/conv.yml @@ -300,8 +300,7 @@ funcs: Converts the input to an `int64` (64-bit signed integer). This function attempts to convert most types of input (strings, numbers, - and booleans), but behaviour when the input can not be converted is - undefined and subject to change. + and booleans). Unconvertable inputs will result in errors. @@ -388,8 +387,7 @@ funcs: Converts the input to a `float64`. This function attempts to convert most types of input (strings, numbers, - and booleans), but behaviour when the input can not be converted is - undefined and subject to change. + and booleans). Unconvertable inputs will result in errors. arguments: diff --git a/docs/content/functions/conv.md b/docs/content/functions/conv.md index ae3370eb3..219414770 100644 --- a/docs/content/functions/conv.md +++ b/docs/content/functions/conv.md @@ -452,8 +452,7 @@ $ gomplate -i '{{ conv.ToBools false "blah" 0 }}' Converts the input to an `int64` (64-bit signed integer). This function attempts to convert most types of input (strings, numbers, -and booleans), but behaviour when the input can not be converted is -undefined and subject to change. +and booleans). Unconvertable inputs will result in errors. @@ -592,8 +591,7 @@ gomplate -i '{{ conv.ToInts true 0x42 "123,456.99" "1.2345e+3"}}' Converts the input to a `float64`. This function attempts to convert most types of input (strings, numbers, -and booleans), but behaviour when the input can not be converted is -undefined and subject to change. +and booleans). Unconvertable inputs will result in errors. diff --git a/internal/cidr/cidr.go b/internal/cidr/cidr.go index 7ac4b053f..49068f753 100644 --- a/internal/cidr/cidr.go +++ b/internal/cidr/cidr.go @@ -28,6 +28,7 @@ func SubnetBig(base netip.Prefix, newBits int, num *big.Int) (netip.Prefix, erro return netip.Prefix{}, fmt.Errorf("insufficient address space to extend prefix of %d by %d", parentLen, newBits) } + //nolint:gosec // G115 doesn't apply here maxNetNum := uint64(1< maxNetNum { return netip.Prefix{}, fmt.Errorf("prefix extension of %d does not accommodate a subnet numbered %d", newBits, num) @@ -50,17 +51,19 @@ func HostBig(base netip.Prefix, num *big.Int) (netip.Addr, error) { hostLen := addrLen - parentLen maxHostNum := big.NewInt(int64(1)) + + //nolint:gosec // G115 doesn't apply here maxHostNum.Lsh(maxHostNum, uint(hostLen)) maxHostNum.Sub(maxHostNum, big.NewInt(1)) - numUint64 := big.NewInt(int64(num.Uint64())) + num2 := big.NewInt(num.Int64()) if num.Cmp(big.NewInt(0)) == -1 { - numUint64.Neg(num) - numUint64.Sub(numUint64, big.NewInt(int64(1))) - num.Sub(maxHostNum, numUint64) + num2.Neg(num) + num2.Sub(num2, big.NewInt(int64(1))) + num.Sub(maxHostNum, num2) } - if numUint64.Cmp(maxHostNum) == 1 { + if num2.Cmp(maxHostNum) == 1 { return netip.Addr{}, fmt.Errorf("prefix of %d does not accommodate a host numbered %d", parentLen, num) } @@ -93,6 +96,8 @@ func intToIP(ipInt *big.Int, bits int) netip.Addr { func insertNumIntoIP(ip netip.Addr, bigNum *big.Int, prefixLen int) netip.Addr { ipInt, totalBits := ipToInt(ip) + + //nolint:gosec // G115 isn't relevant here bigNum.Lsh(bigNum, uint(totalBits-prefixLen)) ipInt.Or(ipInt, bigNum) return intToIP(ipInt, totalBits) diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index bdd68c339..ce59f0272 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -283,7 +283,6 @@ func TestApplyEnvVars(t *testing.T) { } for i, d := range data { - d := d t.Run(fmt.Sprintf("applyEnvVars_%s_%s/%d", d.env, d.value, i), func(t *testing.T) { t.Setenv(d.env, d.value) diff --git a/internal/conv/conv_test.go b/internal/conv/conv_test.go index 40aebaeb5..c83c8dd08 100644 --- a/internal/conv/conv_test.go +++ b/internal/conv/conv_test.go @@ -40,7 +40,6 @@ func BenchmarkInterfaceSlice(b *testing.B) { } for _, d := range data { - d := d b.Run(fmt.Sprintf("%T(%v)", d, d), func(b *testing.B) { for i := 0; i < b.N; i++ { InterfaceSlice(d) diff --git a/internal/datafs/wdfs_test.go b/internal/datafs/wdfs_test.go index c625f5fae..30fd5cf0c 100644 --- a/internal/datafs/wdfs_test.go +++ b/internal/datafs/wdfs_test.go @@ -190,7 +190,6 @@ func TestResolveLocalPath_NonWindows(t *testing.T) { } for _, td := range testdata { - td := td t.Run(td.path, func(t *testing.T) { root, path, err := ResolveLocalPath(fsys, td.path) require.NoError(t, err) @@ -225,7 +224,6 @@ func TestResolveLocalPath_Windows(t *testing.T) { } for _, td := range testdata { - td := td t.Run(td.path, func(t *testing.T) { root, path, err := ResolveLocalPath(fsys, td.path) require.NoError(t, err) @@ -286,7 +284,6 @@ func TestWdFS_ResolveLocalPath_Windows(t *testing.T) { } for _, td := range testdata { - td := td t.Run(td.path, func(t *testing.T) { root, path, err := resolveLocalPath(volname, td.path) require.NoError(t, err) @@ -324,7 +321,6 @@ func TestWin32PathType(t *testing.T) { } for _, td := range testdata { - td := td t.Run(td.path, func(t *testing.T) { assert.Equal(t, td.expected, win32PathType(td.path)) }) diff --git a/internal/funcs/conv_test.go b/internal/funcs/conv_test.go index b20f013cd..51bbba8bf 100644 --- a/internal/funcs/conv_test.go +++ b/internal/funcs/conv_test.go @@ -49,7 +49,6 @@ func TestDefault(t *testing.T) { } for _, d := range data { - d := d t.Run(fmt.Sprintf("%T/%#v empty==%v", d.val, d.val, d.empty), func(t *testing.T) { t.Parallel() diff --git a/internal/funcs/math_test.go b/internal/funcs/math_test.go index 8df794fc8..d55f973a5 100644 --- a/internal/funcs/math_test.go +++ b/internal/funcs/math_test.go @@ -221,7 +221,6 @@ func TestIsIntFloatNum(t *testing.T) { m := MathFuncs{} for _, tt := range tests { - tt := tt t.Run(fmt.Sprintf("%T(%#v)", tt.in, tt.in), func(t *testing.T) { t.Parallel() @@ -239,7 +238,6 @@ func BenchmarkIsFloat(b *testing.B) { m := MathFuncs{} for _, n := range data { - n := n b.Run(fmt.Sprintf("%T(%v)", n, n), func(b *testing.B) { for i := 0; i < b.N; i++ { m.IsFloat(n) @@ -264,7 +262,6 @@ func TestMax(t *testing.T) { {int64(255), []interface{}{"14", "0xff", -5}}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.expected), func(t *testing.T) { t.Parallel() @@ -308,7 +305,6 @@ func TestMin(t *testing.T) { {int64(-5), []interface{}{"14", "0xff", -5}}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.expected), func(t *testing.T) { t.Parallel() @@ -358,7 +354,6 @@ func TestContainsFloat(t *testing.T) { {[]interface{}{"NaN"}, true}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.expected), func(t *testing.T) { t.Parallel() @@ -386,7 +381,6 @@ func TestCeil(t *testing.T) { {-1.9, -1}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.a), func(t *testing.T) { t.Parallel() @@ -425,7 +419,6 @@ func TestFloor(t *testing.T) { {-1.9, -2.}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.a), func(t *testing.T) { t.Parallel() @@ -468,7 +461,6 @@ func TestRound(t *testing.T) { {-4.5, -5}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%v==%v", d.n, d.a), func(t *testing.T) { t.Parallel() @@ -510,7 +502,6 @@ func TestAbs(t *testing.T) { {-2, int64(2)}, } for _, d := range data { - d := d t.Run(fmt.Sprintf("%#v==%v", d.n, d.a), func(t *testing.T) { t.Parallel() diff --git a/internal/funcs/net.go b/internal/funcs/net.go index 9f69579ac..e8d8288d4 100644 --- a/internal/funcs/net.go +++ b/internal/funcs/net.go @@ -157,6 +157,8 @@ func (f *NetFuncs) CIDRNetmask(prefix interface{}) (netip.Addr, error) { // fill an appropriately sized byte slice with as many 1s as prefix bits b := make([]byte, p.Addr().BitLen()/8) for i := 0; i < p.Bits(); i++ { + //nolint:gosec // G115 is not applicable, the value was checked at parse + // time b[i/8] |= 1 << uint(7-i%8) } diff --git a/internal/funcs/strings.go b/internal/funcs/strings.go index c07e09249..a0afc0725 100644 --- a/internal/funcs/strings.go +++ b/internal/funcs/strings.go @@ -8,6 +8,7 @@ package funcs import ( "context" "fmt" + "math" "reflect" "strings" "unicode/utf8" @@ -368,22 +369,32 @@ func (StringFuncs) WordWrap(args ...interface{}) (string, error) { case string: opts.LBSeq = a default: - n, err := conv.ToInt(args[0]) + n, err := conv.ToInt64(args[0]) if err != nil { return "", fmt.Errorf("expected width to be a number: %w", err) } - opts.Width = uint(n) + if n > math.MaxUint32 { + return "", fmt.Errorf("width too large: %d", n) + } + + //nolint:gosec // G115 isn't applicable, we just checked + opts.Width = uint32(n) } } if len(args) == 3 { - n, err := conv.ToInt(args[0]) + n, err := conv.ToInt64(args[0]) if err != nil { return "", fmt.Errorf("expected width to be a number: %w", err) } - opts.Width = uint(n) + if n > math.MaxUint32 { + return "", fmt.Errorf("width too large: %d", n) + } + + //nolint:gosec // G115 isn't applicable, we just checked + opts.Width = uint32(n) opts.LBSeq = conv.ToString(args[1]) } diff --git a/internal/funcs/time.go b/internal/funcs/time.go index 532335c30..fa2f64246 100644 --- a/internal/funcs/time.go +++ b/internal/funcs/time.go @@ -3,6 +3,7 @@ package funcs import ( "context" "fmt" + "math" "strconv" "strings" gotime "time" @@ -205,6 +206,10 @@ func parseNum(in interface{}) (integral int64, fractional int64, err error) { return int64(i), 0, nil } if u, ok := in.(uint64); ok { + if u > math.MaxInt64 { + return 0, 0, fmt.Errorf("can not parse %d - would overflow int64", u) + } + return int64(u), 0, nil } if f, ok := in.(float64); ok { diff --git a/internal/iohelpers/writers_test.go b/internal/iohelpers/writers_test.go index b2bf7e6d1..42d685449 100644 --- a/internal/iohelpers/writers_test.go +++ b/internal/iohelpers/writers_test.go @@ -94,7 +94,6 @@ func TestSameSkipper(t *testing.T) { } for _, d := range testdata { - d := d t.Run(fmt.Sprintf("in:%q/out:%q/same:%v", d.in, d.out, d.same), func(t *testing.T) { r := bytes.NewBuffer(d.out) w := newBufferCloser(&bytes.Buffer{}) diff --git a/internal/tests/integration/inputdir_unix_test.go b/internal/tests/integration/inputdir_unix_test.go index 5cc77e0c6..7080f5525 100644 --- a/internal/tests/integration/inputdir_unix_test.go +++ b/internal/tests/integration/inputdir_unix_test.go @@ -33,9 +33,9 @@ func checkFileUlimit(t *testing.T, b uint64) { } func TestInputDir_RespectsUlimit(t *testing.T) { - numfiles := 32 + numfiles := uint32(32) flist := map[string]string{} - for i := 0; i < numfiles; i++ { + for i := 0; i < int(numfiles); i++ { k := fmt.Sprintf("file_%d", i) flist[k] = fmt.Sprintf("hello world %d\n", i) } @@ -65,9 +65,9 @@ func TestInputDir_RespectsUlimit(t *testing.T) { files, err := os.ReadDir(testdir.Join("out")) assert.NilError(t, err) - assert.Equal(t, numfiles, len(files)) + assert.Equal(t, int(numfiles), len(files)) - for i := 0; i < numfiles; i++ { + for i := 0; i < int(numfiles); i++ { f := testdir.Join("out", fmt.Sprintf("file_%d", i)) _, err := os.Stat(f) assert.NilError(t, err) diff --git a/internal/tests/integration/integration_test.go b/internal/tests/integration/integration_test.go index 8557a1550..ed9faf624 100644 --- a/internal/tests/integration/integration_test.go +++ b/internal/tests/integration/integration_test.go @@ -240,7 +240,6 @@ func (c *command) runInProcess() (o, e string, err error) { // iterate env vars by order of insertion for _, k := range c.envK { - k := k // clean up after ourselves if orig, ok := os.LookupEnv(k); ok { defer os.Setenv(k, orig) diff --git a/internal/texttemplate/funcs.go b/internal/texttemplate/funcs.go index bacc22478..08e6bdbbf 100644 --- a/internal/texttemplate/funcs.go +++ b/internal/texttemplate/funcs.go @@ -7,6 +7,7 @@ package texttemplate import ( "fmt" + "math" "reflect" ) @@ -17,7 +18,12 @@ func indexArg(index reflect.Value, cap int) (int, error) { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: x = index.Int() case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: - x = int64(index.Uint()) + val := index.Uint() + if val > math.MaxInt64 { + return -1, fmt.Errorf("index too large: %d", val) + } + + x = int64(val) case reflect.Invalid: return 0, fmt.Errorf("cannot index slice/array with nil") default: diff --git a/random/random_test.go b/random/random_test.go index c764d1637..1e76298e9 100644 --- a/random/random_test.go +++ b/random/random_test.go @@ -27,7 +27,6 @@ func TestMatchChars(t *testing.T) { } for i, d := range testdata { - d := d t.Run(strconv.Itoa(i), func(t *testing.T) { t.Parallel() diff --git a/strings/strings.go b/strings/strings.go index 248ae9b17..aece28a20 100644 --- a/strings/strings.go +++ b/strings/strings.go @@ -111,7 +111,7 @@ type WordWrapOpts struct { LBSeq string // The desired maximum line length in characters (defaults to 80) - Width uint + Width uint32 } // applies default options diff --git a/strings/strings_fuzz_test.go b/strings/strings_fuzz_test.go index 52472c4a0..33f73bab6 100644 --- a/strings/strings_fuzz_test.go +++ b/strings/strings_fuzz_test.go @@ -61,11 +61,11 @@ things very badly. To wit: https://example.com/a/super-long/url/that-shouldnt-be?wrapped=for+fear+of#the-breaking-of-functionality should appear on its own line, regardless of the desired word-wrapping width that has been set.` - f.Add(out, "", uint(0)) - f.Add(out, "\n", uint(80)) - f.Add(out, "\v", uint(10)) + f.Add(out, "", uint32(0)) + f.Add(out, "\n", uint32(80)) + f.Add(out, "\v", uint32(10)) - f.Fuzz(func(t *testing.T, in, lbSeq string, width uint) { + f.Fuzz(func(t *testing.T, in, lbSeq string, width uint32) { for _, r := range lbSeq { if !unicode.IsSpace(r) { t.Skip("ignore non-whitespace sequences") diff --git a/test/test.go b/test/test.go index a71b8bcef..6a6b20359 100644 --- a/test/test.go +++ b/test/test.go @@ -3,6 +3,7 @@ package test import ( + "errors" "fmt" ) @@ -12,7 +13,7 @@ func Assert(value bool, message string) (string, error) { if message != "" { return "", fmt.Errorf("assertion failed: %s", message) } - return "", fmt.Errorf("assertion failed") + return "", errors.New("assertion failed") } return "", nil } @@ -22,7 +23,7 @@ func Fail(message string) error { if message != "" { return fmt.Errorf("template generation failed: %s", message) } - return fmt.Errorf("template generation failed") + return errors.New("template generation failed") } // Required - @@ -32,7 +33,7 @@ func Required(message string, value interface{}) (interface{}, error) { } if s, ok := value.(string); value == nil || (ok && s == "") { - return nil, fmt.Errorf(message) + return nil, errors.New(message) } return value, nil