From 952f602101d26e9b7ba837600f548e524689dac2 Mon Sep 17 00:00:00 2001 From: Dmitry Oboukhov Date: Thu, 26 Mar 2026 17:04:03 +0300 Subject: [PATCH] fix(default): deepcopy table defaults on each check() call Default table values were stored as a single Lua registry reference and reused by reference on every check() call. Mutating the returned value would corrupt subsequent calls. Fix both application sites: - top-level default in cv_schema_check - field-level default in cv_check_map Field-level default now also runs cv_check_node on the copied value, so nested defaults are applied recursively. Add regression test in test/inplace_default_deepcopy_test.lua. --- cv/cv.c | 81 ++++++++++++++++++++------ cv/init.lua | 1 + test/inplace_default_deepcopy_test.lua | 76 ++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 17 deletions(-) create mode 100644 test/inplace_default_deepcopy_test.lua diff --git a/cv/cv.c b/cv/cv.c index 26fa8b7..49f278c 100644 --- a/cv/cv.c +++ b/cv/cv.c @@ -242,6 +242,7 @@ static int cv_ref_uuid_is = LUA_NOREF; static int cv_ref_tuple_is = LUA_NOREF; static int cv_ref_ffi_typestr = LUA_NOREF; static int cv_ref_box_null = LUA_NOREF; +static int cv_ref_deepcopy = LUA_NOREF; /* * Helper: is cdata at idx an int64_t? @@ -2683,26 +2684,55 @@ cv_check_map(lua_State *L, struct cv_ctx *ctx, if (pp->node->optional) { continue; } - if (pp->node->default_ref != - LUA_NOREF) { - if (!ctx->validate_only) { - /* - * Apply default. Simple - * values are fine as-is; - * tables need deepcopy - * (TODO: deepcopy). - * For now push as-is. - */ - lua_rawgeti(L, - LUA_REGISTRYINDEX, - pp->node->default_ref); + if (pp->node->default_ref != + LUA_NOREF) { + if (!ctx->validate_only) { + /* + * Apply default via deepcopy + * so each check() call gets + * its own independent copy. + * Then run cv_check_node on + * the copy so nested defaults + * are applied too. + */ + lua_rawgeti(L, + LUA_REGISTRYINDEX, + cv_ref_deepcopy); + lua_rawgeti(L, + LUA_REGISTRYINDEX, + pp->node->default_ref); + luaT_call(L, 1, 1); + int dval_idx = lua_gettop(L); + if (ctx->depth < + CV_PATH_MAX_DEPTH) { + ctx->path[ctx->depth] + = pp->key; + ctx->depth++; + } + bool r = cv_check_node(L, + ctx, dval_idx, + pp->node); + if (ctx->depth > 0) + ctx->depth--; + if (r) { + /* write back: value at + * dval_idx may have been + * updated by transform */ + lua_pushvalue(L, dval_idx); cv_map_setfield(L, data_idx, &pp->key); } - /* validate_only: default - * present = ok */ - continue; + lua_pop(L, 1); /* pop dval */ + if (!r) { + ok = false; + if (ctx->fail_fast) + return false; + } } + /* validate_only: default + * present = ok */ + continue; + } /* truly missing */ if (ctx->depth < CV_PATH_MAX_DEPTH) { ctx->path[ctx->depth] = pp->key; @@ -3059,13 +3089,17 @@ cv_schema_check(lua_State *L) /* * Top-level default: if data is nil and * schema has a default, apply it before - * validation (for scalar top-level nodes). + * validation. Use deepcopy so each call + * gets an independent copy of the default. */ if (lua_isnil(L, 2) && (*pp)->default_ref != LUA_NOREF && !validate_only) { + lua_rawgeti(L, LUA_REGISTRYINDEX, + cv_ref_deepcopy); lua_rawgeti(L, LUA_REGISTRYINDEX, (*pp)->default_ref); + luaT_call(L, 1, 1); lua_replace(L, 2); } @@ -3230,6 +3264,19 @@ cv__init(lua_State *L) else lua_pop(L, 1); + /* deepcopy function */ + if (cv_ref_deepcopy != LUA_NOREF) { + luaL_unref(L, LUA_REGISTRYINDEX, + cv_ref_deepcopy); + cv_ref_deepcopy = LUA_NOREF; + } + lua_getfield(L, 1, "deepcopy"); + if (lua_isfunction(L, -1)) + cv_ref_deepcopy = + luaL_ref(L, LUA_REGISTRYINDEX); + else + lua_pop(L, 1); + return 0; } diff --git a/cv/init.lua b/cv/init.lua index a15a2a7..6248d2b 100644 --- a/cv/init.lua +++ b/cv/init.lua @@ -42,6 +42,7 @@ _cv._init({ return tostring(ffi.typeof(v)) end, box_null = box.NULL, + deepcopy = table.deepcopy, }) -- ------------------------------------------------------- diff --git a/test/inplace_default_deepcopy_test.lua b/test/inplace_default_deepcopy_test.lua new file mode 100644 index 0000000..a03a6d8 --- /dev/null +++ b/test/inplace_default_deepcopy_test.lua @@ -0,0 +1,76 @@ +#!/usr/bin/env tarantool +-- Tests that default table values are deepcopied on each +-- check() call, not shared by reference. + +local t = require('luatest') +local cv = require('cv') + +local g = t.group('inplace_default_deepcopy') + +-- Top-level default={} must be deepcopied: mutating the +-- returned value must not affect the next check() call, +-- and the original default variable must stay untouched. +function g.test_toplevel_default_deepcopy() + local top_default = {} + local s = cv.compile({ + type = 'map', + default = top_default, + properties = { + x = {type = 'number', default = 10}, + }, + }) + + local r1, e1 = s:check(nil) + t.assert_equals(e1, {}) + t.assert_equals(r1.x, 10) + + -- mutate the result in place + r1.x = 999 + + -- original default variable must stay empty + t.assert_equals(top_default, {}) + + -- second call must return a fresh copy + local r2, e2 = s:check(nil) + t.assert_equals(e2, {}) + t.assert_equals(r2.x, 10) +end + +-- Field-level default={} must be deepcopied: mutating the +-- returned nested map must not affect the next check() call, +-- and the original default variable must stay untouched. +function g.test_field_default_deepcopy() + local opts_default = {} + local s = cv.compile({ + type = 'map', + properties = { + opts = { + type = 'map', + default = opts_default, + properties = { + retries = { + type = 'number', + default = 3, + }, + }, + }, + }, + }) + + local r1, e1 = s:check({}) + t.assert_equals(e1, {}) + t.assert_equals(r1.opts.retries, 3) + + -- mutate the result in place + r1.opts.retries = 999 + + -- original default variable must stay empty + t.assert_equals(opts_default, {}) + + -- second call must return a fresh copy + local r2, e2 = s:check({}) + t.assert_equals(e2, {}) + t.assert_equals(r2.opts.retries, 3) +end + +-- vim: ts=4 sts=4 sw=4 et