diff --git a/src/store/col.c b/src/store/col.c index b6af7d00..2d1d04d1 100644 --- a/src/store/col.c +++ b/src/store/col.c @@ -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; diff --git a/test/test_link.c b/test/test_link.c index de64821f..96552846 100644 --- a/test/test_link.c +++ b/test/test_link.c @@ -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) { @@ -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 },