Name the setting and value when a size does not parse (closes #429) #452

Merged
clawbot merged 1 commits from issue-429-name-bad-size-setting into next 2026-11-11 06:42:46 +01:00
5 changed files with 214 additions and 20 deletions
Showing only changes of commit c9f3667757 - Show all commits

No files matched your search

+1 -1
View File
@@ -252,7 +252,7 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
{configWithoutRecipients, "snapshots.home.paths", "/srv",
"into []string"},
{configWithoutRecipients, "chunk_size", "fast",
"invalid size format"},
`chunk_size: "fast" is not a size`},
// Values of the right type that fail validation.
{configWithoutRecipients, "compression_level", "99",
"compression_level must be between 1 and 19"},
+5 -3
View File
@@ -290,7 +290,8 @@ func Load(path string) (*Config, error) {
}
// Parse reads YAML config data as Load does, without validating it. It
// refuses a key that no setting has, applies default values for optional
// refuses a key that no setting has and a size setting whose value is not a
// size, naming the setting. It applies default values for optional
// fields, fills in ${...} references using smartconfig, and expands ~ in
// paths.
func Parse(data []byte) (*Config, error) {
@@ -315,8 +316,9 @@ func Parse(data []byte) (*Config, error) {
return nil, fmt.Errorf("failed to load config file: %w", err)
}
// A ${...} reference can bring in keys that are not in the file.
err = checkFilledKeys(sc.Data())
// A ${...} reference can bring in keys that are not in the file, and
// is the value of a size setting only once it is filled in.
err = checkFilledConfig(sc.Data())
if err != nil {
return nil, err
}
+11 -5
View File
@@ -129,10 +129,11 @@ func checkKeys(node *yaml.Node, typ reflect.Type, path string,
return nil
}
// checkFilledKeys is checkKeys for data, the config after smartconfig has
// filled in its ${...} references. A key that a reference brings in is not
// in the file, and the copy that Encode makes has no line numbers.
func checkFilledKeys(data map[string]any) error {
// checkFilledConfig checks data, the config after smartconfig has filled in
// its ${...} references: its keys with checkKeys, then its size settings
// with checkSizes. A key that a reference brings in is not in the file, and
// the copy that Encode makes has no line numbers.
func checkFilledConfig(data map[string]any) error {
var filled yaml.Node
err := filled.Encode(data)
@@ -140,8 +141,13 @@ func checkFilledKeys(data map[string]any) error {
return fmt.Errorf("failed to marshal config data: %w", err)
}
return checkKeys(&filled, reflect.TypeFor[Config](), "",
err = checkKeys(&filled, reflect.TypeFor[Config](), "",
map[*yaml.Node]bool{}, map[nodeAndType]bool{})
if err != nil {
return err
}
return checkSizes(&filled, nil)
}
// checkMappingKeys is checkKeys for a map.
+77 -11
View File
@@ -4,13 +4,20 @@ import (
"errors"
"fmt"
"math"
"reflect"
"slices"
"strconv"
"strings"
"github.com/dustin/go-humanize"
"gopkg.in/yaml.v3"
)
var (
errSizeType = errors.New("size must be a number or string")
errSizeTooLarge = errors.New("size exceeds maximum supported value")
errNotASize = errors.New(
"is not a size; use a number of bytes or a unit such as 10MB")
)
// Size represents a byte size that can be specified in configuration files.
@@ -32,6 +39,15 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
return nil
}
// A number too large for an int64 decodes as a float64. Its text can be
// 1e+20, which is not a size string.
var floatVal float64
err = unmarshal(&floatVal)
if err == nil && floatVal > math.MaxInt64 {
return errSizeTooLarge
}
// Try to unmarshal as string
var strVal string
@@ -40,17 +56,66 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
return errSizeType
}
// Parse the string using go-humanize
bytes, err := humanize.ParseBytes(strVal)
size, err := ParseSize(strVal)
if err != nil {
return fmt.Errorf("invalid size format: %w", err)
return err
}
if bytes > math.MaxInt64 {
return fmt.Errorf("%w: %s", errSizeTooLarge, strVal)
*s = size
return nil
}
// checkSizes returns an error naming the first size setting in node whose
// value is not a size. node is the config, or the part of it at the dotted
// keys, after smartconfig has filled in its ${...} references.
func checkSizes(node *yaml.Node, keys []string) error {
typ, err := SettingType(keys)
if err != nil {
// Unknown keys are refused before this check, so keys is in one of
// two places. The env section holds no settings. A value of the
// wrong shape, such as a map given for hostname or inside exclude,
// is reported later by the YAML decoder.
return nil //nolint:nilerr // not an error here
}
*s = Size(bytes)
if typ == reflect.TypeFor[Size]() {
var size Size
err = node.Decode(&size)
if err != nil {
return fmt.Errorf("%s: %w", strings.Join(keys, "."), err)
}
return nil
}
switch node.Kind {
case yaml.DocumentNode:
for _, child := range node.Content {
err = checkSizes(child, keys)
if err != nil {
return err
}
}
case yaml.MappingNode:
for i := 0; i+1 < len(node.Content); i += 2 {
err = checkSizes(node.Content[i+1],
slices.Concat(keys, []string{node.Content[i].Value}))
if err != nil {
return err
}
}
case yaml.SequenceNode:
for i, child := range node.Content {
err = checkSizes(child, slices.Concat(keys, []string{strconv.Itoa(i)}))
if err != nil {
return err
}
}
case yaml.ScalarNode, yaml.AliasNode:
// A scalar holds no settings, and the filled config has no aliases.
}
return nil
}
@@ -70,16 +135,17 @@ func (s Size) String() string {
return humanize.Bytes(uint64(s))
}
// ParseSize parses a size string into a Size value
// ParseSize parses a size string, such as "10MB", into a Size value. An
// error quotes s.
func ParseSize(s string) (Size, error) {
bytes, err := humanize.ParseBytes(s)
bytes, err := humanize.ParseBigBytes(s)
if err != nil {
return 0, fmt.Errorf("invalid size format: %w", err)
return 0, fmt.Errorf("%q %w", s, errNotASize)
}
if bytes > math.MaxInt64 {
if !bytes.IsInt64() {
return 0, fmt.Errorf("%w: %s", errSizeTooLarge, s)
}
return Size(bytes), nil
return Size(bytes.Int64()), nil
}
+120
View File
@@ -0,0 +1,120 @@
package config //nolint:testpackage // checks errNotASize and errSizeTooLarge
import (
"errors"
"os"
"path/filepath"
"strings"
"testing"
)
// TestParseRefusesValueThatIsNotASize checks that a size setting whose
// value is not a size fails the parse with an error naming the setting and
// quoting the value, also when a ${...} reference fills the value in.
func TestParseRefusesValueThatIsNotASize(t *testing.T) {
t.Parallel()
sizePath := filepath.Join(t.TempDir(), "size")
err := os.WriteFile(sizePath, []byte("lots"), 0o600)
if err != nil {
t.Fatalf("write size: %v", err)
}
tests := []struct {
name string
yaml string
want string
}{
{"chunk_size", keysTestConfig + "chunk_size: banana\n",
`chunk_size: "banana" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"blob_size_limit", keysTestConfig + "blob_size_limit: 10XB\n",
`blob_size_limit: "10XB" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"s3.part_size", strings.Replace(keysTestConfig, " region:",
" part_size: big\n region:", 1),
`s3.part_size: "big" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
{"from a reference",
keysTestConfig + "chunk_size: ${FILE:" + sizePath + "}\n",
`chunk_size: "lots" is not a size; ` +
"use a number of bytes or a unit such as 10MB"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := Parse([]byte(tt.yaml))
if !errors.Is(err, errNotASize) {
t.Fatalf("Parse() error = %v, want errNotASize", err)
}
if err.Error() != tt.want {
t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
}
})
}
}
// TestParseRefusesSizeTooLarge checks that a size setting whose value is
// too large for a size fails the parse with an error naming the setting.
// YAML reads the second value as a float.
func TestParseRefusesSizeTooLarge(t *testing.T) {
t.Parallel()
tests := []struct {
value string
want string
}{
{"100EB", "chunk_size: size exceeds maximum supported value: 100EB"},
{"99999999999999999999",
"chunk_size: size exceeds maximum supported value"},
}
for _, tt := range tests {
t.Run(tt.value, func(t *testing.T) {
t.Parallel()
_, err := Parse([]byte(keysTestConfig + "chunk_size: " + tt.value + "\n"))
if !errors.Is(err, errSizeTooLarge) {
t.Fatalf("Parse() error = %v, want errSizeTooLarge", err)
}
if err.Error() != tt.want {
t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
}
})
}
}
// TestParseAcceptsSizes checks that size settings given as a number of
// bytes, as a unit, and through a ${...} reference still parse.
func TestParseAcceptsSizes(t *testing.T) {
t.Parallel()
sizePath := filepath.Join(t.TempDir(), "size")
err := os.WriteFile(sizePath, []byte("20MB"), 0o600)
if err != nil {
t.Fatalf("write size: %v", err)
}
data := strings.Replace(keysTestConfig, " region:",
" part_size: 6000000\n region:", 1) +
"chunk_size: ${FILE:" + sizePath + "}\n" +
"blob_size_limit: 1GB\n"
cfg, err := Parse([]byte(data))
if err != nil {
t.Fatalf("Parse() unexpected error: %v", err)
}
if cfg.ChunkSize != 20_000_000 || cfg.BlobSizeLimit != 1_000_000_000 ||
cfg.S3.PartSize != 6_000_000 {
t.Errorf("chunk_size, blob_size_limit, s3.part_size = %d, %d, %d, "+
"want 20000000, 1000000000, 6000000",
cfg.ChunkSize, cfg.BlobSizeLimit, cfg.S3.PartSize)
}
}