Skip to content

Commit 738174f

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 0c0eb95 commit 738174f

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
}
@@ -4249,6 +4257,8 @@ static void execute_return_star_after_with(cbm_query_t *q, binding_t *bindings,
42494257
cbm_return_clause_t *wc = q->with_clause;
42504258
char name_bufs[CYP_MAX_VARS][CBM_SZ_128];
42514259
const char *cols[CYP_MAX_VARS];
4260+
/* parse_return_or_with refuses a WITH wider than CYP_MAX_VARS, so this
4261+
* clamp cannot fire. It stays as the bound this function relies on. */
42524262
int col_n = wc->count < CYP_MAX_VARS ? wc->count : CYP_MAX_VARS;
42534263
for (int i = 0; i < col_n; i++) {
42544264
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
@@ -2903,6 +2903,45 @@ TEST(cypher_return_star_after_with_names_aliases) {
29032903
PASS();
29042904
}
29052905

2906+
TEST(cypher_wide_with_refused_not_truncated) {
2907+
/* Every item a WITH projects becomes one variable of the binding that
2908+
* carries the rest of the query, and a binding holds CYP_MAX_VARS (16) of
2909+
* them. A 20-alias WITH used to parse, drop aliases 17 to 20 inside
2910+
* with_add_vbinding_var, and answer RETURN * with 16 columns and no error —
2911+
* a short result the caller could not tell from a complete one. It has to
2912+
* be refused at parse time instead. */
2913+
char query[1024];
2914+
int off = snprintf(query, sizeof(query), "MATCH (f:Function) WITH ");
2915+
for (int i = 0; i < 20; i++) { /* 20 > CYP_MAX_VARS (16) */
2916+
off += snprintf(query + off, sizeof(query) - (size_t)off, "%sf.name AS c%d", i ? ", " : "",
2917+
i);
2918+
}
2919+
snprintf(query + off, sizeof(query) - (size_t)off, " RETURN *");
2920+
2921+
cbm_store_t *s = setup_cypher_store();
2922+
cbm_cypher_result_t r = {0};
2923+
int rc = cbm_cypher_execute(s, query, "test", 0, &r);
2924+
ASSERT_TRUE(rc != 0); /* refused, not silently narrowed to 16 columns */
2925+
cbm_cypher_result_free(&r);
2926+
2927+
/* The width just under the bound still works, so the guard rejects only
2928+
* what the binding genuinely cannot carry. */
2929+
char ok_query[1024];
2930+
off = snprintf(ok_query, sizeof(ok_query), "MATCH (f:Function) WITH ");
2931+
for (int i = 0; i < 16; i++) {
2932+
off += snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, "%sf.name AS c%d",
2933+
i ? ", " : "", i);
2934+
}
2935+
snprintf(ok_query + off, sizeof(ok_query) - (size_t)off, " RETURN *");
2936+
cbm_cypher_result_t r16 = {0};
2937+
ASSERT_EQ(cbm_cypher_execute(s, ok_query, "test", 0, &r16), 0);
2938+
ASSERT_EQ(r16.col_count, 16);
2939+
cbm_cypher_result_free(&r16);
2940+
2941+
cbm_store_close(s);
2942+
PASS();
2943+
}
2944+
29062945
TEST(cypher_parse_neq) {
29072946
cbm_query_t *q = NULL;
29082947
char *err = NULL;
@@ -4207,6 +4246,7 @@ SUITE(cypher) {
42074246
RUN_TEST(cypher_exec_return_star);
42084247
RUN_TEST(cypher_return_star_dedups_repeated_pattern_var);
42094248
RUN_TEST(cypher_return_star_after_with_names_aliases);
4249+
RUN_TEST(cypher_wide_with_refused_not_truncated);
42104250
RUN_TEST(cypher_parse_neq);
42114251
RUN_TEST(cypher_parse_in);
42124252
RUN_TEST(cypher_parse_is_null);

0 commit comments

Comments
 (0)