Skip to content

Commit 426e415

Browse files
fix(cypher): refuse a WITH wider than a binding can carry
execute_return_star_after_with sized its column arrays at CYP_MAX_VARS while the parser allowed up to CBM_SZ_32 items. A WITH naming 17 to 32 aliases parsed cleanly, then answered 16 columns and dropped the rest with no error. The caller could not tell that from a query that genuinely had 16 columns. Widening those arrays to 32 is the wrong fix and would make the result worse. A binding holds exactly CYP_MAX_VARS variables, and with_add_vbinding_var drops any alias past the 16th on the way in. The projection would then report 20 columns of which 4 are always blank — silent wrong data in place of a silent short answer. The 16 was not an arbitrary undersize; it matched what the binding can carry. So bound the WITH where the RETURN is already bounded, in parse_return_or_with, and refuse the query instead of answering it short. The clamp in execute_return_star_after_with stays: it can no longer fire, and it is what keeps that function correct if the bound ever moves. The test asserts both halves — a 20-alias WITH is refused, and a 16-alias WITH still succeeds with 16 columns — so the guard rejects only what the binding genuinely cannot hold. Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
1 parent 63b9997 commit 426e415

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

src/cypher/cypher.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,8 +1802,16 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_
18021802
/* Projection is materialized per row into fixed-width stack arrays sized at
18031803
* CBM_SZ_32 columns (execute_return_simple and its siblings). Bound the
18041804
* parsed item count to that width so an over-wide RETURN is rejected here
1805-
* instead of writing past those arrays downstream. */
1806-
if (r->count > CBM_SZ_32) {
1805+
* instead of writing past those arrays downstream.
1806+
*
1807+
* WITH is bounded tighter, by CYP_MAX_VARS. Every item a WITH projects
1808+
* becomes one variable of the binding that carries the rest of the query,
1809+
* and binding_t holds exactly CYP_MAX_VARS variables. A wider WITH used to
1810+
* parse, then lose every alias past the 16th in with_add_vbinding_var and
1811+
* answer with silently blank columns. Refuse it here, the same way an
1812+
* over-wide RETURN is refused, so the caller sees an error instead of a
1813+
* short or empty result. */
1814+
if (r->count > (is_with ? CYP_MAX_VARS : CBM_SZ_32)) {
18071815
free_return_clause(r);
18081816
return CBM_NOT_FOUND;
18091817
}
@@ -4286,6 +4294,8 @@ static void execute_return_star_after_with(cbm_query_t *q, binding_t *bindings,
42864294
cbm_return_clause_t *wc = q->with_clause;
42874295
char name_bufs[CYP_MAX_VARS][CBM_SZ_128];
42884296
const char *cols[CYP_MAX_VARS];
4297+
/* parse_return_or_with refuses a WITH wider than CYP_MAX_VARS, so this
4298+
* clamp cannot fire. It stays as the bound this function relies on. */
42894299
int col_n = wc->count < CYP_MAX_VARS ? wc->count : CYP_MAX_VARS;
42904300
for (int i = 0; i < col_n; i++) {
42914301
cols[i] = resolve_item_alias(&wc->items[i], name_bufs[i], sizeof(name_bufs[i]));

tests/test_cypher.c

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3026,6 +3026,45 @@ TEST(cypher_return_star_after_with_names_aliases) {
30263026
PASS();
30273027
}
30283028

3029+
TEST(cypher_wide_with_refused_not_truncated) {
3030+
/* Every item a WITH projects becomes one variable of the binding that
3031+
* carries the rest of the query, and a binding holds CYP_MAX_VARS (16) of
3032+
* them. A 20-alias WITH used to parse, drop aliases 17 to 20 inside
3033+
* with_add_vbinding_var, and answer RETURN * with 16 columns and no error —
3034+
* a short result the caller could not tell from a complete one. It has to
3035+
* be refused at parse time instead. */
3036+
char query[1024];
3037+
int off = snprintf(query, sizeof(query), "MATCH (f:Function) WITH ");
3038+
for (int i = 0; i < 20; i++) { /* 20 > CYP_MAX_VARS (16) */
3039+
off += snprintf(query + off, sizeof(query) - (size_t)off, "%sf.name AS c%d", i ? ", " : "",
3040+
i);
3041+
}
3042+
snprintf(query + off, sizeof(query) - (size_t)off, " RETURN *");
3043+
3044+
cbm_store_t *s = setup_cypher_store();
3045+
cbm_cypher_result_t r = {0};
3046+
int rc = cbm_cypher_execute(s, query, "test", 0, &r);
3047+
ASSERT_TRUE(rc != 0); /* refused, not silently narrowed to 16 columns */
3048+
cbm_cypher_result_free(&r);
3049+
3050+
/* The width just under the bound still works, so the guard rejects only
3051+
* what the binding genuinely cannot carry. */
3052+
char ok_query[1024];
3053+
off = snprintf(ok_query, sizeof(ok_query), "MATCH (f:Function) WITH ");
3054+
for (int i = 0; i < 16; i++) {
3055+
off += snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, "%sf.name AS c%d",
3056+
i ? ", " : "", i);
3057+
}
3058+
snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, " RETURN *");
3059+
cbm_cypher_result_t r16 = {0};
3060+
ASSERT_EQ(cbm_cypher_execute(s, ok_query, "test", 0, &r16), 0);
3061+
ASSERT_EQ(r16.col_count, 16);
3062+
cbm_cypher_result_free(&r16);
3063+
3064+
cbm_store_close(s);
3065+
PASS();
3066+
}
3067+
30293068
TEST(cypher_parse_neq) {
30303069
cbm_query_t *q = NULL;
30313070
char *err = NULL;
@@ -4336,6 +4375,7 @@ SUITE(cypher) {
43364375
RUN_TEST(cypher_exec_return_star);
43374376
RUN_TEST(cypher_return_star_dedups_repeated_pattern_var);
43384377
RUN_TEST(cypher_return_star_after_with_names_aliases);
4378+
RUN_TEST(cypher_wide_with_refused_not_truncated);
43394379
RUN_TEST(cypher_parse_neq);
43404380
RUN_TEST(cypher_parse_in);
43414381
RUN_TEST(cypher_parse_is_null);

0 commit comments

Comments
 (0)