sneak/neoirc
1
0
Fork 0
Code Issues 10 Pull Requests 4 Actions 42 Packages Projects Releases Wiki Activity

A setting that cannot be parsed silently falls back to zero or false #219

New Issue
Open
opened 2026-10-27 16:39:14 +01:00 by clawbot · 1 comment
clawbot commented 2026-10-27 16:39:14 +01:00
Collaborator
Copy Link
Copy Source

config.New in internal/config/config.go reads every number and true/false setting with viper's GetInt, GetBool and GetFloat64. They return 0 or false for a value that does not parse, with no error. So a typo silently changes how the server runs:

  • NEOIRC_HASHCASH_BITS=twenty turns proof of work off;
  • PORT=80a makes the HTTP listener take a random port;
  • the same holds for MAX_MESSAGE_SIZE, LOGIN_RATE_LIMIT, LOGIN_RATE_BURST, DEBUG and MAINTENANCE_MODE.

A setting that is set but cannot be parsed must stop the server at startup, not fall back silently. Found by the review of #215; read from the code on 83a1585.

Fix: when one of these settings is set and does not parse as its type, startup fails with an error naming the setting and its value. Check the duration settings (MESSAGE_MAX_AGE, QUEUE_MAX_AGE, SESSION_IDLE_TIMEOUT) and every other setting config.New reads the same way, and fix any that fall back silently. An unset setting keeps its default. Change nothing else.

Definition of done

  1. Each number, true/false and duration setting that is set to a value that does not parse stops startup with an error naming it.
  2. Unset settings still take their defaults, and valid values work as before.
  3. Tests in internal/config for a bad value of each kind and for the defaults.
  4. If README.md describes how a bad setting is handled, it says what the code now does.
  5. One reviewed PR against next.

Model: opus-5-5

`config.New` in `internal/config/config.go` reads every number and true/false setting with viper's `GetInt`, `GetBool` and `GetFloat64`. They return `0` or `false` for a value that does not parse, with no error. So a typo silently changes how the server runs: - `NEOIRC_HASHCASH_BITS=twenty` turns proof of work off; - `PORT=80a` makes the HTTP listener take a random port; - the same holds for `MAX_MESSAGE_SIZE`, `LOGIN_RATE_LIMIT`, `LOGIN_RATE_BURST`, `DEBUG` and `MAINTENANCE_MODE`. A setting that is set but cannot be parsed must stop the server at startup, not fall back silently. Found by the review of https://git.eeqj.de/sneak/neoirc/pulls/215; read from the code on `83a1585`. Fix: when one of these settings is set and does not parse as its type, startup fails with an error naming the setting and its value. Check the duration settings (`MESSAGE_MAX_AGE`, `QUEUE_MAX_AGE`, `SESSION_IDLE_TIMEOUT`) and every other setting `config.New` reads the same way, and fix any that fall back silently. An unset setting keeps its default. Change nothing else. ## Definition of done 1. Each number, true/false and duration setting that is set to a value that does not parse stops startup with an error naming it. 2. Unset settings still take their defaults, and valid values work as before. 3. Tests in `internal/config` for a bad value of each kind and for the defaults. 4. If `README.md` describes how a bad setting is handled, it says what the code now does. 5. One reviewed PR against `next`. Model: opus-5-5
clawbot commented 2026-10-27 22:48:53 +01:00
Author
Collaborator
Copy Link
Copy Source

Built in #222. config.New now returns an error naming each number, true/false or duration setting whose value does not parse, so startup stops. Unset settings keep their defaults. The PR body lists two judgement calls on values viper used to accept.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/neoirc/pulls/222. `config.New` now returns an error naming each number, true/false or duration setting whose value does not parse, so startup stops. Unset settings keep their defaults. The PR body lists two judgement calls on values viper used to accept. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/neoirc#219