diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 36c07ea4c..dab5eb8e6 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -1817,20 +1817,27 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_ } } - /* Optional SKIP */ + /* Optional SKIP. A non-numeric operand is a loud parse error: expect() has + * already filled p->error, and dropping the clause instead would leave + * r->skip at its "none" default while the query still reports success. */ if (match(p, TOK_SKIP)) { const cbm_token_t *num = expect(p, TOK_NUMBER); - if (num) { - r->skip = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE); + if (!num) { + free_return_clause(r); + return CBM_NOT_FOUND; } + r->skip = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE); } - /* Optional LIMIT */ + /* Optional LIMIT. Same rule: a dropped LIMIT is the failure mode #1334 + * banned - the caller would silently receive the whole result set. */ if (match(p, TOK_LIMIT)) { const cbm_token_t *num = expect(p, TOK_NUMBER); - if (num) { - r->limit = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE); + if (!num) { + free_return_clause(r); + return CBM_NOT_FOUND; } + r->limit = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE); } *out = r; diff --git a/tests/test_cypher.c b/tests/test_cypher.c index 7882be82f..4d72a9097 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -420,6 +420,53 @@ TEST(cypher_parse_order_by_over_cap_rejected_issue1334) { PASS(); } +/* #1994: a non-numeric SKIP/LIMIT operand must be a loud parse error too. The old + * failure mode was the same one #1334 banned by a different route: expect() + * returned NULL, the clause was dropped, and the query still reported success + * with limit left at its -1 "no LIMIT" sentinel - so query_graph answered a + * bounded query with the entire result set. Cypher parameters ($limit) are the + * everyday trigger: they are what a Neo4j-shaped client writes by default. */ +TEST(cypher_parse_nonnumeric_limit_rejected_issue1994) { + cbm_query_t *q = NULL; + char *err = NULL; + int rc = cbm_cypher_parse("MATCH (f:Function) RETURN f.name LIMIT $limit", &q, &err); + ASSERT(rc != 0); + free(err); + PASS(); +} + +TEST(cypher_parse_nonnumeric_skip_rejected_issue1994) { + cbm_query_t *q = NULL; + char *err = NULL; + /* The orphaned operand also swallowed the LIMIT that followed it. */ + int rc = cbm_cypher_parse("MATCH (f:Function) RETURN f.name SKIP $offset LIMIT 10", &q, &err); + ASSERT(rc != 0); + free(err); + PASS(); +} + +TEST(cypher_parse_word_limit_operand_rejected_issue1994) { + cbm_query_t *q = NULL; + char *err = NULL; + int rc = cbm_cypher_parse("MATCH (f:Function) RETURN f.name LIMIT abc", &q, &err); + ASSERT(rc != 0); + free(err); + PASS(); +} + +/* Control: a well-formed SKIP/LIMIT still parses and still carries its values. */ +TEST(cypher_parse_numeric_skip_limit_still_accepted) { + cbm_query_t *q = NULL; + char *err = NULL; + int rc = cbm_cypher_parse("MATCH (f:Function) RETURN f.name SKIP 2 LIMIT 10", &q, &err); + ASSERT_EQ(rc, 0); + ASSERT_EQ(q->ret->skip, 2); + ASSERT_EQ(q->ret->limit, 10); + + cbm_query_free(q); + PASS(); +} + TEST(cypher_parse_return_distinct) { cbm_query_t *q = NULL; char *err = NULL; @@ -4116,6 +4163,10 @@ SUITE(cypher) { RUN_TEST(cypher_parse_return_order_limit); RUN_TEST(cypher_parse_multikey_order_by_issue1334); RUN_TEST(cypher_parse_order_by_over_cap_rejected_issue1334); + RUN_TEST(cypher_parse_nonnumeric_limit_rejected_issue1994); + RUN_TEST(cypher_parse_nonnumeric_skip_rejected_issue1994); + RUN_TEST(cypher_parse_word_limit_operand_rejected_issue1994); + RUN_TEST(cypher_parse_numeric_skip_limit_still_accepted); RUN_TEST(cypher_parse_return_distinct); RUN_TEST(cypher_parse_inline_props); RUN_TEST(cypher_parse_error);