From 185166b8d3c21c483d7422aebf85d83af1ce9516 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Mon, 11 Aug 2025 11:27:06 +0200 Subject: [PATCH] sqldb+config: validate maximum batch size config value Now that the SQL query config values are configurable, we add some validation to make sure that the user doesnt set a max batch size that is larger than the limits for sqlite/postgres that have been determined by the TestSQLSliceQueries test. --- lncfg/db.go | 3 +++ sqldb/config.go | 14 ++++++++++++++ sqldb/paginate.go | 40 ++++++++++++++++++++++++++++++++++++++++ sqldb/paginate_test.go | 14 ++++++++++---- 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lncfg/db.go b/lncfg/db.go index 388d51803..927e76759 100644 --- a/lncfg/db.go +++ b/lncfg/db.go @@ -143,6 +143,9 @@ func (db *DB) Validate() error { } case SqliteBackend: + if err := db.Sqlite.Validate(); err != nil { + return err + } case PostgresBackend: if err := db.Postgres.Validate(); err != nil { return err diff --git a/sqldb/config.go b/sqldb/config.go index c5f49a98f..34de293c1 100644 --- a/sqldb/config.go +++ b/sqldb/config.go @@ -31,6 +31,15 @@ type SqliteConfig struct { QueryConfig `group:"query" namespace:"query"` } +// Validate checks that the SqliteConfig values are valid. +func (p *SqliteConfig) Validate() error { + if err := p.QueryConfig.Validate(true); err != nil { + return fmt.Errorf("invalid query config: %w", err) + } + + return nil +} + // PostgresConfig holds the postgres database configuration. // //nolint:ll @@ -42,6 +51,7 @@ type PostgresConfig struct { QueryConfig `group:"query" namespace:"query"` } +// Validate checks that the PostgresConfig values are valid. func (p *PostgresConfig) Validate() error { if p.Dsn == "" { return fmt.Errorf("DSN is required") @@ -53,5 +63,9 @@ func (p *PostgresConfig) Validate() error { return fmt.Errorf("invalid DSN: %w", err) } + if err := p.QueryConfig.Validate(false); err != nil { + return fmt.Errorf("invalid query config: %w", err) + } + return nil } diff --git a/sqldb/paginate.go b/sqldb/paginate.go index d81032508..e71f2e1c3 100644 --- a/sqldb/paginate.go +++ b/sqldb/paginate.go @@ -5,6 +5,18 @@ import ( "fmt" ) +const ( + // maxSQLiteBatchSize is the maximum number of items that can be + // included in a batch query IN clause for SQLite. This was determined + // using the TestSQLSliceQueries test. + maxSQLiteBatchSize = 32766 + + // maxPostgresBatchSize is the maximum number of items that can be + // included in a batch query IN clause for Postgres. This was determined + // using the TestSQLSliceQueries test. + maxPostgresBatchSize = 65535 +) + // QueryConfig holds configuration values for SQL queries. // //nolint:ll @@ -18,6 +30,34 @@ type QueryConfig struct { MaxPageSize int32 `long:"max-page-size" description:"The maximum number of items to return in a single page of results. This is used for paginated queries."` } +// Validate checks that the QueryConfig values are valid. +func (c *QueryConfig) Validate(sqlite bool) error { + if c.MaxBatchSize <= 0 { + return fmt.Errorf("max batch size must be greater than "+ + "zero, got %d", c.MaxBatchSize) + } + if c.MaxPageSize <= 0 { + return fmt.Errorf("max page size must be greater than "+ + "zero, got %d", c.MaxPageSize) + } + + if sqlite { + if c.MaxBatchSize > maxSQLiteBatchSize { + return fmt.Errorf("max batch size for SQLite cannot "+ + "exceed %d, got %d", maxSQLiteBatchSize, + c.MaxBatchSize) + } + } else { + if c.MaxBatchSize > maxPostgresBatchSize { + return fmt.Errorf("max batch size for Postgres cannot "+ + "exceed %d, got %d", maxPostgresBatchSize, + c.MaxBatchSize) + } + } + + return nil +} + // DefaultQueryConfig returns a default configuration for SQL queries. func DefaultQueryConfig() *QueryConfig { return &QueryConfig{ diff --git a/sqldb/paginate_test.go b/sqldb/paginate_test.go index dff0a16b1..5b0683a06 100644 --- a/sqldb/paginate_test.go +++ b/sqldb/paginate_test.go @@ -281,11 +281,17 @@ func TestSQLSliceQueries(t *testing.T) { break } - x *= 10 + // If it succeeded, we expect it to be under the maximum that + // we expect for this DB. + if isSQLite { + require.LessOrEqual(t, x, maxSQLiteBatchSize, + "SQLite should not exceed 32766 parameters") + } else { + require.LessOrEqual(t, x, maxPostgresBatchSize, + "Postgres should not exceed 65535 parameters") + } - // Just to make sure that the test doesn't carry on too long, - // we assert that we don't exceed a reasonable limit. - require.LessOrEqual(t, x, 100000) + x *= 10 } // Now that we have found the limit that the raw query can handle, we