Hojun-Cho/got
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cli: parse each option value once

Browse Source 
validate() parsed every value to bounds-check it, then the typed getters
re-parsed the same string at read time (with a comment admitting the
failure was unreachable). A single parseValue() now validates and converts,
finalize() runs it once per option after layering, and the getters just
read the stored typed value.
This commit is contained in:
Hojun-Cho
2026-06-22 08:29:25 +09:00
parent b98c36acb7
commit 2afcb861e4
3 changed files with 102 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
cli/options.go
View File

@@ -9,15 +9,17 @@ import (
// Options is the resolved configuration: a flat name->value map plus a record
// of which names were explicitly set (so callers can tell a default from a
// chosen value). Values are kept as strings end-to-end and converted only at
// the point of use by the typed getters below.
// chosen value). The raw strings carry the layered override merge; finalize()
// then parses each one exactly once into typed, and the numeric/bool getters
// read that — no value is parsed twice.
type Options struct {
vals map[string]string
set map[string]bool
vals map[string]string
set map[string]bool
typed map[string]any // parsed value per option, filled once by finalize()
}
func newOptions() *Options {
return &Options{vals: map[string]string{}, set: map[string]bool{}}
return &Options{vals: map[string]string{}, set: map[string]bool{}, typed: map[string]any{}}
}
// IsSet reports whether name was given on the command line or in the config
@@ -44,30 +46,13 @@ func boolWord(s string) (val, ok bool) {
return val, ok
}
// Bool reports whether the value is a truthy boolean word.
func (o *Options) Bool(name string) bool {
val, _ := boolWord(o.vals[name])
return val
}
// Int returns the value as an int, or 0 if empty/invalid. The value has already
// passed validate(), so a parse failure here is unreachable in normal flow.
func (o *Options) Int(name string) int {
n, _ := strconv.ParseInt(strings.TrimSpace(o.vals[name]), 10, 64)
return int(n)
}
// Float returns the value as a float64, or 0 if empty/invalid.
func (o *Options) Float(name string) float64 {
f, _ := strconv.ParseFloat(strings.TrimSpace(o.vals[name]), 64)
return f
}
// Size returns a byte count parsed from a value like "20M" or "512K".
func (o *Options) Size(name string) int64 {
n, _ := parseSize(o.vals[name])
return n
}
// Bool, Int, Float and Size read the value finalize() already parsed and stored
// in typed; an unset option (no entry) reads as the zero value. The conversion
// happened once, at finalize, so these never re-parse a string.
func (o *Options) Bool(name string) bool { v, _ := o.typed[name].(bool); return v }
func (o *Options) Int(name string) int { v, _ := o.typed[name].(int64); return int(v) }
func (o *Options) Float(name string) float64 { v, _ := o.typed[name].(float64); return v }
func (o *Options) Size(name string) int64 { v, _ := o.typed[name].(int64); return v }
// List returns a repeatable option's accumulated values.
func (o *Options) List(name string) []string {

76
cli/parse.go
View File

@@ -55,9 +55,34 @@ func Parse(args []string) (*Result, error) {
return nil, err
}
}
if err := opts.finalize(); err != nil {
return nil, err
}
return &Result{Action: Run, Options: opts, URIs: uris}, nil
}
// finalize parses every resolved value once into its typed form, validating it
// in the same pass; the numeric/bool getters then just read the result. It runs
// after all layering, so each option is converted exactly once, from its final
// (last-wins) string — not once per layer and again per read.
func (o *Options) finalize() error {
for i := range options {
opt := &options[i]
raw, ok := o.vals[opt.Long]
if !ok {
continue
}
v, err := parseValue(opt, raw)
if err != nil {
return err
}
if v != nil {
o.typed[opt.Long] = v
}
}
return nil
}
// parseArgs walks the argument list once. It recognises --long, --long=val,
// -x, -xval, -x val and short bool bundles like -cq. Like getopt_long, it
// permutes: a non-flag token becomes a URI but scanning continues so flags may
@@ -214,16 +239,13 @@ func applyEnv(o *Options) {
}
}
// set validates a value against its option descriptor and stores it, marking
// the option as explicitly set.
// set stores a raw value, marking the option as explicitly set. Validation and
// conversion happen later, once, in finalize(); set only records the string.
func set(o *Options, name, val string) error {
opt := byLong[name]
if opt == nil {
return fmt.Errorf("unknown option %q", name)
}
if err := validate(opt, val); err != nil {
return err
}
if opt.Kind == List && o.set[name] {
val = appendList(o.vals[name], val)
}
@@ -232,52 +254,58 @@ func set(o *Options, name, val string) error {
return nil
}
func validate(o *Opt, val string) error {
// parseValue converts a raw string to the typed value for o.Kind, validating it
// in the process: it is the single place a value is parsed. Bool/Int/Size/Float
// return their typed value (read back by the getters); Str/List/Enum need no
// stored conversion and return nil, with Enum still checked for membership.
func parseValue(o *Opt, val string) (any, error) {
switch o.Kind {
case Bool:
// Booleans accept true/false plus the yes/no/1/0/on/off spellings, but
// nothing else, so e.g. --enable-dht=flase is a parse error. The accepted
// set is boolWords, the same vocabulary the readers honour.
if _, ok := boolWord(val); !ok {
return fmt.Errorf("--%s: %q is not a boolean (true/false)", o.Long, val)
// Booleans accept exactly true/false (the boolWords vocabulary the Bool
// getter honours); --enable-dht=flase is a parse error.
v, ok := boolWord(val)
if !ok {
return nil, fmt.Errorf("--%s: %q is not a boolean (true/false)", o.Long, val)
}
return v, nil
case Int:
n, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64)
if err != nil {
return fmt.Errorf("--%s: %q is not an integer", o.Long, val)
return nil, fmt.Errorf("--%s: %q is not an integer", o.Long, val)
}
if err := checkBounds(o, n); err != nil {
return err
return nil, err
}
return n, nil
case Size:
n, err := parseSize(val)
if err != nil {
return fmt.Errorf("--%s: %v", o.Long, err)
return nil, fmt.Errorf("--%s: %v", o.Long, err)
}
if err := checkBounds(o, n); err != nil {
return err
return nil, err
}
return n, nil
case Float:
// seed-time and seed-ratio are both rates/durations that must be
// non-negative; a negative SeedTime would fire the seed timer immediately
// and a negative SeedRatio is silently ignored. Reject negatives here.
// Opt.Min/Max are int64, so we only enforce the floor, not full bounds.
// seed-time and seed-ratio must be non-negative: a negative SeedTime would
// fire the seed timer immediately and a negative SeedRatio is meaningless.
f, err := strconv.ParseFloat(strings.TrimSpace(val), 64)
if err != nil {
return fmt.Errorf("--%s: %q is not a number", o.Long, val)
return nil, fmt.Errorf("--%s: %q is not a number", o.Long, val)
}
if f < 0 {
return fmt.Errorf("--%s: %v below minimum 0", o.Long, f)
return nil, fmt.Errorf("--%s: %v below minimum 0", o.Long, f)
}
return f, nil
case Enum:
for _, e := range o.Enum {
if val == e {
return nil
return nil, nil
}
}
return fmt.Errorf("--%s: %q not one of %s", o.Long, val, strings.Join(o.Enum, "|"))
return nil, fmt.Errorf("--%s: %q not one of %s", o.Long, val, strings.Join(o.Enum, "|"))
}
return nil
return nil, nil
}
// checkBounds enforces an Int/Size option's numeric range. Min defaults to a

69
cli/parse_test.go
View File

@@ -94,79 +94,79 @@ func TestParseSize(t *testing.T) {
}
}
func TestValidateRange(t *testing.T) {
func TestParseValueRange(t *testing.T) {
x := byLong["max-connection-per-server"]
if err := validate(x, "16"); err != nil {
t.Errorf("validate 16: %v", err)
if _, err := parseValue(x, "16"); err != nil {
t.Errorf("parseValue 16: %v", err)
}
if err := validate(x, "99"); err == nil {
t.Errorf("validate 99: want out-of-range error")
if _, err := parseValue(x, "99"); err == nil {
t.Errorf("parseValue 99: want out-of-range error")
}
a := byLong["file-allocation"]
if err := validate(a, "bogus"); err == nil {
t.Errorf("validate enum bogus: want error")
if _, err := parseValue(a, "bogus"); err == nil {
t.Errorf("parseValue enum bogus: want error")
}
}
func TestValidateIntFloor(t *testing.T) {
// Min:0 now enforces a floor of 0: negatives are rejected.
func TestParseValueIntFloor(t *testing.T) {
// Min:0 enforces a floor of 0: negatives are rejected.
m := byLong["max-tries"] // Min 0, "0 = unlimited" runtime meaning preserved
if err := validate(m, "0"); err != nil {
t.Errorf("validate max-tries 0: %v, want nil (0 stays a valid input)", err)
if _, err := parseValue(m, "0"); err != nil {
t.Errorf("parseValue max-tries 0: %v, want nil (0 stays a valid input)", err)
}
if err := validate(m, "-1"); err == nil {
t.Errorf("validate max-tries -1: want below-minimum error")
if _, err := parseValue(m, "-1"); err == nil {
t.Errorf("parseValue max-tries -1: want below-minimum error")
}
}
func TestValidateFloatNonNegative(t *testing.T) {
func TestParseValueFloatNonNegative(t *testing.T) {
// Float options (seed-time, seed-ratio) reject negatives: a negative
// SeedTime would fire the seed timer immediately and a negative SeedRatio
// is silently ignored, so neither is a meaningful input.
for _, name := range []string{"seed-ratio", "seed-time"} {
o := byLong[name]
if err := validate(o, "-1"); err == nil {
t.Errorf("validate %s -1: want below-minimum error", name)
if _, err := parseValue(o, "-1"); err == nil {
t.Errorf("parseValue %s -1: want below-minimum error", name)
}
if err := validate(o, "0"); err != nil {
t.Errorf("validate %s 0: %v, want nil", name, err)
if _, err := parseValue(o, "0"); err != nil {
t.Errorf("parseValue %s 0: %v, want nil", name, err)
}
if err := validate(o, "1.5"); err != nil {
t.Errorf("validate %s 1.5: %v, want nil", name, err)
if _, err := parseValue(o, "1.5"); err != nil {
t.Errorf("parseValue %s 1.5: %v, want nil", name, err)
}
}
}
func TestValidateBool(t *testing.T) {
func TestParseValueBool(t *testing.T) {
b := byLong["enable-dht"]
// Only literal true/false are accepted (case-sensitive), with surrounding
// whitespace trimmed.
for _, ok := range []string{"true", "false", " true ", "false\t"} {
if err := validate(b, ok); err != nil {
t.Errorf("validate bool %q: %v", ok, err)
if _, err := parseValue(b, ok); err != nil {
t.Errorf("parseValue bool %q: %v", ok, err)
}
}
// The invented spellings and any case variant must now fail.
// The invented spellings and any case variant must fail.
for _, bad := range []string{"yes", "no", "1", "0", "on", "off", "flase",
"True", "TRUE", "tRuE", "False", "FALSE"} {
if err := validate(b, bad); err == nil {
t.Errorf("validate bool %q: want error (only true/false accepted)", bad)
if _, err := parseValue(b, bad); err == nil {
t.Errorf("parseValue bool %q: want error (only true/false accepted)", bad)
}
}
}
func TestValidateSizeBounds(t *testing.T) {
func TestParseValueSizeBounds(t *testing.T) {
k := byLong["min-split-size"] // 1M..1024M (no G unit)
if err := validate(k, "20M"); err != nil {
t.Errorf("validate min-split-size 20M: %v", err)
if _, err := parseValue(k, "20M"); err != nil {
t.Errorf("parseValue min-split-size 20M: %v", err)
}
if err := validate(k, "512K"); err == nil {
t.Errorf("validate min-split-size 512K: want below-minimum error")
if _, err := parseValue(k, "512K"); err == nil {
t.Errorf("parseValue min-split-size 512K: want below-minimum error")
}
// 1025M exceeds the 1<<30 cap and still parses, so it tests the bounds path
// (where "2G" would have failed earlier as an unknown unit).
if err := validate(k, "1025M"); err == nil {
t.Errorf("validate min-split-size 1025M: want above-maximum error")
if _, err := parseValue(k, "1025M"); err == nil {
t.Errorf("parseValue min-split-size 1025M: want above-maximum error")
}
}
@@ -207,6 +207,9 @@ func TestOptionsGetters(t *testing.T) {
o.vals["min-split-size"] = "20M"
o.vals["continue"] = "true"
o.vals["seed-ratio"] = "1.5"
if err := o.finalize(); err != nil { // parse the raw values once, as Parse does
t.Fatalf("finalize: %v", err)
}
if o.Int("split") != 8 {
t.Errorf("Int split = %d", o.Int("split"))
}