Skip to content

Commit 0541ea9

Browse files
committed
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.
1 parent 593800a commit 0541ea9

3 files changed

Lines changed: 141 additions & 17 deletions

File tree

cv/cv.c

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ static int cv_ref_uuid_is = LUA_NOREF;
242242
static int cv_ref_tuple_is = LUA_NOREF;
243243
static int cv_ref_ffi_typestr = LUA_NOREF;
244244
static int cv_ref_box_null = LUA_NOREF;
245+
static int cv_ref_deepcopy = LUA_NOREF;
245246

246247
/*
247248
* Helper: is cdata at idx an int64_t?
@@ -2701,26 +2702,55 @@ cv_check_map(lua_State *L, struct cv_ctx *ctx,
27012702
if (pp->node->optional) {
27022703
continue;
27032704
}
2704-
if (pp->node->default_ref !=
2705-
LUA_NOREF) {
2706-
if (!ctx->validate_only) {
2707-
/*
2708-
* Apply default. Simple
2709-
* values are fine as-is;
2710-
* tables need deepcopy
2711-
* (TODO: deepcopy).
2712-
* For now push as-is.
2713-
*/
2714-
lua_rawgeti(L,
2715-
LUA_REGISTRYINDEX,
2716-
pp->node->default_ref);
2705+
if (pp->node->default_ref !=
2706+
LUA_NOREF) {
2707+
if (!ctx->validate_only) {
2708+
/*
2709+
* Apply default via deepcopy
2710+
* so each check() call gets
2711+
* its own independent copy.
2712+
* Then run cv_check_node on
2713+
* the copy so nested defaults
2714+
* are applied too.
2715+
*/
2716+
lua_rawgeti(L,
2717+
LUA_REGISTRYINDEX,
2718+
cv_ref_deepcopy);
2719+
lua_rawgeti(L,
2720+
LUA_REGISTRYINDEX,
2721+
pp->node->default_ref);
2722+
luaT_call(L, 1, 1);
2723+
int dval_idx = lua_gettop(L);
2724+
if (ctx->depth <
2725+
CV_PATH_MAX_DEPTH) {
2726+
ctx->path[ctx->depth]
2727+
= pp->key;
2728+
ctx->depth++;
2729+
}
2730+
bool r = cv_check_node(L,
2731+
ctx, dval_idx,
2732+
pp->node);
2733+
if (ctx->depth > 0)
2734+
ctx->depth--;
2735+
if (r) {
2736+
/* write back: value at
2737+
* dval_idx may have been
2738+
* updated by transform */
2739+
lua_pushvalue(L, dval_idx);
27172740
cv_map_setfield(L,
27182741
data_idx, &pp->key);
27192742
}
2720-
/* validate_only: default
2721-
* present = ok */
2722-
continue;
2743+
lua_pop(L, 1); /* pop dval */
2744+
if (!r) {
2745+
ok = false;
2746+
if (ctx->fail_fast)
2747+
return false;
2748+
}
27232749
}
2750+
/* validate_only: default
2751+
* present = ok */
2752+
continue;
2753+
}
27242754
/* truly missing */
27252755
if (ctx->depth < CV_PATH_MAX_DEPTH) {
27262756
ctx->path[ctx->depth] = pp->key;
@@ -3077,13 +3107,17 @@ cv_schema_check(lua_State *L)
30773107
/*
30783108
* Top-level default: if data is nil and
30793109
* schema has a default, apply it before
3080-
* validation (for scalar top-level nodes).
3110+
* validation. Use deepcopy so each call
3111+
* gets an independent copy of the default.
30813112
*/
30823113
if (lua_isnil(L, 2) &&
30833114
(*pp)->default_ref != LUA_NOREF &&
30843115
!validate_only) {
3116+
lua_rawgeti(L, LUA_REGISTRYINDEX,
3117+
cv_ref_deepcopy);
30853118
lua_rawgeti(L, LUA_REGISTRYINDEX,
30863119
(*pp)->default_ref);
3120+
luaT_call(L, 1, 1);
30873121
lua_replace(L, 2);
30883122
}
30893123

@@ -3248,6 +3282,19 @@ cv__init(lua_State *L)
32483282
else
32493283
lua_pop(L, 1);
32503284

3285+
/* deepcopy function */
3286+
if (cv_ref_deepcopy != LUA_NOREF) {
3287+
luaL_unref(L, LUA_REGISTRYINDEX,
3288+
cv_ref_deepcopy);
3289+
cv_ref_deepcopy = LUA_NOREF;
3290+
}
3291+
lua_getfield(L, 1, "deepcopy");
3292+
if (lua_isfunction(L, -1))
3293+
cv_ref_deepcopy =
3294+
luaL_ref(L, LUA_REGISTRYINDEX);
3295+
else
3296+
lua_pop(L, 1);
3297+
32513298
return 0;
32523299
}
32533300

cv/init.lua

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ _cv._init({
4242
return tostring(ffi.typeof(v))
4343
end,
4444
box_null = box.NULL,
45+
deepcopy = table.deepcopy,
4546
})
4647

4748
-- -------------------------------------------------------
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#!/usr/bin/env tarantool
2+
-- Tests that default table values are deepcopied on each
3+
-- check() call, not shared by reference.
4+
5+
local t = require('luatest')
6+
local cv = require('cv')
7+
8+
local g = t.group('inplace_default_deepcopy')
9+
10+
-- Top-level default={} must be deepcopied: mutating the
11+
-- returned value must not affect the next check() call,
12+
-- and the original default variable must stay untouched.
13+
function g.test_toplevel_default_deepcopy()
14+
local top_default = {}
15+
local s = cv.compile({
16+
type = 'map',
17+
default = top_default,
18+
properties = {
19+
x = {type = 'number', default = 10},
20+
},
21+
})
22+
23+
local r1, e1 = s:check(nil)
24+
t.assert_equals(e1, {})
25+
t.assert_equals(r1.x, 10)
26+
27+
-- mutate the result in place
28+
r1.x = 999
29+
30+
-- original default variable must stay empty
31+
t.assert_equals(top_default, {})
32+
33+
-- second call must return a fresh copy
34+
local r2, e2 = s:check(nil)
35+
t.assert_equals(e2, {})
36+
t.assert_equals(r2.x, 10)
37+
end
38+
39+
-- Field-level default={} must be deepcopied: mutating the
40+
-- returned nested map must not affect the next check() call,
41+
-- and the original default variable must stay untouched.
42+
function g.test_field_default_deepcopy()
43+
local opts_default = {}
44+
local s = cv.compile({
45+
type = 'map',
46+
properties = {
47+
opts = {
48+
type = 'map',
49+
default = opts_default,
50+
properties = {
51+
retries = {
52+
type = 'number',
53+
default = 3,
54+
},
55+
},
56+
},
57+
},
58+
})
59+
60+
local r1, e1 = s:check({})
61+
t.assert_equals(e1, {})
62+
t.assert_equals(r1.opts.retries, 3)
63+
64+
-- mutate the result in place
65+
r1.opts.retries = 999
66+
67+
-- original default variable must stay empty
68+
t.assert_equals(opts_default, {})
69+
70+
-- second call must return a fresh copy
71+
local r2, e2 = s:check({})
72+
t.assert_equals(e2, {})
73+
t.assert_equals(r2.opts.retries, 3)
74+
end
75+
76+
-- vim: ts=4 sts=4 sw=4 et

0 commit comments

Comments
 (0)