From ca7d955693d8cc5aff75a9e230a37e5d1f8a5a42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sun, 4 Oct 2026 13:51:25 +0200 Subject: [PATCH 1/2] fix: the filter parser refuses what PostgreSQL refuses, and bounds its nesting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expression the parser accepts is spliced into a WHERE clause, so it must not be one PostgreSQL calls a syntax error. Eight shapes were accepted and are one there, or an operator that does not exist; one that runs was refused; and one input ended the process. Refused now: - `id DISTINCT FROM 1`, `id NOT DISTINCT FROM 1`: SQL writes `IS [NOT] DISTINCT FROM`, and IS is the only way in. - a hexadecimal float (`0x1p-2`): text/scanner reads Go numbers. A number is a decimal literal, for integers and floats alike (so `1_000.5`, which ran, is refused as `1_000` already was). - a word glued to a number (`id=1AND name='x'`): PostgreSQL's "trailing junk after numeric literal". The whole word is the illegal token. - `active IS YES`: YES and NO are boolean literals, not truth values of IS. - `name NOT ~~ 'x'`: NOT negates the keyword LIKE, not its symbol. - a keyword spelled with a non-ASCII letter (`lıke`, `Iſ`, `aſc`): strings.ToUpper maps 'ı' and 'ſ' to 'I' and 'S', PostgreSQL folds case in ASCII only. - a leading byte order mark, which text/scanner dropped without a token. - more than 100 levels of nesting. The parser is recursive and the input chose the depth: about 250 000 opening parentheses ended the process with a stack overflow, a fatal error no recover catches. Accepted now: a negative number (`id = -1`, `IN (-1, 2)`, `BETWEEN -5 AND 5`). A minus sign glued to a decimal literal is part of it; not after an operator written with `!` or `~`, which PostgreSQL reads as one longer operator (`!=-`). Every row of the new tests was run against PostgreSQL 18, and so was every expression the parser accepts out of nine million random token sequences: none is a syntax error. Reviewed by comparing this parser with the previous one on 43 million expressions; that found the underscore float, the two letters, the mark and the overflow. Eighteen mutants of the change, seventeen caught; the eighteenth is equivalent. Adds FuzzFilterParser: no panic, and a node or an error, never both. Co-Authored-By: Claude Opus 5.5 --- docs/configuration.md | 6 +- docs/filtering.md | 54 ++++ docs/migration.md | 55 ++++ filter_lexer.go | 110 ++++++- filter_nodes.go | 8 +- filter_parser.go | 124 ++++++-- filter_parser_regression_test.go | 6 +- filter_parser_test.go | 4 +- filter_postgres_literals_test.go | 484 +++++++++++++++++++++++++++++++ sort_parser.go | 2 +- 10 files changed, 812 insertions(+), 41 deletions(-) create mode 100644 filter_postgres_literals_test.go diff --git a/docs/configuration.md b/docs/configuration.md index d8601b0..ce872b2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -103,15 +103,17 @@ A negated **predicate** is governed by its base operator, not by `OpNot`: are allowed whenever `OpIn` / `OpLike` / `OpBetween` / `OpSimilarTo` / `OpIsDistinctFrom` (respectively) are allowed. - `OpNot` governs only the **standalone** logical `NOT`, e.g. `NOT (age > 30)`. +- `IS NOT DISTINCT FROM` is negated inside the `IS` predicate, not by a + leading `NOT`: `age NOT DISTINCT FROM 30` is refused. ```mermaid flowchart TD N["NOT appears in input"] --> K{"what follows?"} K -->|"( expr ) or a comparison"| U["UnaryOperatorNode
governed by OpNot"] - K -->|"IN / LIKE / BETWEEN /
SIMILAR TO / DISTINCT FROM"| P["predicate node with IsNot=true
governed by the base operator"] + K -->|"IN / LIKE / ILIKE /
BETWEEN / SIMILAR TO"| P["predicate node with IsNot=true
governed by the base operator"] U -. "allow-list check" .-> gnot{"OpNot allowed?"} - P -. "allow-list check" .-> gbase{"OpIn / OpLike / OpBetween /
OpSimilarTo / OpIsDistinctFrom allowed?"} + P -. "allow-list check" .-> gbase{"OpIn / OpLike / OpILike /
OpBetween / OpSimilarTo allowed?"} ``` So allowing `OpEqual` but not `OpNot` accepts `a = 1` but rejects `NOT (a = 1)`. diff --git a/docs/filtering.md b/docs/filtering.md index b8016d7..ffbf6f0 100644 --- a/docs/filtering.md +++ b/docs/filtering.md @@ -76,6 +76,50 @@ binds tighter than `OR`. **Literals**: single-quoted strings (`''` escapes a quote), integers, floats, and booleans (`TRUE`/`FALSE`/`YES`/`NO`). +### Numbers + +A number is a decimal literal, with an optional minus sign **glued to it**: +`42`, `-1`, `3.14`, `-.5`, `1.`, `1e5`, `-1e-5`. The sign is part of the +literal wherever a literal is taken (`id IN (-1, 2)`, `age BETWEEN -5 AND 5`, +`n IS DISTINCT FROM -1`); the grammar has no arithmetic, so `- 1` (with a +space), `1-1`, `+1` and `-name` are refused. + +The parser refuses what PostgreSQL refuses, so that an accepted expression +can be spliced into a `WHERE` clause: + +| Input | Why it is refused | +| --- | --- | +| `id = 1AND name = 'x'` | a word glued to a number: PostgreSQL's "trailing junk after numeric literal". (`name = 'x'AND id = 1` is fine: a string ends at its quote.) | +| `price = 0x1p-2` | a hexadecimal float is a Go literal, not a SQL one | +| `id !=-1`, `name ~~-1`, `name ~~*-1`, `name !~-1` | PostgreSQL reads the longest operator, and one written with `!` or `~` may end in `-`: this is the unknown operator `!=-`. Write `id != -1`. (`id =-1`, `id <>-1` and `id >=-1` are fine.) | +| `id = --1`, `id = 1--` | `--` opens a SQL comment | +| `id = 1e` | an exponent with no digits | + +It is also stricter than PostgreSQL on purpose: `0x10`, `0b101`, `0o17`, +`1_000` and `1_000.5` are numbers in PostgreSQL 16 and later and are refused +here. A number is written with decimal digits, a point and an exponent, and +nothing else. + +### Keywords, and what bounds an expression + +- **A keyword is ASCII, in any case.** `like`, `Like` and `LIKE` are the + keyword; `lıke` (a dotless `ı`) and `Iſ` (a long `ſ`) are not, although + Unicode upper-cases those two letters to `I` and `S`. PostgreSQL folds + case in ASCII only and reads them as names. +- **An expression does not start with a byte order mark** (U+FEFF). +- **Nesting is at most 100 levels deep**: parentheses inside parentheses, + `NOT` applied to `NOT`. The parser is recursive and the input decides how + deep it recurses; without a bound, a quarter of a megabyte of `(` ends the + process with a stack overflow, which no `recover` catches. A long flat + expression (`a = 1 AND b = 2 AND …`) is not nested and is not limited by + this. **Bound the length of what you hand to `Parse`** as well: the parser + reads its whole input before it answers. + +**The parser does not know a column's type.** `name LIKE -1`, `id = 'x'` and a +bare `-1` are accepted, and PostgreSQL refuses each for its type, not for its +syntax. What this page promises is that an accepted expression is not a +syntax error; a caller still has to answer for a value of the wrong type. + ## Nested (dot-notation) field names Field names may contain dots, so you can allow-list and filter on nested paths: @@ -109,6 +153,14 @@ flowchart LR N -. "shorthands" .- SH["field ISNULL / NOTNULL
→ IsNullNode"] ``` +`IS` is the only way in. `id DISTINCT FROM 1` and `id NOT DISTINCT FROM 1`, +without `IS`, are not SQL and are refused; so is `active IS YES`. `YES` and +`NO` are this parser's spellings of a boolean **literal** and not truth values +of the `IS` test. They are not PostgreSQL's either: `active = YES` parses to a +`LiteralNode` whose `Value` is `true` and which renders as `true`, but the +text `YES` spliced into SQL is read as a column name. Write `TRUE`/`FALSE` +where the input itself is spliced. + ## Worked examples ```go @@ -124,12 +176,14 @@ flowchart LR "age > 30" "age <> 30" // not equal "age != 30" // not equal (alias) +"balance < -10.5" // a sign glued to the number // Pattern matching "first_name LIKE 'J%'" "first_name NOT LIKE 'J%'" "first_name ILIKE 'j%'" // case-insensitive "first_name NOT ILIKE 'j%'" +"first_name !~~ 'J%'" // NOT LIKE, as an operator ("NOT ~~" is not SQL) "name SIMILAR TO 'J%n'" // SQL-standard regex "name NOT SIMILAR TO 'J%n'" diff --git a/docs/migration.md b/docs/migration.md index 061f570..3022268 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1,5 +1,60 @@ # Migration Guide +## v1.0.2 → v1.0.3 + +`v1.0.3` makes the filter parser refuse what PostgreSQL refuses as a syntax +error. A caller that passed one of these inputs through now gets the parser's +error instead of the database's. + +| Input | v1.0.2 | v1.0.3 | PostgreSQL 18 | +| --- | --- | --- | --- | +| `id DISTINCT FROM 1`, `id NOT DISTINCT FROM 1` | ✅ accepted | ❌ error | syntax error: write `IS [NOT] DISTINCT FROM` | +| `price = 0x1p-2` (a hexadecimal float) | ✅ accepted | ❌ error | trailing junk after numeric literal | +| `id=1AND name='x'` (a word glued to a number) | ✅ accepted | ❌ error | trailing junk after numeric literal | +| `active IS YES`, `active IS NOT NO` | ✅ accepted | ❌ error | syntax error: `IS` takes `TRUE`, `FALSE`, `UNKNOWN`, `NULL` | +| `name NOT ~~ 'x'`, `name NOT ~~* 'x'` | ✅ accepted | ❌ error | syntax error: write `NOT LIKE` or `!~~` | + +Three more refusals, of input no client writes by accident: + +| Input | v1.0.2 | v1.0.3 | +| --- | --- | --- | +| a keyword spelled with a non-ASCII letter (`name lıke 'x'`, `id Iſ NULL`, `name aſc` in a sort) | ✅ accepted | ❌ error: PostgreSQL folds case in ASCII only, and reads these as names | +| an expression that starts with a byte order mark (U+FEFF) | ✅ accepted, the mark dropped | ❌ error | +| an expression nested more than 100 levels deep (`((((…`, `NOT NOT NOT …`) | ✅ accepted, or **the process ended**: about 250 000 opening parentheses overflow the stack, a fatal error no `recover` catches | ❌ error | + +The last one is a reason to upgrade whatever else you do, if the expression +comes from a request: bound its length before calling `Parse`, too. + +**One input that ran is now refused:** a float written with `_` separators +(`price = 1_000.5`, `1e1_0`). v1.0.2 refused the integer form (`1_000`) and +accepted the float by accident of how each was converted; PostgreSQL 16 and +later run both. A number is now decimal digits, a point and an exponent, for +integers and floats alike. Write `1000.5`. + +**One input that was refused is now accepted:** a negative number. + +| Input | v1.0.2 | v1.0.3 | +| --- | --- | --- | +| `id = -1`, `id IN (-1, 2)`, `age BETWEEN -5 AND 5` | ❌ error | ✅ accepted | + +The sign is part of the literal: `LiteralNode.Text` is `-1` and `Value` is +`int64(-1)`. It must be glued to the number, and not to an operator written +with `!` or `~`: `id !=-1` is refused, because PostgreSQL reads `!=-` as one +operator. The parser still does not know a column's type, so a negative +number is accepted wherever a literal is (`name LIKE -1`, a bare `-1`), as a +positive one already was; PostgreSQL refuses those for their type. See +[Filtering › Numbers](filtering.md#numbers). + +**Error messages and tokens.** For an input that was and is refused, the text +can differ: `0x10`, `0b101`, `0o17` and `1_000` are `illegal token` (it was +`invalid integer`), and a word glued to a number is one illegal token named +whole (`error on field '1name': illegal token`). `Lexer` users: `-` followed +by a digit or a point is no longer its own illegal token but the start of an +`INT` or `FLOAT` whose `Value` carries the sign. + +**Migration:** replace `x [NOT] DISTINCT FROM y` with `x IS [NOT] DISTINCT FROM +y`, and write numbers without `_`. + ## v0.0.x → v0.1.0 `v0.1.0` fixes correctness bugs in the filter parser, adds the PostgreSQL 18 diff --git a/filter_lexer.go b/filter_lexer.go index f1c6c7c..319f359 100644 --- a/filter_lexer.go +++ b/filter_lexer.go @@ -36,8 +36,34 @@ func NewLexer(input string) *Lexer { return l } +// asciiUpper returns s in upper case when s is written in ASCII, and s as it +// is otherwise. +// +// It is how a keyword is recognised. SQL folds case in ASCII only, and +// strings.ToUpper does not: it maps the dotless 'ı' (U+0131) to 'I' and the +// long 'ſ' (U+017F) to 'S', so "lıke" and "Iſ" read as LIKE and IS here and +// are a syntax error in PostgreSQL. A word with a non-ASCII letter is never a +// keyword. +func asciiUpper(s string) string { + for i := 0; i < len(s); i++ { + if s[i] >= 0x80 { + return s + } + } + + return strings.ToUpper(s) +} + +// byteOrderMark is U+FEFF. text/scanner drops one at the start of its input +// without a token; PostgreSQL reads it as part of the first word. +const byteOrderMark = "\uFEFF" + // Parse reads all tokens from the scanner and buffers them. func (l *Lexer) Parse() { + if strings.HasPrefix(l.input, byteOrderMark) { + l.tokens = append(l.tokens, Token{Type: TokenIllegal, Value: byteOrderMark}) + } + for { scanTok := l.s.Scan() pos := l.s.Position @@ -53,7 +79,7 @@ func (l *Lexer) Parse() { if lit == "is" { tok = TokenIdentifier } else { - upperLit := strings.ToUpper(lit) + upperLit := asciiUpper(lit) switch upperLit { case "AND": tok = TokenOperatorAnd @@ -86,10 +112,18 @@ func (l *Lexer) Parse() { tok = TokenIdentifier } } - case scanner.Int: - tok = TokenInt - case scanner.Float: - tok = TokenFloat + case scanner.Int, scanner.Float: + tok, lit = l.numberToken(scanTok, lit) + case '-': + // A minus sign is the sign of a numeric literal when it is glued + // to one ("-1", "-.5"). Anything else stays illegal: the grammar + // has no arithmetic, and "--" opens a SQL comment. + tok = TokenIllegal + + if next := l.s.Peek(); (next == '.' || (next >= '0' && next <= '9')) && !l.gluedToOperator(pos) { + numTok := l.s.Scan() + tok, lit = l.numberToken(numTok, "-"+l.s.TokenText()) + } case scanner.String: // Built-in scanner string (double quotes) - treat as illegal for this SQL-like syntax tok = TokenIllegal // For the test cases, we need to ensure the token value matches the expected format @@ -235,6 +269,72 @@ func (l *Lexer) Parse() { } } +// numberToken classifies a number read by text/scanner, whose text is lit. +// +// text/scanner reads Go numbers, and those are not SQL's. It takes a +// hexadecimal float ("0x1p-2"), prefixed integers and '_' separators, and it +// ends a number at the first character that cannot continue it, so "1AND" is +// the number 1 followed by the keyword AND. PostgreSQL has no hexadecimal +// float and refuses a number with a word glued to it ("trailing junk after +// numeric literal"), so both are illegal tokens here: an expression this +// package accepts must be one PostgreSQL can run. +func (l *Lexer) numberToken(scanTok rune, lit string) (TokenType, string) { + // A word glued to the number: consume it, so that the error names the + // whole of what was written. + if next := l.s.Peek(); next == '_' || unicode.IsLetter(next) { + l.s.Scan() + + return TokenIllegal, lit + l.s.TokenText() + } + + if !isDecimalNumber(lit) { + return TokenIllegal, lit + } + + switch scanTok { + case scanner.Int: + return TokenInt, lit + case scanner.Float: + return TokenFloat, lit + } + + // A sign followed by something that is not a number after all ("-."). + return TokenIllegal, lit +} + +// isDecimalNumber reports whether lit is written with decimal digits, a +// decimal point, an exponent and signs only. Whether it is well formed ("1e" +// is not) is left to strconv, in the parser. +func isDecimalNumber(lit string) bool { + for _, r := range lit { + if (r < '0' || r > '9') && !strings.ContainsRune(".eE+-", r) { + return false + } + } + + return true +} + +// gluedToOperator reports whether the character at pos directly follows an +// operator written with '!' or '~'. +// +// PostgreSQL reads the longest operator it can, and an operator that contains +// '!' or '~' may end in '-': "a !=-1" is the unknown operator "!=-" applied to +// 1, not "a != -1". An operator made of '=', '<' and '>' alone cannot end in +// '-', so "a =-1" and "a <>-1" compare with minus one. +func (l *Lexer) gluedToOperator(pos scanner.Position) bool { + if len(l.tokens) == 0 { + return false + } + + prev := l.tokens[len(l.tokens)-1] + if prev.Pos.Offset+len(prev.Value) != pos.Offset { + return false + } + + return strings.ContainsAny(prev.Value, "!~") && strings.Trim(prev.Value, "!~*=<>") == "" +} + // Peek returns the next token without consuming it. func (l *Lexer) Peek() Token { if l.pos+1 >= len(l.tokens) { diff --git a/filter_nodes.go b/filter_nodes.go index 56c9ce6..3669cf0 100644 --- a/filter_nodes.go +++ b/filter_nodes.go @@ -19,8 +19,8 @@ const ( NodeTypeGroup NodeType = "GROUP" // (expression) -> (name = "John" AND age > 30) NodeTypeIsNull NodeType = "IS_NULL" // (field) -> name IS NULL NodeTypeIsNotNull NodeType = "IS_NOT_NULL" // (field) -> name IS NOT NULL - NodeTypeDistinct NodeType = "DISTINCT" // (field) -> name DISTINCT - NodeTypeNotDistinct NodeType = "NOT_DISTINCT" // (field) -> name NOT DISTINCT + NodeTypeDistinct NodeType = "DISTINCT" // (field) -> name IS DISTINCT FROM + NodeTypeNotDistinct NodeType = "NOT_DISTINCT" // (field) -> name IS NOT DISTINCT FROM NodeTypeBetween NodeType = "BETWEEN" // (field, lower, upper) -> age BETWEEN 30 AND 40 NodeTypeNotBetween NodeType = "NOT_BETWEEN" // (field, lower, upper) -> age NOT BETWEEN 30 AND 40 NodeTypeIn NodeType = "IN" // (field, values) -> name IN ("John", "Doe") @@ -218,12 +218,12 @@ func (n *InNode) String() string { } func (n *InNode) Pos() scanner.Position { return n.pos } -// DistinctNode represents a DISTINCT FROM expression (e.g., name DISTINCT FROM 'John') +// DistinctNode represents an IS [NOT] DISTINCT FROM expression (e.g., name IS DISTINCT FROM 'John') type DistinctNode struct { baseNode Field Node Value Node // the value being compared against; nil when absent - IsNot bool // true for NOT DISTINCT FROM + IsNot bool // true for IS NOT DISTINCT FROM } func (n *DistinctNode) Type() NodeType { diff --git a/filter_parser.go b/filter_parser.go index 33f569f..f7dc9fa 100644 --- a/filter_parser.go +++ b/filter_parser.go @@ -62,6 +62,8 @@ type filterParseState struct { lexer *Lexer currentToken Token errors []error + depth int + tooDeep bool } // Parse parses the filter query and returns the AST @@ -120,6 +122,12 @@ func (p *filterParseState) expect(tokenType TokenType) bool { // addError adds an error to the error list func (p *filterParseState) addError(err error) { + // Past the nesting limit the rest of the input was thrown away; what the + // unwinding levels then miss is not the caller's mistake. + if p.tooDeep { + return + } + p.errors = append(p.errors, err) } @@ -135,6 +143,36 @@ func (p *filterParseState) requireOp(op Operator) { } } +// maxFilterDepth is how deep an expression may nest: parentheses inside +// parentheses, NOT applied to NOT. The parser is recursive, and a caller's +// input decides how deep it recurses: 250 000 opening parentheses, 250 KB of +// query string, ended the process with a stack overflow, which is a fatal +// error and not a panic -- nothing recovers from it. No filter a person +// writes is a hundred levels deep. +const maxFilterDepth = 100 + +// descend enters one more level of nesting and reports whether the parser +// may go on. Past the limit it records one error and consumes the rest of +// the input, so that the levels already entered unwind without one error +// each. +func (p *filterParseState) descend() bool { + p.depth++ + if p.depth <= maxFilterDepth { + return true + } + + if !p.tooDeep { + p.addError(&QFVFilterError{Message: fmt.Sprintf("expression is nested more than %d levels deep", maxFilterDepth)}) + p.tooDeep = true + } + + for p.currentToken.Type != TokenEOF { + p.nextToken() + } + + return false +} + // parseExpression parses an expression func (p *filterParseState) parseExpression() Node { return p.parseLogicalOr() @@ -189,7 +227,14 @@ func (p *filterParseState) parseComparison() Node { pos := p.currentToken.Pos p.requireOp(OpNot) p.nextToken() + + if !p.descend() { + return &LiteralNode{} + } + expr := p.parseComparison() + p.depth-- + return &UnaryOperatorNode{ baseNode: baseNode{pos: pos}, Operator: TokenOperatorNot, @@ -201,7 +246,14 @@ func (p *filterParseState) parseComparison() Node { if p.currentToken.Type == TokenLPAREN { pos := p.currentToken.Pos p.nextToken() + + if !p.descend() { + return &LiteralNode{} + } + expr := p.parseExpression() + p.depth-- + if !p.expect(TokenRPAREN) { p.addError(&QFVFilterError{Message: "expected closing parenthesis"}) } @@ -227,7 +279,7 @@ func (p *filterParseState) parseComparison() Node { // Non-standard PostgreSQL shorthands: `field ISNULL` and `field NOTNULL`. // These are single identifier tokens (unlike the `IS [NOT] NULL` keywords). if p.currentToken.Type == TokenIdentifier { - switch strings.ToUpper(p.currentToken.Value) { + switch asciiUpper(p.currentToken.Value) { case "ISNULL": pos := p.currentToken.Pos p.requireOp(OpIsNull) @@ -275,10 +327,6 @@ func (p *filterParseState) parseComparison() Node { case TokenOperatorIsNull: p.nextToken() // Consume IS return p.parseIsPredicate(field) - case TokenOperatorDistinct: - p.requireOp(OpIsDistinctFrom) - p.nextToken() // Consume DISTINCT - return p.parseDistinctOperator(field, false) case TokenOperatorSimilarTo: p.requireOp(OpSimilarTo) p.nextToken() // Consume SIMILAR @@ -306,10 +354,12 @@ func (p *filterParseState) parseComparison() Node { IsCaseInsensitive: opToken.Type == TokenOperatorRegexMatchCI || opToken.Type == TokenOperatorNotRegexMatchCI, } case TokenOperatorNot: - // Handle NOT operators (NOT IN, NOT BETWEEN, NOT LIKE, NOT SIMILAR TO, - // NOT DISTINCT FROM). Negation is recorded on the resulting node via its - // IsNot flag (or the NOT LIKE operator), not by wrapping in a NOT node, - // so that consumers get a single, self-describing node. + // Handle NOT operators (NOT IN, NOT BETWEEN, NOT LIKE, NOT SIMILAR TO). + // Negation is recorded on the resulting node via its IsNot flag (or + // the NOT LIKE operator), not by wrapping in a NOT node, so that + // consumers get a single, self-describing node. DISTINCT FROM is not + // among them: SQL writes it IS [NOT] DISTINCT FROM, and it is parsed + // with the IS predicates. p.nextToken() // Consume NOT switch p.currentToken.Type { @@ -322,10 +372,18 @@ func (p *filterParseState) parseComparison() Node { p.nextToken() // Consume BETWEEN return p.parseBetweenOperator(field, true) case TokenOperatorLike: + if !p.isKeyword("LIKE") { + return p.unexpectedAfterNot(field) + } + p.requireOp(OpLike) p.nextToken() // Consume LIKE return p.parseLikeOperator(field, TokenOperatorNotLike) case TokenOperatorILike: + if !p.isKeyword("ILIKE") { + return p.unexpectedAfterNot(field) + } + p.requireOp(OpILike) p.nextToken() // Consume ILIKE return p.parseLikeOperator(field, TokenOperatorNotILike) @@ -333,13 +391,8 @@ func (p *filterParseState) parseComparison() Node { p.requireOp(OpSimilarTo) p.nextToken() // Consume SIMILAR return p.parseSimilarToOperator(field, true) - case TokenOperatorDistinct: - p.requireOp(OpIsDistinctFrom) - p.nextToken() // Consume DISTINCT - return p.parseDistinctOperator(field, true) default: - p.addError(&QFVFilterError{Message: fmt.Sprintf("unexpected token after NOT: %s", p.currentToken.Type)}) - return field + return p.unexpectedAfterNot(field) } default: @@ -370,7 +423,7 @@ func (p *filterParseState) parseComparisonOperator(field Node) Node { // Expects the current token to be TO after SIMILAR was consumed. func (p *filterParseState) parseSimilarToOperator(field Node, isNot bool) Node { pos := p.lexer.Current().Pos // Use position of SIMILAR token (already consumed) - if p.currentToken.Type != TokenIdentifier || strings.ToUpper(p.currentToken.Value) != "TO" { + if p.currentToken.Type != TokenIdentifier || asciiUpper(p.currentToken.Value) != "TO" { p.addError(&QFVFilterError{Message: "expected TO after SIMILAR"}) return field // Return field on error } @@ -446,7 +499,7 @@ func (p *filterParseState) parseBetweenOperator(field Node, isNot bool) Node { // Optional SYMMETRIC / ASYMMETRIC modifier (ASYMMETRIC is the default). isSymmetric := false if p.currentToken.Type == TokenIdentifier { - switch strings.ToUpper(p.currentToken.Value) { + switch asciiUpper(p.currentToken.Value) { case "SYMMETRIC": isSymmetric = true p.nextToken() @@ -474,6 +527,28 @@ func (p *filterParseState) parseBetweenOperator(field Node, isNot bool) Node { } } +// isKeyword reports whether the current token is written as the keyword word, +// in any case. A token type is not enough where SQL takes the word and not its +// symbol: LIKE and "~~" are one token type, and "NOT ~~" is not SQL. +func (p *filterParseState) isKeyword(word string) bool { + return asciiUpper(p.currentToken.Value) == word +} + +// unexpectedAfterNot records that what follows "field NOT" is not one of the +// predicates NOT negates (IN, BETWEEN, LIKE, ILIKE, SIMILAR TO). +func (p *filterParseState) unexpectedAfterNot(field Node) Node { + switch p.currentToken.Type { + case TokenOperatorLike, TokenOperatorILike: + // The symbol, not the keyword: naming the token type would tell the + // caller that LIKE is unexpected after NOT, the one spelling that is. + p.addError(&QFVFilterError{Message: fmt.Sprintf("unexpected token after NOT: %s (NOT negates LIKE and ILIKE; the operators are !~~ and !~~*)", p.currentToken.Value)}) + default: + p.addError(&QFVFilterError{Message: fmt.Sprintf("unexpected token after NOT: %s", p.currentToken.Type)}) + } + + return field +} + // parseIsPredicate parses the family of IS predicates after IS was consumed: // // IS [NOT] NULL @@ -496,22 +571,23 @@ func (p *filterParseState) parseIsPredicate(field Node) Node { p.nextToken() // Consume DISTINCT return p.parseDistinctOperator(field, isNot) - case p.currentToken.Type == TokenBoolean: - // IS [NOT] TRUE | FALSE (TRUE/YES and FALSE/NO are lexed as booleans) + case p.currentToken.Type == TokenBoolean && (p.isKeyword("TRUE") || p.isKeyword("FALSE")): + // IS [NOT] TRUE | FALSE. YES and NO are lexed as booleans too, and are + // literals only: SQL has no "IS YES". p.requireOp(OpBooleanTest) truth := BooleanTrue - if v := strings.ToUpper(p.currentToken.Value); v == "FALSE" || v == "NO" { + if p.isKeyword("FALSE") { truth = BooleanFalse } p.nextToken() // Consume TRUE/FALSE return &BooleanTestNode{baseNode: baseNode{pos: pos}, Field: field, Value: truth, IsNot: isNot} - case p.currentToken.Type == TokenIdentifier && strings.ToUpper(p.currentToken.Value) == "NULL": + case p.currentToken.Type == TokenIdentifier && asciiUpper(p.currentToken.Value) == "NULL": p.requireOp(OpIsNull) p.nextToken() // Consume NULL return &IsNullNode{baseNode: baseNode{pos: pos}, Field: field, IsNot: isNot} - case p.currentToken.Type == TokenIdentifier && strings.ToUpper(p.currentToken.Value) == "UNKNOWN": + case p.currentToken.Type == TokenIdentifier && asciiUpper(p.currentToken.Value) == "UNKNOWN": p.requireOp(OpBooleanTest) p.nextToken() // Consume UNKNOWN return &BooleanTestNode{baseNode: baseNode{pos: pos}, Field: field, Value: BooleanUnknown, IsNot: isNot} @@ -530,7 +606,7 @@ func (p *filterParseState) parseIsPredicate(field Node) Node { func (p *filterParseState) parseDistinctOperator(field Node, isNot bool) Node { pos := p.lexer.Current().Pos // Use position of DISTINCT token (already consumed) // Expect FROM (treated as identifier by lexer) - if p.currentToken.Type != TokenIdentifier || strings.ToUpper(p.currentToken.Value) != "FROM" { + if p.currentToken.Type != TokenIdentifier || asciiUpper(p.currentToken.Value) != "FROM" { p.addError(&QFVFilterError{Message: "expected FROM after DISTINCT"}) return field // Return field on error } @@ -589,7 +665,7 @@ func (p *filterParseState) parsePrimary() Node { return node case TokenBoolean: - val := strings.ToUpper(p.currentToken.Value) == "TRUE" || strings.ToUpper(p.currentToken.Value) == "YES" + val := asciiUpper(p.currentToken.Value) == "TRUE" || asciiUpper(p.currentToken.Value) == "YES" node := &LiteralNode{ baseNode: baseNode{pos: p.currentToken.Pos}, Value: val, diff --git a/filter_parser_regression_test.go b/filter_parser_regression_test.go index ac6a7bf..ffa6c8c 100644 --- a/filter_parser_regression_test.go +++ b/filter_parser_regression_test.go @@ -27,18 +27,18 @@ func TestFilterParser_RejectsTrailingTokens(t *testing.T) { } } -// TestFilterParser_DistinctPreservesValue ensures DISTINCT FROM keeps the +// TestFilterParser_DistinctPreservesValue ensures IS DISTINCT FROM keeps the // compared value and records negation on the node. func TestFilterParser_DistinctPreservesValue(t *testing.T) { p := NewFilterParser([]string{"name"}) - node, err := p.Parse("name IS NOT NULL AND name NOT DISTINCT FROM 'John'") + node, err := p.Parse("name IS NOT NULL AND name IS NOT DISTINCT FROM 'John'") if err != nil { t.Fatalf("unexpected error: %v", err) } _ = node // structure is covered in filter_parser_test.go; here we just want no error - n, err := p.Parse("name DISTINCT FROM 'John'") + n, err := p.Parse("name IS DISTINCT FROM 'John'") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/filter_parser_test.go b/filter_parser_test.go index 414c96c..374b0ba 100644 --- a/filter_parser_test.go +++ b/filter_parser_test.go @@ -494,8 +494,8 @@ func TestFilterParser_Parse(t *testing.T) { wantErr: true, // Expect error because pattern is missing }, { - name: "NOT DISTINCT FROM operator", - input: "name NOT DISTINCT FROM 'John'", + name: "IS NOT DISTINCT FROM operator", + input: "name IS NOT DISTINCT FROM 'John'", allowedFields: []string{"name"}, wantErr: false, checkNode: func(t *testing.T, node Node) { diff --git a/filter_postgres_literals_test.go b/filter_postgres_literals_test.go new file mode 100644 index 0000000..ad65298 --- /dev/null +++ b/filter_postgres_literals_test.go @@ -0,0 +1,484 @@ +package qfv + +import ( + "reflect" + "strings" + "testing" +) + +// An expression this package accepts must be one PostgreSQL can run: a caller +// splices it into a WHERE clause. Each row below was run against PostgreSQL +// 18 as `SELECT … WHERE ()`; what it answered is in the comment. + +// TestFilterParser_RefusesWhatPostgresRefuses holds shapes PostgreSQL refuses +// before it looks at a type: a syntax error (42601), or an operator that does +// not exist because the lexer read a longer one (42883). The first twelve +// were accepted by this parser; the rest it already refused, and they hold +// the minus sign to the same rule. +func TestFilterParser_RefusesWhatPostgresRefuses(t *testing.T) { + allowed := []string{"id", "name", "price", "active"} + + tests := []struct { + name string + input string + }{ + // syntax error at or near "DISTINCT": SQL writes IS DISTINCT FROM. + {name: "DISTINCT FROM without IS", input: "id DISTINCT FROM 1"}, + // syntax error at or near "NOT" + {name: "NOT DISTINCT FROM without IS", input: "id NOT DISTINCT FROM 1"}, + // trailing junk after numeric literal at or near "0x1p" + {name: "hexadecimal float", input: "price = 0x1p-2"}, + // ... at or near "0X1P" + {name: "hexadecimal float, upper case", input: "price = 0X1P+3"}, + // ... at or near ".8p1" + {name: "hexadecimal float with a fraction", input: "price = 0x1.8p1"}, + // trailing junk after numeric literal at or near "1AND" + {name: "integer glued to a keyword", input: "name=1AND id=2"}, + // ... at or near "1and" + {name: "integer glued to a keyword, lower case", input: "id=1and id=2"}, + // ... at or near "1.AND" + {name: "float glued to a keyword", input: "price = 1.AND id = 1"}, + // syntax error at or near "yes": IS takes TRUE, FALSE, UNKNOWN or NULL. + {name: "IS YES", input: "active IS yes"}, + // syntax error at or near "NO" + {name: "IS NOT NO", input: "active IS NOT NO"}, + // syntax error at or near "NOT": NOT negates the keyword, not its symbol. + {name: "NOT before ~~", input: "name NOT ~~ 'x'"}, + {name: "NOT before ~~*", input: "name NOT ~~* 'x'"}, + + // Already refused, and still: the sign does not open a way around. + // trailing junk after numeric literal at or near "1AND" + {name: "negative integer glued to a keyword", input: "id = -1AND id = 2"}, + // ... at or near "1name" + {name: "integer glued to a field", input: "id = 1name"}, + // ... at or near "1e" + {name: "exponent without digits", input: "price = 1e"}, + // operator does not exist: integer !=- integer + {name: "minus glued to !=", input: "id !=-1"}, + // operator does not exist: text ~~- integer, and so on: an operator + // written with ! or ~ may end in a minus sign. + {name: "minus glued to ~~", input: "name ~~-1"}, + {name: "minus glued to !~~", input: "name !~~-1"}, + {name: "minus glued to ~~*", input: "name ~~*-1"}, + {name: "minus glued to !~~*", input: "name !~~*-1"}, + {name: "minus glued to ~", input: "name ~-1"}, + {name: "minus glued to ~*", input: "name ~*-1"}, + {name: "minus glued to !~", input: "name !~-1"}, + {name: "minus glued to !~*", input: "name !~*-1"}, + // syntax error at or near ")" + {name: "nothing to negate", input: "id = -"}, + {name: "a sign and a point", input: "price = -."}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + node, err := NewFilterParser(allowed).Parse(tt.input) + if err == nil { + t.Fatalf("Parse(%q) = %v, want an error: PostgreSQL refuses it", tt.input, node) + } + }) + } +} + +// TestFilterParser_NotBeforeASymbolNamesTheSymbol: the error for "NOT ~~" +// names what was written. The token's type is LIKE, and "unexpected token +// after NOT: LIKE" would name the one spelling that is valid. +func TestFilterParser_NotBeforeASymbolNamesTheSymbol(t *testing.T) { + for input, symbol := range map[string]string{"name NOT ~~ 'x'": "~~", "name NOT ~~* 'x'": "~~*"} { + _, err := NewFilterParser([]string{"name"}).Parse(input) + if err == nil { + t.Fatalf("Parse(%q): no error", input) + } + + if want := "unexpected token after NOT: " + symbol + " "; !strings.Contains(err.Error(), want) { + t.Errorf("Parse(%q) says %q, want it to contain %q", input, err.Error(), want) + } + } +} + +// TestFilterParser_NumbersAreDecimal holds what the parser refuses although +// PostgreSQL 18 runs it. The grammar is deliberately the smaller one: a +// number is a decimal literal with an optional sign glued to it, and there is +// no arithmetic. +func TestFilterParser_NumbersAreDecimal(t *testing.T) { + allowed := []string{"id", "name", "price"} + + for _, input := range []string{ + "id = 0x10", // 16, since PostgreSQL 16 + "id = -0x10", // -16 + "id = 0b101", // 5 + "id = 0o17", // 15 + "id = 1_000", // 1000 + "price = 1_000.5", // 1000.5: accepted by v1.0.2, where only the integer form was refused + "price = 1e1_0", // 1e10 + "price = .5_5", // .55 + "id = - 1", // unary minus, with a space + "id = +1", // unary plus + "id = 1-1", // arithmetic + "id = 1 -1", // arithmetic + "id - 1 = 0", // arithmetic on a field + "id = -id", // a sign on a field + "id = -(1)", // a sign on a group + "id IN (- 1)", // unary minus in a list + "id BETWEEN - 1 AND 1", + } { + t.Run(input, func(t *testing.T) { + if node, err := NewFilterParser(allowed).Parse(input); err == nil { + t.Fatalf("Parse(%q) = %v, want an error", input, node) + } + }) + } +} + +// TestFilterParser_ACommentIsRefused: "--" opens a SQL comment that runs to +// the end of the line. Spliced as `WHERE ()` it takes the closing +// parenthesis with it (syntax error at end of input); on a line of its own, +// `id = -1--` runs. Neither is a filter: two minus signs are never a sign. +func TestFilterParser_ACommentIsRefused(t *testing.T) { + for _, input := range []string{"id = --1", "id = -1--", "id = 1 -- AND name = 'x'", "id = 1--"} { + t.Run(input, func(t *testing.T) { + if node, err := NewFilterParser([]string{"id", "name"}).Parse(input); err == nil { + t.Fatalf("Parse(%q) = %v, want an error", input, node) + } + }) + } +} + +// TestFilterParser_ASignOnAStringIsRefused: PostgreSQL refuses it for its +// type (operator is not unique: - unknown), this parser for its grammar. +func TestFilterParser_ASignOnAStringIsRefused(t *testing.T) { + if node, err := NewFilterParser([]string{"id"}).Parse("id = -'x'"); err == nil { + t.Fatalf("Parse = %v, want an error", node) + } +} + +// TestFilterParser_NegativeNumbers: a sign glued to a decimal literal is part +// of the literal, wherever a literal is taken. +func TestFilterParser_NegativeNumbers(t *testing.T) { + allowed := []string{"id", "price", "name"} + + tests := []struct { + input string + want string // the node's rendering + }{ + {input: "id = -1", want: "(id = -1)"}, + {input: "id =-1", want: "(id = -1)"}, + {input: "id <>-1", want: "(id <> -1)"}, + {input: "id >=-1", want: "(id >= -1)"}, + {input: "id <-1", want: "(id < -1)"}, + {input: "id != -1", want: "(id != -1)"}, + {input: "id > -1", want: "(id > -1)"}, + {input: "price = -1.5", want: "(price = -1.5)"}, + {input: "price = -.5", want: "(price = -.5)"}, + {input: "price = -1e-5", want: "(price = -1e-5)"}, + {input: "price = -1e+5", want: "(price = -1e+5)"}, + {input: "id = -9223372036854775808", want: "(id = -9223372036854775808)"}, + {input: "id IN (-1, 2)", want: "id IN (-1, 2)"}, + {input: "id IN (-1,-2)", want: "id IN (-1, -2)"}, + {input: "id BETWEEN -5 AND 5", want: "id BETWEEN -5 AND 5"}, + {input: "id NOT BETWEEN -5 AND -1", want: "id NOT BETWEEN -5 AND -1"}, + {input: "id IS NOT DISTINCT FROM -1", want: "id IS NOT DISTINCT FROM -1"}, + {input: "id = -1 AND name = 'x'", want: "((id = -1) AND (name = 'x'))"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + node, err := NewFilterParser(allowed).Parse(tt.input) + if err != nil { + t.Fatalf("Parse(%q): %v", tt.input, err) + } + + if got := node.String(); got != tt.want { + t.Errorf("Parse(%q).String() = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +// TestFilterParser_NegativeLiteralValue: the sign reaches the literal's value +// and its kind, not only its text. +func TestFilterParser_NegativeLiteralValue(t *testing.T) { + tests := []struct { + input string + value any + kind reflect.Kind + }{ + {input: "id = -42", value: int64(-42), kind: reflect.Int64}, + {input: "id = -9223372036854775808", value: int64(-9223372036854775808), kind: reflect.Int64}, + {input: "id = -1.5", value: -1.5, kind: reflect.Float64}, + {input: "id = -.5", value: -0.5, kind: reflect.Float64}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + node, err := NewFilterParser([]string{"id"}).Parse(tt.input) + if err != nil { + t.Fatalf("Parse(%q): %v", tt.input, err) + } + + cmp, ok := node.(*BinaryOperatorNode) + if !ok { + t.Fatalf("Parse(%q) = %T, want *BinaryOperatorNode", tt.input, node) + } + + lit, ok := cmp.Right.(*LiteralNode) + if !ok { + t.Fatalf("right side = %T, want *LiteralNode", cmp.Right) + } + + if lit.Value != tt.value || lit.Kind != tt.kind { + t.Errorf("literal = %v (%s), want %v (%s)", lit.Value, lit.Kind, tt.value, tt.kind) + } + }) + } +} + +// TestFilterParser_StillAcceptsWhatPostgresRuns: the new refusals took +// nothing valid with them. Each of these ran against PostgreSQL 18. +func TestFilterParser_StillAcceptsWhatPostgresRuns(t *testing.T) { + allowed := []string{"id", "name", "price", "active"} + + for _, input := range []string{ + "price = 1e5", + "price = 1E5", + "price = 1e+5", + "price = 1E+5", + "price = 1.5e-3", + "price = .5", + "price = 1.", + "id = 017", + "id=1 AND name='x'", + "name = 'x'AND id = 1", // a string ends at its quote: nothing is glued + "id IN (1,2)AND name = 'x'", + "name = '1AND'", + "name = '0x1p-2'", + "name = '-'", + "name LIKE '%-1%'", + "id IS DISTINCT FROM 1", + "id IS NOT DISTINCT FROM 1", + "active IS TRUE", + "active IS NOT false", + "name NOT LIKE 'x'", + "name not ilike 'x'", + "name !~~ 'x'", + "name !~~* 'x'", + } { + t.Run(input, func(t *testing.T) { + if _, err := NewFilterParser(allowed).Parse(input); err != nil { + t.Fatalf("Parse(%q): %v", input, err) + } + }) + } +} + +// TestLexer_NumberTokens: what the lexer makes of a number and of what is +// glued to it. +func TestLexer_NumberTokens(t *testing.T) { + tests := []struct { + input string + want []Token + }{ + {input: "-1", want: []Token{{Type: TokenInt, Value: "-1"}}}, + {input: "-1.5", want: []Token{{Type: TokenFloat, Value: "-1.5"}}}, + {input: "-.5", want: []Token{{Type: TokenFloat, Value: "-.5"}}}, + {input: "1AND", want: []Token{{Type: TokenIllegal, Value: "1AND"}}}, + {input: "-1AND", want: []Token{{Type: TokenIllegal, Value: "-1AND"}}}, + {input: "0x1p-2", want: []Token{{Type: TokenIllegal, Value: "0x1p-2"}}}, + {input: "0x10", want: []Token{{Type: TokenIllegal, Value: "0x10"}}}, + {input: "1_000", want: []Token{{Type: TokenIllegal, Value: "1_000"}}}, + {input: "1_000.5", want: []Token{{Type: TokenIllegal, Value: "1_000.5"}}}, + {input: "0b101", want: []Token{{Type: TokenIllegal, Value: "0b101"}}}, + {input: "0o17", want: []Token{{Type: TokenIllegal, Value: "0o17"}}}, + {input: "1p5", want: []Token{{Type: TokenIllegal, Value: "1p5"}}}, + {input: "-.", want: []Token{{Type: TokenIllegal, Value: "-."}}}, + {input: "-._", want: []Token{{Type: TokenIllegal, Value: "-._"}}}, + {input: "1e+5", want: []Token{{Type: TokenFloat, Value: "1e+5"}}}, + // A string that ends in an operator's character is not an operator. + {input: "'x!'-1", want: []Token{{Type: TokenString, Value: "'x!'"}, {Type: TokenInt, Value: "-1"}}}, + {input: "~~*-1", want: []Token{{Type: TokenOperatorILike, Value: "~~*"}, {Type: TokenIllegal, Value: "-"}, {Type: TokenInt, Value: "1"}}}, + {input: "!~*-1", want: []Token{{Type: TokenOperatorNotRegexMatchCI, Value: "!~*"}, {Type: TokenIllegal, Value: "-"}, {Type: TokenInt, Value: "1"}}}, + {input: "- 1", want: []Token{{Type: TokenIllegal, Value: "-"}, {Type: TokenInt, Value: "1"}}}, + {input: "--1", want: []Token{{Type: TokenIllegal, Value: "-"}, {Type: TokenInt, Value: "-1"}}}, + {input: "1-1", want: []Token{{Type: TokenInt, Value: "1"}, {Type: TokenInt, Value: "-1"}}}, + {input: "=-1", want: []Token{{Type: TokenOperatorEqual, Value: "="}, {Type: TokenInt, Value: "-1"}}}, + {input: "!=-1", want: []Token{{Type: TokenOperatorNotEqualAlias, Value: "!="}, {Type: TokenIllegal, Value: "-"}, {Type: TokenInt, Value: "1"}}}, + {input: "!= -1", want: []Token{{Type: TokenOperatorNotEqualAlias, Value: "!="}, {Type: TokenInt, Value: "-1"}}}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + l := NewLexer(tt.input) + l.Parse() + + got := l.tokens[:len(l.tokens)-1] // without EOF + if len(got) != len(tt.want) { + t.Fatalf("tokens = %v, want %v", got, tt.want) + } + + for i := range got { + if got[i].Type != tt.want[i].Type || got[i].Value != tt.want[i].Value { + t.Errorf("token %d = %s %q, want %s %q", i, got[i].Type, got[i].Value, tt.want[i].Type, tt.want[i].Value) + } + } + }) + } +} + +// FuzzFilterParser: the parser takes text a client wrote. It must not panic, +// and it answers with a node or with an error, never both and never neither. +func FuzzFilterParser(f *testing.F) { + for _, seed := range []string{ + "id = 1", "id = -1", "id =-1", "id !=-1", "name=1AND id=2", "price = 0x1p-2", + "id DISTINCT FROM 1", "id IS NOT DISTINCT FROM -1", "id IN (-1, 2)", "id BETWEEN -5 AND 5", + "name = 'it''s'", "name ~~* 'a%'", "NOT (id = 1)", "id = --1", "id = 1e", "id = -.5e+3", + "(", "'", "-", "1", "", "id = 1 AND", "a.b.c = 1", + "\uFEFFid = 1", "name lıke 'x'", "id Iſ NULL", "((((((((id = 1))))))))", "NOT NOT NOT id = 1", + } { + f.Add(seed) + } + + parser := NewFilterParser([]string{"id", "name", "price", "a.b.c"}) + + f.Fuzz(func(t *testing.T, input string) { + node, err := parser.Parse(input) + if (node == nil) == (err == nil) { + t.Fatalf("Parse(%q) = %v, %v: want a node or an error", input, node, err) + } + }) +} + +// TestFilterParser_KeywordsAreASCII: SQL folds case in ASCII only. The +// dotless 'ı' and the long 'ſ' upper-case to 'I' and 'S' in Unicode, so +// "lıke" and "Iſ" read as keywords here and were a syntax error in +// PostgreSQL ("syntax error at or near "lıke""). +func TestFilterParser_KeywordsAreASCII(t *testing.T) { + allowed := []string{"id", "name", "active"} + + for _, input := range []string{ + "name lıke 'x'", + "name ıLIKE 'x'", + "name NOT lıke 'x'", + "id ıN (1)", + "id ıS NULL", + "id Iſ NULL", + "active IS FALſE", + "active IS NOT FALſE", + "active IS UNKNOWıN", + "id ıſNULL", + "name ſIMILAR TO 'x'", + "name SIMILAR tO 'x' OR name ſimilar to 'x'", + "id IS DIſTINCT FROM 1", + "id BETWEEN ſYMMETRIC 1 AND 2", + "id = 1 AND id ıs null", + "active = FALſE", + "NOT id = 1 OR ıd = 1", + } { + t.Run(input, func(t *testing.T) { + if node, err := NewFilterParser(allowed).Parse(input); err == nil { + t.Fatalf("Parse(%q) = %v, want an error: PostgreSQL does not read it as the keyword", input, node) + } + }) + } + + // The same words in ASCII, in any case, are the keywords. + for _, input := range []string{"name like 'x'", "id In (1)", "id Is nUlL", "active IS false", "name similar To 'x'", "id is distinct from 1"} { + t.Run(input, func(t *testing.T) { + if input == "id is distinct from 1" || input == "id Is nUlL" { + // "is" in lower case is an older special case of the lexer and + // is refused; it is not this test's subject. + input = strings.Replace(input, "is ", "IS ", 1) + } + + if _, err := NewFilterParser(allowed).Parse(input); err != nil { + t.Fatalf("Parse(%q): %v", input, err) + } + }) + } +} + +// TestSortParser_DirectionsAreASCII: the same for a sort direction. +func TestSortParser_DirectionsAreASCII(t *testing.T) { + p := NewSortParser([]string{"name"}) + + for _, input := range []string{"name aſc", "name deſc", "name DEſC"} { + if nodes, err := p.Parse(input); err == nil { + t.Errorf("Parse(%q) = %v, want an error", input, nodes) + } + } + + for _, input := range []string{"name asc", "name DESC", "name Desc"} { + if _, err := p.Parse(input); err != nil { + t.Errorf("Parse(%q): %v", input, err) + } + } +} + +// TestFilterParser_AByteOrderMarkIsRefused: text/scanner drops a U+FEFF at +// the start of its input without a token, so the expression behind it was +// accepted. PostgreSQL reads the mark as part of the first word: a syntax +// error, or a column that does not exist. +func TestFilterParser_AByteOrderMarkIsRefused(t *testing.T) { + for _, input := range []string{"\uFEFFid = 1", "\uFEFF id = 1", "\uFEFFNOT id = 1", "\uFEFF-1", "\uFEFF"} { + if node, err := NewFilterParser([]string{"id"}).Parse(input); err == nil { + t.Errorf("Parse(%q) = %v, want an error", input, node) + } + } + + l := NewLexer("\uFEFFid") + l.Parse() + + if l.tokens[0].Type != TokenIllegal || l.tokens[0].Value != "\uFEFF" { + t.Errorf("first token = %s %q, want the mark as an illegal token", l.tokens[0].Type, l.tokens[0].Value) + } +} + +// TestFilterParser_NestingIsBounded: the parser is recursive and the input +// decides how deep. 250 000 opening parentheses ended the process with a +// stack overflow -- a fatal error, which nothing recovers from, so this test +// would not fail but kill the test binary. +func TestFilterParser_NestingIsBounded(t *testing.T) { + p := NewFilterParser([]string{"id"}) + + nested := func(depth int) string { + return strings.Repeat("(", depth) + "id = 1" + strings.Repeat(")", depth) + } + + if _, err := p.Parse(nested(maxFilterDepth)); err != nil { + t.Fatalf("%d levels of parentheses: %v", maxFilterDepth, err) + } + + if _, err := p.Parse(strings.Repeat("NOT ", maxFilterDepth) + "id = 1"); err != nil { + t.Fatalf("%d NOTs: %v", maxFilterDepth, err) + } + + for name, input := range map[string]string{ + "one level too many": nested(maxFilterDepth + 1), + "one NOT too many": strings.Repeat("NOT ", maxFilterDepth+1) + "id = 1", + "NOT and parentheses together": strings.Repeat("NOT (", maxFilterDepth) + "id = 1" + strings.Repeat(")", maxFilterDepth), + "a million opening parentheses": strings.Repeat("(", 1<<20), + "a million balanced": nested(1 << 20), + "a quarter of a million NOTs": strings.Repeat("NOT ", 1<<18) + "id = 1", + } { + t.Run(name, func(t *testing.T) { + node, err := p.Parse(input) + if err == nil { + t.Fatalf("Parse = %v, want an error", node) + } + + // One error, not one for each level that unwinds. + if got := strings.Count(err.Error(), "\n"); got > 2 { + t.Errorf("%d lines of error for one mistake: %.200s", got+1, err.Error()) + } + + if !strings.Contains(err.Error(), "nested more than") { + t.Errorf("the error does not say what is wrong: %.200s", err.Error()) + } + }) + } + + // Depth is how deep, not how many: a long flat expression is fine. + flat := strings.Repeat("(id = 1) AND ", 500) + "NOT id = 2" + if _, err := p.Parse(flat); err != nil { + t.Fatalf("a flat expression of 500 groups: %.200s", err) + } +} diff --git a/sort_parser.go b/sort_parser.go index 69a55a4..3b97f66 100644 --- a/sort_parser.go +++ b/sort_parser.go @@ -137,7 +137,7 @@ func (p *SortParser) Parse(input string) (SortNode, error) { } if len(sortParts) > 1 { - dirStr := strings.ToUpper(sortParts[1]) + dirStr := asciiUpper(sortParts[1]) switch dirStr { case SortDesc.String(): From b2f4258b4471e76281423eabe2d658a18d18e636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sun, 4 Oct 2026 16:45:32 +0200 Subject: [PATCH 2/2] ci: pin go-test-coverage to the last version that builds with Go 1.26 The coverage step installed the tool with "@latest". v2.20.0 requires Go 1.27, this module builds with the Go of its go.mod (1.26, GOTOOLCHAIN=local), and the job failed the day that version was published: on this pull request, and it would have failed the release workflow the same way. v2.19.0 is the last version whose go.mod says 1.26. Raise the pin together with the go directive. Checked with go1.26.8: the tool installs, the suite passes with the race detector, and the coverage thresholds are met (97.5%). Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yaml | 5 ++++- .github/workflows/release.yml | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 5b0e912..df0d7f1 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -75,7 +75,10 @@ jobs: run: | echo "## Test Coverage" >> $GITHUB_STEP_SUMMARY - go install github.com/vladopajic/go-test-coverage/v2@latest + # Pinned: v2.20.0 requires Go 1.27 and this module builds with the + # Go of its go.mod (1.26, GOTOOLCHAIN=local). "@latest" broke the job + # the day that version was published. Raise it with the go directive. + go install github.com/vladopajic/go-test-coverage/v2@v2.19.0 # execute again to get the summary echo "" >> $GITHUB_STEP_SUMMARY diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 404fba7..c903acb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -55,7 +55,10 @@ jobs: run: | echo "## Test Coverage" >> $GITHUB_STEP_SUMMARY - go install github.com/vladopajic/go-test-coverage/v2@latest + # Pinned: v2.20.0 requires Go 1.27 and this module builds with the + # Go of its go.mod (1.26, GOTOOLCHAIN=local). "@latest" broke the job + # the day that version was published. Raise it with the go directive. + go install github.com/vladopajic/go-test-coverage/v2@v2.19.0 # execute again to get the summary echo "" >> $GITHUB_STEP_SUMMARY