From 0e74b25f626a33554feb7fb192b95ed99f807b00 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 19 Sep 2025 21:46:53 +0200 Subject: [PATCH 1/6] C API: Fix bounds checking in _byidx functions The docs weren't 100% clear about bounds checking, but suggested that errors would be caught. The bounds checks are cheap compared to the function calls they're in, so we have no reason to omit them. --- src/libexpr-c/nix_api_value.cc | 12 +++++++ src/libexpr-tests/nix_api_value.cc | 55 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 3339790f4..3442bf1a1 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -326,6 +326,10 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value, try { auto & v = check_value_in(value); assert(v.type() == nix::nList); + if (ix >= v.listSize()) { + nix_set_err_msg(context, NIX_ERR_KEY, "list index out of bounds"); + return nullptr; + } auto * p = v.listView()[ix]; nix_gc_incref(nullptr, p); if (p != nullptr) @@ -389,6 +393,10 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state try { auto & v = check_value_in(value); collapse_attrset_layer_chain_if_needed(v, state); + if (i >= v.attrs()->size()) { + nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds"); + return nullptr; + } const nix::Attr & a = (*v.attrs())[i]; *name = state->state.symbols[a.name].c_str(); nix_gc_incref(nullptr, a.value); @@ -405,6 +413,10 @@ const char * nix_get_attr_name_byidx(nix_c_context * context, nix_value * value, try { auto & v = check_value_in(value); collapse_attrset_layer_chain_if_needed(v, state); + if (i >= v.attrs()->size()) { + nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds (Nix C API contract violation)"); + return nullptr; + } const nix::Attr & a = (*v.attrs())[i]; return state->state.symbols[a.name].c_str(); } diff --git a/src/libexpr-tests/nix_api_value.cc b/src/libexpr-tests/nix_api_value.cc index af95224de..c74c3258f 100644 --- a/src/libexpr-tests/nix_api_value.cc +++ b/src/libexpr-tests/nix_api_value.cc @@ -162,6 +162,29 @@ TEST_F(nix_api_expr_test, nix_build_and_init_list) nix_gc_decref(ctx, intValue); } +TEST_F(nix_api_expr_test, nix_get_list_byidx_large_indices) +{ + // Create a small list to test extremely large out-of-bounds access + ListBuilder * builder = nix_make_list_builder(ctx, state, 2); + nix_value * intValue = nix_alloc_value(ctx, state); + nix_init_int(ctx, intValue, 42); + nix_list_builder_insert(ctx, builder, 0, intValue); + nix_list_builder_insert(ctx, builder, 1, intValue); + nix_make_list(ctx, builder, value); + nix_list_builder_free(builder); + + // Test extremely large indices that would definitely crash without bounds checking + ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, 1000000)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, UINT_MAX / 2)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, UINT_MAX / 2 + 1000000)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + + // Clean up + nix_gc_decref(ctx, intValue); +} + TEST_F(nix_api_expr_test, nix_build_and_init_attr_invalid) { ASSERT_EQ(nullptr, nix_get_attr_byname(ctx, nullptr, state, 0)); @@ -244,6 +267,38 @@ TEST_F(nix_api_expr_test, nix_build_and_init_attr) free(out_name); } +TEST_F(nix_api_expr_test, nix_get_attr_byidx_large_indices) +{ + // Create a small attribute set to test extremely large out-of-bounds access + const char ** out_name = (const char **) malloc(sizeof(char *)); + BindingsBuilder * builder = nix_make_bindings_builder(ctx, state, 2); + nix_value * intValue = nix_alloc_value(ctx, state); + nix_init_int(ctx, intValue, 42); + nix_bindings_builder_insert(ctx, builder, "test", intValue); + nix_make_attrs(ctx, value, builder); + nix_bindings_builder_free(builder); + + // Test extremely large indices that would definitely crash without bounds checking + ASSERT_EQ(nullptr, nix_get_attr_byidx(ctx, value, state, 1000000, out_name)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_attr_byidx(ctx, value, state, UINT_MAX / 2, out_name)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_attr_byidx(ctx, value, state, UINT_MAX / 2 + 1000000, out_name)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + + // Test nix_get_attr_name_byidx with large indices too + ASSERT_EQ(nullptr, nix_get_attr_name_byidx(ctx, value, state, 1000000)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_attr_name_byidx(ctx, value, state, UINT_MAX / 2)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + ASSERT_EQ(nullptr, nix_get_attr_name_byidx(ctx, value, state, UINT_MAX / 2 + 1000000)); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + + // Clean up + nix_gc_decref(ctx, intValue); + free(out_name); +} + TEST_F(nix_api_expr_test, nix_value_init) { // Setup From 7c553a30a9e3efcb917752b1d9019ab464aeccdc Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 19 Sep 2025 22:20:20 +0200 Subject: [PATCH 2/6] C API: Improve nix_get_attr_name_byidx() doc --- src/libexpr-c/nix_api_value.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index ddff494b7..2c4e35b66 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -302,7 +302,7 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state /** @brief Get an attribute name by index in the sorted bindings * - * Useful when you want the name but want to avoid evaluation. + * Returns the attribute name without forcing evaluation of the attribute's value. * * Owned by the nix EvalState * @param[out] context Optional, stores error information From 3d777eb37f42cb8b6cd88b6a7c6a846dcb8cbcff Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 19 Sep 2025 23:06:51 +0200 Subject: [PATCH 3/6] C API: Add lazy attribute value and list item accessors --- src/libexpr-c/nix_api_value.cc | 62 +++++++ src/libexpr-c/nix_api_value.h | 50 +++++- src/libexpr-tests/nix_api_value.cc | 272 +++++++++++++++++++++++++++++ 3 files changed, 382 insertions(+), 2 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 3442bf1a1..c58d4fe89 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -339,6 +339,26 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value, NIXC_CATCH_ERRS_NULL } +nix_value * +nix_get_list_byidx_lazy(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix) +{ + if (context) + context->last_err_code = NIX_OK; + try { + auto & v = check_value_in(value); + assert(v.type() == nix::nList); + if (ix >= v.listSize()) { + nix_set_err_msg(context, NIX_ERR_KEY, "list index out of bounds"); + return nullptr; + } + auto * p = v.listView()[ix]; + nix_gc_incref(nullptr, p); + // Note: intentionally NOT calling forceValue() to keep the element lazy + return as_nix_value_ptr(p); + } + NIXC_CATCH_ERRS_NULL +} + nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name) { if (context) @@ -359,6 +379,27 @@ nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value NIXC_CATCH_ERRS_NULL } +nix_value * +nix_get_attr_byname_lazy(nix_c_context * context, const nix_value * value, EvalState * state, const char * name) +{ + if (context) + context->last_err_code = NIX_OK; + try { + auto & v = check_value_in(value); + assert(v.type() == nix::nAttrs); + nix::Symbol s = state->state.symbols.create(name); + auto attr = v.attrs()->get(s); + if (attr) { + nix_gc_incref(nullptr, attr->value); + // Note: intentionally NOT calling forceValue() to keep the attribute lazy + return as_nix_value_ptr(attr->value); + } + nix_set_err_msg(context, NIX_ERR_KEY, "missing attribute"); + return nullptr; + } + NIXC_CATCH_ERRS_NULL +} + bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name) { if (context) @@ -406,6 +447,27 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state NIXC_CATCH_ERRS_NULL } +nix_value * nix_get_attr_byidx_lazy( + nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name) +{ + if (context) + context->last_err_code = NIX_OK; + try { + auto & v = check_value_in(value); + collapse_attrset_layer_chain_if_needed(v, state); + if (i >= v.attrs()->size()) { + nix_set_err_msg(context, NIX_ERR_KEY, "attribute index out of bounds (Nix C API contract violation)"); + return nullptr; + } + const nix::Attr & a = (*v.attrs())[i]; + *name = state->state.symbols[a.name].c_str(); + nix_gc_incref(nullptr, a.value); + // Note: intentionally NOT calling forceValue() to keep the attribute lazy + return as_nix_value_ptr(a.value); + } + NIXC_CATCH_ERRS_NULL +} + const char * nix_get_attr_name_byidx(nix_c_context * context, nix_value * value, EvalState * state, unsigned int i) { if (context) diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index 2c4e35b66..38fede62b 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -265,10 +265,25 @@ ExternalValue * nix_get_external(nix_c_context * context, nix_value * value); */ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix); -/** @brief Get an attr by name +/** @brief Get the ix'th element of a list without forcing evaluation of the element + * + * Returns the list element without forcing its evaluation, allowing access to lazy values. + * The list value itself must already be evaluated. * * Owned by the GC. Use nix_gc_decref when you're done with the pointer * @param[out] context Optional, stores error information + * @param[in] value Nix value to inspect (must be an evaluated list) + * @param[in] state nix evaluator state + * @param[in] ix list element to get + * @return value, NULL in case of errors + */ +nix_value * +nix_get_list_byidx_lazy(nix_c_context * context, const nix_value * value, EvalState * state, unsigned int ix); + +/** @brief Get an attr by name + * + * Use nix_gc_decref when you're done with the pointer + * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect * @param[in] state nix evaluator state * @param[in] name attribute name @@ -276,6 +291,21 @@ nix_value * nix_get_list_byidx(nix_c_context * context, const nix_value * value, */ nix_value * nix_get_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name); +/** @brief Get an attribute value by attribute name, without forcing evaluation of the attribute's value + * + * Returns the attribute value without forcing its evaluation, allowing access to lazy values. + * The attribute set value itself must already be evaluated. + * + * Use nix_gc_decref when you're done with the pointer + * @param[out] context Optional, stores error information + * @param[in] value Nix value to inspect (must be an evaluated attribute set) + * @param[in] state nix evaluator state + * @param[in] name attribute name + * @return value, NULL in case of errors + */ +nix_value * +nix_get_attr_byname_lazy(nix_c_context * context, const nix_value * value, EvalState * state, const char * name); + /** @brief Check if an attribute name exists on a value * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect @@ -289,7 +319,7 @@ bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalS * * Also gives you the name. * - * Owned by the GC. Use nix_gc_decref when you're done with the pointer + * Use nix_gc_decref when you're done with the pointer * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect * @param[in] state nix evaluator state @@ -300,6 +330,22 @@ bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalS nix_value * nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name); +/** @brief Get an attribute by index in the sorted bindings, without forcing evaluation of the attribute's value + * + * Also gives you the name. Returns the attribute value without forcing its evaluation, allowing access to lazy values. + * The attribute set value itself must already be evaluated. + * + * Use nix_gc_decref when you're done with the pointer + * @param[out] context Optional, stores error information + * @param[in] value Nix value to inspect (must be an evaluated attribute set) + * @param[in] state nix evaluator state + * @param[in] i attribute index + * @param[out] name will store a pointer to the attribute name + * @return value, NULL in case of errors + */ +nix_value * nix_get_attr_byidx_lazy( + nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name); + /** @brief Get an attribute name by index in the sorted bindings * * Returns the attribute name without forcing evaluation of the attribute's value. diff --git a/src/libexpr-tests/nix_api_value.cc b/src/libexpr-tests/nix_api_value.cc index c74c3258f..830637f3e 100644 --- a/src/libexpr-tests/nix_api_value.cc +++ b/src/libexpr-tests/nix_api_value.cc @@ -185,6 +185,91 @@ TEST_F(nix_api_expr_test, nix_get_list_byidx_large_indices) nix_gc_decref(ctx, intValue); } +TEST_F(nix_api_expr_test, nix_get_list_byidx_lazy) +{ + // Create a list with a throwing lazy element, an already-evaluated int, and a lazy function call + + // 1. Throwing lazy element - create a function application thunk that will throw when forced + nix_value * throwingFn = nix_alloc_value(ctx, state); + nix_value * throwingValue = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string( + ctx, + state, + R"( + _: throw "This should not be evaluated by the lazy accessor" + )", + "", + throwingFn); + assert_ctx_ok(); + + nix_init_apply(ctx, throwingValue, throwingFn, throwingFn); + assert_ctx_ok(); + + // 2. Already evaluated int (not lazy) + nix_value * intValue = nix_alloc_value(ctx, state); + nix_init_int(ctx, intValue, 42); + assert_ctx_ok(); + + // 3. Lazy function application that would compute increment 5 = 6 + nix_value * lazyApply = nix_alloc_value(ctx, state); + nix_value * incrementFn = nix_alloc_value(ctx, state); + nix_value * argFive = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string(ctx, state, "x: x + 1", "", incrementFn); + assert_ctx_ok(); + nix_init_int(ctx, argFive, 5); + + // Create a lazy application: (x: x + 1) 5 + nix_init_apply(ctx, lazyApply, incrementFn, argFive); + assert_ctx_ok(); + + ListBuilder * builder = nix_make_list_builder(ctx, state, 3); + nix_list_builder_insert(ctx, builder, 0, throwingValue); + nix_list_builder_insert(ctx, builder, 1, intValue); + nix_list_builder_insert(ctx, builder, 2, lazyApply); + nix_make_list(ctx, builder, value); + nix_list_builder_free(builder); + + // Test 1: Lazy accessor should return the throwing element without forcing evaluation + nix_value * lazyThrowingElement = nix_get_list_byidx_lazy(ctx, value, state, 0); + assert_ctx_ok(); + ASSERT_NE(nullptr, lazyThrowingElement); + + // Verify the element is still lazy by checking that forcing it throws + nix_value_force(ctx, state, lazyThrowingElement); + assert_ctx_err(); + ASSERT_THAT( + nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("This should not be evaluated by the lazy accessor")); + + // Test 2: Lazy accessor should return the already-evaluated int + nix_value * intElement = nix_get_list_byidx_lazy(ctx, value, state, 1); + assert_ctx_ok(); + ASSERT_NE(nullptr, intElement); + ASSERT_EQ(42, nix_get_int(ctx, intElement)); + + // Test 3: Lazy accessor should return the lazy function application without forcing + nix_value * lazyFunctionElement = nix_get_list_byidx_lazy(ctx, value, state, 2); + assert_ctx_ok(); + ASSERT_NE(nullptr, lazyFunctionElement); + + // Force the lazy function application - should compute 5 + 1 = 6 + nix_value_force(ctx, state, lazyFunctionElement); + assert_ctx_ok(); + ASSERT_EQ(6, nix_get_int(ctx, lazyFunctionElement)); + + // Clean up + nix_gc_decref(ctx, throwingFn); + nix_gc_decref(ctx, throwingValue); + nix_gc_decref(ctx, intValue); + nix_gc_decref(ctx, lazyApply); + nix_gc_decref(ctx, incrementFn); + nix_gc_decref(ctx, argFive); + nix_gc_decref(ctx, lazyThrowingElement); + nix_gc_decref(ctx, intElement); + nix_gc_decref(ctx, lazyFunctionElement); +} + TEST_F(nix_api_expr_test, nix_build_and_init_attr_invalid) { ASSERT_EQ(nullptr, nix_get_attr_byname(ctx, nullptr, state, 0)); @@ -299,6 +384,193 @@ TEST_F(nix_api_expr_test, nix_get_attr_byidx_large_indices) free(out_name); } +TEST_F(nix_api_expr_test, nix_get_attr_byname_lazy) +{ + // Create an attribute set with a throwing lazy attribute, an already-evaluated int, and a lazy function call + + // 1. Throwing lazy element - create a function application thunk that will throw when forced + nix_value * throwingFn = nix_alloc_value(ctx, state); + nix_value * throwingValue = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string( + ctx, + state, + R"( + _: throw "This should not be evaluated by the lazy accessor" + )", + "", + throwingFn); + assert_ctx_ok(); + + nix_init_apply(ctx, throwingValue, throwingFn, throwingFn); + assert_ctx_ok(); + + // 2. Already evaluated int (not lazy) + nix_value * intValue = nix_alloc_value(ctx, state); + nix_init_int(ctx, intValue, 42); + assert_ctx_ok(); + + // 3. Lazy function application that would compute increment 7 = 8 + nix_value * lazyApply = nix_alloc_value(ctx, state); + nix_value * incrementFn = nix_alloc_value(ctx, state); + nix_value * argSeven = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string(ctx, state, "x: x + 1", "", incrementFn); + assert_ctx_ok(); + nix_init_int(ctx, argSeven, 7); + + // Create a lazy application: (x: x + 1) 7 + nix_init_apply(ctx, lazyApply, incrementFn, argSeven); + assert_ctx_ok(); + + BindingsBuilder * builder = nix_make_bindings_builder(ctx, state, 3); + nix_bindings_builder_insert(ctx, builder, "throwing", throwingValue); + nix_bindings_builder_insert(ctx, builder, "normal", intValue); + nix_bindings_builder_insert(ctx, builder, "lazy", lazyApply); + nix_make_attrs(ctx, value, builder); + nix_bindings_builder_free(builder); + + // Test 1: Lazy accessor should return the throwing attribute without forcing evaluation + nix_value * lazyThrowingAttr = nix_get_attr_byname_lazy(ctx, value, state, "throwing"); + assert_ctx_ok(); + ASSERT_NE(nullptr, lazyThrowingAttr); + + // Verify the attribute is still lazy by checking that forcing it throws + nix_value_force(ctx, state, lazyThrowingAttr); + assert_ctx_err(); + ASSERT_THAT( + nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("This should not be evaluated by the lazy accessor")); + + // Test 2: Lazy accessor should return the already-evaluated int + nix_value * intAttr = nix_get_attr_byname_lazy(ctx, value, state, "normal"); + assert_ctx_ok(); + ASSERT_NE(nullptr, intAttr); + ASSERT_EQ(42, nix_get_int(ctx, intAttr)); + + // Test 3: Lazy accessor should return the lazy function application without forcing + nix_value * lazyFunctionAttr = nix_get_attr_byname_lazy(ctx, value, state, "lazy"); + assert_ctx_ok(); + ASSERT_NE(nullptr, lazyFunctionAttr); + + // Force the lazy function application - should compute 7 + 1 = 8 + nix_value_force(ctx, state, lazyFunctionAttr); + assert_ctx_ok(); + ASSERT_EQ(8, nix_get_int(ctx, lazyFunctionAttr)); + + // Test 4: Missing attribute should return NULL with NIX_ERR_KEY + nix_value * missingAttr = nix_get_attr_byname_lazy(ctx, value, state, "nonexistent"); + ASSERT_EQ(nullptr, missingAttr); + ASSERT_EQ(NIX_ERR_KEY, nix_err_code(ctx)); + + // Clean up + nix_gc_decref(ctx, throwingFn); + nix_gc_decref(ctx, throwingValue); + nix_gc_decref(ctx, intValue); + nix_gc_decref(ctx, lazyApply); + nix_gc_decref(ctx, incrementFn); + nix_gc_decref(ctx, argSeven); + nix_gc_decref(ctx, lazyThrowingAttr); + nix_gc_decref(ctx, intAttr); + nix_gc_decref(ctx, lazyFunctionAttr); +} + +TEST_F(nix_api_expr_test, nix_get_attr_byidx_lazy) +{ + // Create an attribute set with a throwing lazy attribute, an already-evaluated int, and a lazy function call + + // 1. Throwing lazy element - create a function application thunk that will throw when forced + nix_value * throwingFn = nix_alloc_value(ctx, state); + nix_value * throwingValue = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string( + ctx, + state, + R"( + _: throw "This should not be evaluated by the lazy accessor" + )", + "", + throwingFn); + assert_ctx_ok(); + + nix_init_apply(ctx, throwingValue, throwingFn, throwingFn); + assert_ctx_ok(); + + // 2. Already evaluated int (not lazy) + nix_value * intValue = nix_alloc_value(ctx, state); + nix_init_int(ctx, intValue, 99); + assert_ctx_ok(); + + // 3. Lazy function application that would compute increment 10 = 11 + nix_value * lazyApply = nix_alloc_value(ctx, state); + nix_value * incrementFn = nix_alloc_value(ctx, state); + nix_value * argTen = nix_alloc_value(ctx, state); + + nix_expr_eval_from_string(ctx, state, "x: x + 1", "", incrementFn); + assert_ctx_ok(); + nix_init_int(ctx, argTen, 10); + + // Create a lazy application: (x: x + 1) 10 + nix_init_apply(ctx, lazyApply, incrementFn, argTen); + assert_ctx_ok(); + + BindingsBuilder * builder = nix_make_bindings_builder(ctx, state, 3); + nix_bindings_builder_insert(ctx, builder, "a_throwing", throwingValue); + nix_bindings_builder_insert(ctx, builder, "b_normal", intValue); + nix_bindings_builder_insert(ctx, builder, "c_lazy", lazyApply); + nix_make_attrs(ctx, value, builder); + nix_bindings_builder_free(builder); + + // Proper usage: first get the size and gather all attributes into a map + unsigned int attrCount = nix_get_attrs_size(ctx, value); + assert_ctx_ok(); + ASSERT_EQ(3u, attrCount); + + // Gather all attributes into a map (proper contract usage) + std::map attrMap; + const char * name; + + for (unsigned int i = 0; i < attrCount; i++) { + nix_value * attr = nix_get_attr_byidx_lazy(ctx, value, state, i, &name); + assert_ctx_ok(); + ASSERT_NE(nullptr, attr); + attrMap[std::string(name)] = attr; + } + + // Now test the gathered attributes + ASSERT_EQ(3u, attrMap.size()); + ASSERT_TRUE(attrMap.count("a_throwing")); + ASSERT_TRUE(attrMap.count("b_normal")); + ASSERT_TRUE(attrMap.count("c_lazy")); + + // Test 1: Throwing attribute should be lazy + nix_value * throwingAttr = attrMap["a_throwing"]; + nix_value_force(ctx, state, throwingAttr); + assert_ctx_err(); + ASSERT_THAT( + nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("This should not be evaluated by the lazy accessor")); + + // Test 2: Normal attribute should be already evaluated + nix_value * normalAttr = attrMap["b_normal"]; + ASSERT_EQ(99, nix_get_int(ctx, normalAttr)); + + // Test 3: Lazy function should compute when forced + nix_value * lazyAttr = attrMap["c_lazy"]; + nix_value_force(ctx, state, lazyAttr); + assert_ctx_ok(); + ASSERT_EQ(11, nix_get_int(ctx, lazyAttr)); + + // Clean up + nix_gc_decref(ctx, throwingFn); + nix_gc_decref(ctx, throwingValue); + nix_gc_decref(ctx, intValue); + nix_gc_decref(ctx, lazyApply); + nix_gc_decref(ctx, incrementFn); + nix_gc_decref(ctx, argTen); + for (auto & pair : attrMap) { + nix_gc_decref(ctx, pair.second); + } +} + TEST_F(nix_api_expr_test, nix_value_init) { // Setup From d0b1caf53af6fb648b0c5b3d5d3dbac0a9a1b611 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 20 Sep 2025 00:13:50 +0200 Subject: [PATCH 4/6] C API: Document and verify NIX_ERR_KEY behavior --- src/libexpr-tests/nix_api_expr.cc | 49 +++++++++++++++++++++++++++++++ src/libutil-c/nix_api_util.h | 19 +++++++++--- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/libexpr-tests/nix_api_expr.cc b/src/libexpr-tests/nix_api_expr.cc index dce8c6cb9..de508b4e4 100644 --- a/src/libexpr-tests/nix_api_expr.cc +++ b/src/libexpr-tests/nix_api_expr.cc @@ -423,6 +423,55 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_return_thunk) ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badReturnThunk")); } +static void primop_with_nix_err_key( + void * user_data, nix_c_context * context, EvalState * state, nix_value ** args, nix_value * ret) +{ + nix_set_err_msg(context, NIX_ERR_KEY, "Test error from primop"); +} + +TEST_F(nix_api_expr_test, nix_expr_primop_nix_err_key_conversion) +{ + // Test that NIX_ERR_KEY from a custom primop gets converted to a generic EvalError + // + // RATIONALE: NIX_ERR_KEY must not be propagated from custom primops because it would + // create semantic confusion. NIX_ERR_KEY indicates missing keys/indices in C API functions + // (like nix_get_attr_byname, nix_get_list_byidx). If custom primops could return NIX_ERR_KEY, + // an evaluation error would be indistinguishable from an actual missing attribute. + // + // For example, if nix_get_attr_byname returned NIX_ERR_KEY when the attribute is present + // but the value evaluation fails, callers expecting NIX_ERR_KEY to mean "missing attribute" + // would incorrectly handle evaluation failures as missing attributes. In places where + // missing attributes are tolerated (like optional attributes), this would cause the + // program to continue after swallowing the error, leading to silent failures. + PrimOp * primop = nix_alloc_primop( + ctx, primop_with_nix_err_key, 1, "testErrorPrimop", nullptr, "a test primop that sets NIX_ERR_KEY", nullptr); + assert_ctx_ok(); + nix_value * primopValue = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_init_primop(ctx, primopValue, primop); + assert_ctx_ok(); + + nix_value * arg = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_init_int(ctx, arg, 42); + assert_ctx_ok(); + + nix_value * result = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_value_call(ctx, state, primopValue, arg, result); + + // Verify that NIX_ERR_KEY gets converted to NIX_ERR_NIX_ERROR (generic evaluation error) + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("Error from custom function")); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("Test error from primop")); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("testErrorPrimop")); + + // Clean up + nix_gc_decref(ctx, primopValue); + nix_gc_decref(ctx, arg); + nix_gc_decref(ctx, result); +} + TEST_F(nix_api_expr_test, nix_value_call_multi_no_args) { nix_value * n = nix_alloc_value(ctx, state); diff --git a/src/libutil-c/nix_api_util.h b/src/libutil-c/nix_api_util.h index 5f42641d4..eaa07c9de 100644 --- a/src/libutil-c/nix_api_util.h +++ b/src/libutil-c/nix_api_util.h @@ -53,7 +53,7 @@ extern "C" { * - NIX_OK: No error occurred (0) * - NIX_ERR_UNKNOWN: An unknown error occurred (-1) * - NIX_ERR_OVERFLOW: An overflow error occurred (-2) - * - NIX_ERR_KEY: A key error occurred (-3) + * - NIX_ERR_KEY: A key/index access error occurred in C API functions (-3) * - NIX_ERR_NIX_ERROR: A generic Nix error occurred (-4) */ enum nix_err { @@ -83,10 +83,21 @@ enum nix_err { NIX_ERR_OVERFLOW = -2, /** - * @brief A key error occurred. + * @brief A key/index access error occurred in C API functions. * - * This error code is returned when a key error occurred during the function - * execution. + * This error code is returned when accessing a key, index, or identifier that + * does not exist in C API functions. Common scenarios include: + * - Setting keys that don't exist (nix_setting_get, nix_setting_set) + * - List indices that are out of bounds (nix_get_list_byidx*) + * - Attribute names that don't exist (nix_get_attr_byname*) + * - Attribute indices that are out of bounds (nix_get_attr_byidx*, nix_get_attr_name_byidx) + * + * This error typically indicates incorrect usage or assumptions about data structure + * contents, rather than internal Nix evaluation errors. + * + * @note This error code should ONLY be returned by C API functions themselves, + * not by underlying Nix evaluation. For example, evaluating `{}.foo` in Nix + * will throw a normal error (NIX_ERR_NIX_ERROR), not NIX_ERR_KEY. */ NIX_ERR_KEY = -3, From c121c6564052381c8067ce5c31d5418e968f69e5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 20 Sep 2025 00:47:00 +0200 Subject: [PATCH 5/6] C API: Clarify valid use of bindings ordering --- src/libexpr-c/nix_api_value.h | 42 ++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index 38fede62b..835eaec6e 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -315,10 +315,20 @@ nix_get_attr_byname_lazy(nix_c_context * context, const nix_value * value, EvalS */ bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalState * state, const char * name); -/** @brief Get an attribute by index in the sorted bindings +/** @brief Get an attribute by index * * Also gives you the name. * + * Attributes are returned in an unspecified order which is NOT suitable for + * reproducible operations. In Nix's domain, reproducibility is paramount. The caller + * is responsible for sorting the attributes or storing them in an ordered map to + * ensure deterministic behavior in your application. + * + * @note When Nix does sort attributes, which it does for virtually all intermediate + * operations and outputs, it uses byte-wise lexicographic order (equivalent to + * lexicographic order by Unicode scalar value for valid UTF-8). We recommend + * applying this same ordering for consistency. + * * Use nix_gc_decref when you're done with the pointer * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect @@ -330,10 +340,22 @@ bool nix_has_attr_byname(nix_c_context * context, const nix_value * value, EvalS nix_value * nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name); -/** @brief Get an attribute by index in the sorted bindings, without forcing evaluation of the attribute's value +/** @brief Get an attribute by index, without forcing evaluation of the attribute's value * - * Also gives you the name. Returns the attribute value without forcing its evaluation, allowing access to lazy values. - * The attribute set value itself must already be evaluated. + * Also gives you the name. + * + * Returns the attribute value without forcing its evaluation, allowing access to lazy values. + * The attribute set value itself must already have been evaluated. + * + * Attributes are returned in an unspecified order which is NOT suitable for + * reproducible operations. In Nix's domain, reproducibility is paramount. The caller + * is responsible for sorting the attributes or storing them in an ordered map to + * ensure deterministic behavior in your application. + * + * @note When Nix does sort attributes, which it does for virtually all intermediate + * operations and outputs, it uses byte-wise lexicographic order (equivalent to + * lexicographic order by Unicode scalar value for valid UTF-8). We recommend + * applying this same ordering for consistency. * * Use nix_gc_decref when you're done with the pointer * @param[out] context Optional, stores error information @@ -346,10 +368,20 @@ nix_get_attr_byidx(nix_c_context * context, nix_value * value, EvalState * state nix_value * nix_get_attr_byidx_lazy( nix_c_context * context, nix_value * value, EvalState * state, unsigned int i, const char ** name); -/** @brief Get an attribute name by index in the sorted bindings +/** @brief Get an attribute name by index * * Returns the attribute name without forcing evaluation of the attribute's value. * + * Attributes are returned in an unspecified order which is NOT suitable for + * reproducible operations. In Nix's domain, reproducibility is paramount. The caller + * is responsible for sorting the attributes or storing them in an ordered map to + * ensure deterministic behavior in your application. + * + * @note When Nix does sort attributes, which it does for virtually all intermediate + * operations and outputs, it uses byte-wise lexicographic order (equivalent to + * lexicographic order by Unicode scalar value for valid UTF-8). We recommend + * applying this same ordering for consistency. + * * Owned by the nix EvalState * @param[out] context Optional, stores error information * @param[in] value Nix value to inspect From a08ae1d024ab32e014e847ae7f70a661e7e380a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 25 Sep 2025 08:45:27 +0200 Subject: [PATCH 6/6] doc: Add release notes for C API lazy accessors --- doc/manual/rl-next/c-api-lazy-accessors.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 doc/manual/rl-next/c-api-lazy-accessors.md diff --git a/doc/manual/rl-next/c-api-lazy-accessors.md b/doc/manual/rl-next/c-api-lazy-accessors.md new file mode 100644 index 000000000..bd0604f0d --- /dev/null +++ b/doc/manual/rl-next/c-api-lazy-accessors.md @@ -0,0 +1,16 @@ +--- +synopsis: "C API: Add lazy attribute and list item accessors" +prs: [14030] +--- + +The C API now includes lazy accessor functions for retrieving values from lists and attribute sets without forcing evaluation: + +- `nix_get_list_byidx_lazy()` - Get a list element without forcing its evaluation +- `nix_get_attr_byname_lazy()` - Get an attribute value by name without forcing evaluation +- `nix_get_attr_byidx_lazy()` - Get an attribute by index without forcing evaluation + +These functions are useful when forwarding unevaluated sub-values to other lists, attribute sets, or function calls. They allow more efficient handling of Nix values by deferring evaluation until actually needed. + +Additionally, bounds checking has been improved for all `_byidx` functions to properly validate indices before access, preventing potential out-of-bounds errors. + +The documentation for `NIX_ERR_KEY` error handling has also been clarified to specify when this error code is returned. \ No newline at end of file