Config changes

- a source's interval must be greater than 0: a source is turned off with
  enabled, not with a zero interval, so JSCSourcesSource rejects an interval of
  0 or less (its validate is promoted to each source's section);
- max_message_size rejects -1: JSCServerWebSocket rejects a max_message_size
  of -1 (util.ByteSizeNoLimit), as a client could then exhaust the server's
  memory;
- every setting documents what 0 or empty means (zero/none, no limit or a
  default).

internal/config/README.md

@@ -33,30 +33,32 @@ is kept.
 | Key | Type | Default | Notes |
 |---|---|---|---|
 | `logging.level` | string | `warn` | One of `debug`, `info`, `warn`, `error` |
-| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`); other commands log to the terminal |
-| `database.file` | string | | Path to the SQLite database file, used by the `migrate` commands |
+| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`); other commands log to the terminal. Empty: log to the terminal |
+| `database.file` | string | | Path to the SQLite database file, used by the `migrate` commands. Empty: commands that need the database fail |
 | `sources.<source>.enabled` | bool | `false` | For every source |
-| `sources.<source>.interval` | duration | `10m` | For every source; e.g. `30s`, `1h` |
+| `sources.<source>.interval` | duration | `10m` | For every source; e.g. `30s`, `1h`. Must be greater than 0; turn a source off with `enabled` |
 | `sources.find_a_tender.api_root` | string | `https://www.find-tender.service.gov.uk/api/1.0` | URL of the Find a Tender API |
-| `sources.fts_mock.api_root` | string | | URL of the mock FTS server (see `internal/mock`) |
+| `sources.fts_mock.api_root` | string | | URL of the mock FTS server (see `internal/mock`); must be set when the source is enabled |
 | `sources.sleeper.block_shutdown` | bool | `false` | Test source: if true, its sleep ignores shutdown |
-| `server.host` | string | `127.0.0.1` | Address to listen on |
-| `server.port` | int | `9393` | Port to listen on |
-| `server.timeouts.shutdown` | duration | `15s` | How long shutdown waits for clients to close |
-| `server.rate_limits.idle_ttl` | duration | `10m` | How long a client's rate limiter is kept after its last request |
+| `server.host` | string | `127.0.0.1` | Address to listen on; empty: every interface |
+| `server.port` | int | `9393` | Port to listen on; `0`: any free port |
+| `server.timeouts.shutdown` | duration | `15s` | How long shutdown waits for clients to close; `0s`: until they have |
+| `server.rate_limits.idle_ttl` | duration | `10m` | How long a client's rate limiter is kept after its last request; `0s`: for ever |
 | `server.websocket.enable_compression` | bool | `false` | permessage-deflate |
-| `server.websocket.max_message_size` | byte size | `32KiB` | Largest message read from a client; `0B` for 32KiB |
+| `server.websocket.max_message_size` | byte size | `32KiB` | Largest message read from a client; `0B` for 32KiB. `-1` (no limit) is rejected, as a client could then exhaust the server's memory |
 | `server.websocket.connection_limit.*` | | | A rate limit rule for new connections from each client IP, WebSocket and plain TCP alike; limiters are kept for `server.rate_limits.idle_ttl` |
 | `server.websocket.message_rate_limit.*` | | | A rate limit rule for messages from each connection |
 | `server.websocket.write_timeout` | duration | `10s` | `0s` for none |
-| `server.websocket.ping_interval` | duration | `30s` | |
+| `server.websocket.ping_interval` | duration | `30s` | How long a connection may be idle before it is pinged; `0s`: no pings |
 | `server.websocket.pong_timeout` | duration | `10s` | `0s` for none |
 
-The current sources are `find_a_tender`, `fts_mock` and `sleeper`. See 
+The current sources are `find_a_tender`, `fts_mock` and `sleeper`. See
 "Addition of a new opportunities source" in the top level README for how to add
 a new source.
 
 A rate limit rule (`JSCRateLimitRule`) has `enabled`, `per_second` and `burst`.
+It is off unless `enabled` and `per_second` is greater than 0; a `burst` below
+1 means 1.
 It is used under `server.websocket.connection_limit` and
 `server.websocket.message_rate_limit`.
 
@@ -97,8 +99,11 @@ per key:
 - `util.ByteSize` fields work the same way, with size strings such as `"32KiB"`
   or `"1MB"` (KB, MB, ... are powers of 1000; KiB, MiB, ... powers of 1024),
   and are given out exactly in the largest unit that divides them (`"1MB"`).
+  `-1` means no limit (`util.ByteSizeNoLimit`); no other size may be
+  negative. A setting for which no limit is unsafe rejects `-1`.
 - A section can reject values by implementing `validate(key, value) error` on
-  its pointer type. `JobSearchConfigLogging` does this for `level`.
+  its pointer type. `JSCLogging` does this for `level`, and
+  `JSCServerWebSocket` for `max_message_size` (rejecting `-1`).
 
 Every section implements the `Section` interface defined in `section.go`:
 `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, each a one-line call into

internal/config/conf_database.go

@@ -6,7 +6,8 @@ import "reflect"
 const JSCKDatabase string = "database"
 
 // JSCDatabase configures the SQLite database. File is the path to
-// the database file.
+// the database file; empty means none, and commands that need the database
+// fail with a message saying so.
 type JSCDatabase struct {
 	File string `cfg:"file"`
 }

internal/config/conf_logging.go

@@ -10,7 +10,8 @@ import (
 const JSCKLogging string = "logging"
 
 // JSCLogging configures logging. Level is one of debug, info,
-// warn or error. File, if set, is where commands that log to a file write.
+// warn or error. File, if set, is where commands that log to a file write;
+// empty means they log to the terminal.
 type JSCLogging struct {
 	Level string `cfg:"level" default:"warn"`
 	File  string `cfg:"file"`

internal/config/conf_ratelimitrule.go

@@ -2,12 +2,14 @@ package config
 
 import "reflect"
 
-// JSCRateLimitRule configures one rate limit. The type is used under several
-// keys so it has no JSCK* constant and its Flatten keys are not prefixed.
+// JSCRateLimitRule configures one rate limit: PerSecond sustained, with
+// bursts of up to Burst. It is off unless Enabled and PerSecond is greater
+// than 0; a Burst below 1 means 1. The type is used under several keys so it
+// has no JSCK* constant and its Flatten keys are not prefixed.
 type JSCRateLimitRule struct {
-	Enabled   bool `cfg:"enabled"             default:"false"`
+	Enabled   bool `cfg:"enabled"    default:"false"`
 	PerSecond int  `cfg:"per_second" default:"50"`
-	Burst     int  `cfg:"burst"               default:"100"`
+	Burst     int  `cfg:"burst"      default:"100"`
 }
 
 // ---------- Section methods (see Section) ----------

internal/config/conf_sections_test.go

@@ -527,3 +527,49 @@ func TestViperValue(t *testing.T) {
 		}
 	}
 }
+
+// max_message_size can't be -1 (no limit), directly or through the whole
+// configuration; 0 (the default) and explicit sizes are accepted.
+func TestWebSocketMaxMessageSizeNoLimit(t *testing.T) {
+	var c JSCServerWebSocket
+	if err := c.SetValue("max_message_size", "-1"); err == nil ||
+		!strings.Contains(err.Error(), "can't be -1") {
+		t.Errorf("SetValue(max_message_size, -1) = %v; want rejected", err)
+	}
+	if c.MaxMessageSize != 0 {
+		t.Errorf("rejected size was stored: %v", c.MaxMessageSize)
+	}
+	for _, text := range []string{"0B", "64KiB"} {
+		if err := c.SetValue("max_message_size", text); err != nil {
+			t.Errorf("SetValue(max_message_size, %s): %v", text, err)
+		}
+	}
+	conf := defaultConfig(t)
+	if err := conf.SetValue("server.websocket.max_message_size", "-1"); err == nil {
+		t.Error("JobSearchConfig.SetValue(server.websocket.max_message_size, -1) succeeded")
+	}
+}
+
+// A source's interval must be greater than 0, through the embedded section's
+// promoted validate; a source is turned off with enabled.
+func TestSourceIntervalMustBePositive(t *testing.T) {
+	conf := defaultConfig(t)
+	for _, key := range []string{
+		"sources.find_a_tender.interval",
+		"sources.fts_mock.interval",
+		"sources.sleeper.interval",
+	} {
+		for _, value := range []string{"0s", "-1m"} {
+			err := conf.SetValue(key, value)
+			if err == nil || !strings.Contains(err.Error(), "greater than 0") {
+				t.Errorf("SetValue(%s, %s) = %v; want rejected", key, value, err)
+			}
+		}
+		if err := conf.SetValue(key, "1m"); err != nil {
+			t.Errorf("SetValue(%s, 1m): %v", key, err)
+		}
+	}
+	if conf.Sources.FindATender.Interval.Minutes() != 1 {
+		t.Errorf("FindATender.Interval = %v; want 1m", conf.Sources.FindATender.Interval)
+	}
+}

internal/config/conf_server.go

@@ -7,7 +7,9 @@ import (
 // JSCKServer is the server section's configuration key.
 const JSCKServer string = "server"
 
-// JSCServer configures the outward server.
+// JSCServer configures the outward server. Host is the address to listen
+// on (empty means every interface) and Port the port (0 means any free
+// port).
 type JSCServer struct {
 	Host       string              `cfg:"host"        default:"127.0.0.1"`
 	Port       int                 `cfg:"port"        default:"9393"`

internal/config/conf_server_ratelimits.go

@@ -9,7 +9,8 @@ import (
 const JSCKServerRateLimits string = "rate_limits"
 
 // JSCServerRateLimits configures the server's rate limiters. IdleTTL is how
-// long a client's limiter is kept after its last request.
+// long a client's limiter is kept after its last request; 0 or less means
+// limiters are never evicted.
 type JSCServerRateLimits struct {
 	IdleTTL time.Duration `cfg:"idle_ttl" default:"10m"`
 }

internal/config/conf_server_timeouts.go

@@ -9,7 +9,8 @@ import (
 const JSCKServerTimeouts string = "timeouts"
 
 // JSCServerTimeouts configures the server's timeouts. ShutdownTimeout is how
-// long shutdown waits for clients to close.
+// long shutdown waits for clients to close; 0 or less means it waits until
+// they have.
 type JSCServerTimeouts struct {
 	ShutdownTimeout time.Duration `cfg:"shutdown" default:"15s"`
 }

internal/config/conf_server_websocket.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"reflect"
 	"time"
 
@@ -11,6 +12,10 @@ import (
 const JSCKServerWebSocket string = "websocket"
 
 // JSCServerWebSocket configures the server's websocket connections.
+// MaxMessageSize is the largest message read from a client; 0 means the
+// default (32KiB), and -1 (no limit) is rejected (see validate). A
+// WriteTimeout or PongTimeout of 0 or less means no limit, and a
+// PingInterval of 0 or less turns pings off.
 type JSCServerWebSocket struct {
 	EnableCompression bool             `cfg:"enable_compression" default:"false"`
 	MaxMessageSize    util.ByteSize    `cfg:"max_message_size"   default:"32KiB"`
@@ -40,3 +45,17 @@ func (c JSCServerWebSocket) ValueOf(key string) (any, error) {
 func (c *JSCServerWebSocket) SetValue(key string, value any) error {
 	return setValue(c, key, value)
 }
+
+// ---------- Section specific methods ----------
+
+// validate checks values before they are stored (see validator).
+// max_message_size can't be -1 (no limit): a client could then send a
+// message of any size, and the server holds each message in memory.
+func (c *JSCServerWebSocket) validate(key string, value any) error {
+	if key == "max_message_size" && value.(util.ByteSize) == util.ByteSizeNoLimit {
+		return errors.New(
+			"max_message_size can't be -1 (no limit): set an explicit size, or 0 for the default",
+		)
+	}
+	return nil
+}

internal/config/conf_source_ftsmock.go

@@ -6,7 +6,8 @@ import "reflect"
 const JSCKSourceFTSMock string = "fts_mock"
 
 // JSCSourceFTSMock configures fetching from a Find a Tender mock
-// server (cmd/mock-fts) at APIRoot.
+// server (cmd/mock-fts) at APIRoot, which has no default and must be set when
+// the source is enabled.
 type JSCSourceFTSMock struct {
 	JSCSourcesSource
 	APIRoot string `cfg:"api_root"`

internal/config/conf_sources.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"reflect"
 	"time"
 )
@@ -39,7 +40,9 @@ func (c *JSCSources) SetValue(key string, value any) error {
 // ========== Generic source ==========
 
 // JSCSourcesSource holds the settings every source has, and is embedded
-// in each source's section. Interval is how often an enabled source fetches.
+// in each source's section. Interval is how often an enabled source fetches;
+// it must be greater than 0 (validate rejects 0 or less: a source is turned
+// off with Enabled, not with its interval).
 type JSCSourcesSource struct {
 	Enabled  bool          `cfg:"enabled"`
 	Interval time.Duration `cfg:"interval" default:"10m"`
@@ -67,3 +70,14 @@ func (c *JSCSourcesSource) SetValue(key string, value any) error {
 
 // Keys returns the keys directly in the section.
 func (c *JSCSourcesSource) Keys() []string { return keysOf(c) }
+
+// validate checks values before they are stored (see validator); it is
+// promoted to each source's section. interval must be greater than 0.
+func (c *JSCSourcesSource) validate(key string, value any) error {
+	if key == "interval" && value.(time.Duration) <= 0 {
+		return errors.New(
+			"interval must be greater than 0; set enabled to false to turn the source off",
+		)
+	}
+	return nil
+}