Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/store/col.c
Original file line number Diff line number Diff line change
Expand Up @@ -580,14 +580,28 @@ static void try_load_link_sidecar(ray_t* vec, const char* path) {
memcpy(link_path + plen, ".link", 6);
FILE* lf = fopen(link_path, "rb");
if (!lf) return;
char buf[256];
size_t n = fread(buf, 1, sizeof(buf) - 1, lf);
/* Read the whole sidecar. The target sym name can be longer than any fixed
* buffer, and a truncated read would intern a DIFFERENT symbol — silently
* linking the column to the wrong table. Size to the file, capped so a
* corrupt/oversized sidecar can't force a huge allocation. */
if (fseek(lf, 0, SEEK_END) != 0) { fclose(lf); return; }
long fsz = ftell(lf);
if (fsz <= 0 || fsz > (1L << 20)) { fclose(lf); return; }
rewind(lf);
char* buf = (char*)ray_alloc_raw((size_t)fsz);
if (!buf) { fclose(lf); return; }
size_t n = fread(buf, 1, (size_t)fsz, lf);
fclose(lf);
/* Reject a short read (I/O error, or the sidecar shrank after the size
* check above): interning a partial name would link the WRONG symbol —
* the very failure this reader guards against. */
if (n != (size_t)fsz) { ray_free_raw(buf); return; }
while (n > 0 && (buf[n-1] == '\n' || buf[n-1] == '\r'
|| buf[n-1] == ' ' || buf[n-1] == '\t'
|| buf[n-1] == '\0')) n--;
if (n == 0) return;
if (n == 0) { ray_free_raw(buf); return; }
int64_t target_sym = ray_sym_intern(buf, n);
ray_free_raw(buf);
if (target_sym < 0) return;
vec->link_target = target_sym;
vec->attrs |= RAY_ATTR_HAS_LINK;
Expand Down
42 changes: 42 additions & 0 deletions test/test_link.c
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,47 @@ static test_result_t test_link_persistence_roundtrip(void) {
PASS();
}

/* Regression: a link sidecar whose target sym name is longer than the old
* 256-byte read buffer must round-trip intact. The buggy reader truncated to
* 255 bytes and interned a DIFFERENT symbol, silently linking the column to the
* wrong table. Use a 300-byte name and assert the loaded link_target still
* resolves to the original symbol. */
static test_result_t test_link_persistence_long_target_name(void) {
char longname[300];
memset(longname, 'a', sizeof(longname)); /* 300 bytes, > 255 */
int64_t long_sym = ray_sym_intern(longname, sizeof(longname));
TEST_ASSERT_TRUE(long_sym >= 0);

ray_t* target = build_target_table("custs");
ray_env_set(long_sym, target);
ray_release(target);

int64_t rids[] = { 0, 1, 2 };
ray_t* w = make_i64_vec(rids, 3);
TEST_ASSERT_FALSE(RAY_IS_ERR(ray_link_attach(&w, long_sym)));
TEST_ASSERT_EQ_I(w->link_target, long_sym);

char path[] = "/tmp/link_long_name_test_XXXXXX";
int fd = mkstemp(path);
TEST_ASSERT_TRUE(fd >= 0);
close(fd);
TEST_ASSERT_EQ_I(ray_col_save(w, path), RAY_OK);

ray_t* loaded = ray_col_load(path);
TEST_ASSERT_FALSE(RAY_IS_ERR(loaded));
TEST_ASSERT_TRUE(loaded->attrs & RAY_ATTR_HAS_LINK);
/* Fails if the reader truncated the sidecar to 255 bytes (wrong symbol). */
TEST_ASSERT_EQ_I(loaded->link_target, long_sym);

char link_path[512];
snprintf(link_path, sizeof link_path, "%s.link", path);
unlink(path);
unlink(link_path);
ray_release(loaded);
ray_release(w);
PASS();
}

/* ─── Sidecar must be picked up by ray_col_mmap too (splay-mmap path) ── */

static test_result_t test_link_mmap_loads_sidecar(void) {
Expand Down Expand Up @@ -1210,6 +1251,7 @@ const test_entry_t link_entries[] = {
{ "link/deref_null_propagation", test_link_deref_null_propagation, link_setup, link_teardown },
{ "link/deref_oob_yields_null", test_link_deref_oob_yields_null, link_setup, link_teardown },
{ "link/persistence_roundtrip", test_link_persistence_roundtrip, link_setup, link_teardown },
{ "link/persistence_long_target_name", test_link_persistence_long_target_name, link_setup, link_teardown },
{ "link/coexists_with_index", test_link_coexists_with_index, link_setup, link_teardown },
{ "link/mmap_loads_sidecar", test_link_mmap_loads_sidecar, link_setup, link_teardown },
{ "link/deref_no_target_leak", test_link_deref_no_target_leak, link_setup, link_teardown },
Expand Down
Loading