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 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():