From 28977360766a96d183c61075a680ea527f44a598 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Fri, 9 Oct 2026 18:54:41 -0700 Subject: [PATCH] Modules: treat expired shared dict entries as absent Expired entries are hidden by read APIs but can still be found by write operations. replace() can revive an expired entry, delete() can report that it existed, and incr() can use its old value instead of init. Without an explicit timeout, incr() can return a value that a subsequent get() cannot retrieve. Use a common write-side lookup to remove expired entries and handle them as missing in both engines. incr() then creates a new entry with init plus delta and the directive or explicit timeout. Also return false instead of undefined when QuickJS delete() is called for a missing key. --- nginx/ngx_js_shared_dict.c | 80 +++++++++++++++++++------------------- nginx/t/js_shared_dict.t | 62 +++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 44 deletions(-) diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c index 5c14d67c..355f4e51 100644 --- a/nginx/ngx_js_shared_dict.c +++ b/nginx/ngx_js_shared_dict.c @@ -1455,6 +1455,24 @@ ngx_js_dict_expire_link(ngx_js_dict_t *dict, ngx_js_dict_node_t *node, } +static ngx_js_dict_node_t * +ngx_js_dict_lookup_write(ngx_js_dict_t *dict, ngx_str_t *key, ngx_msec_t now) +{ + ngx_js_dict_node_t *node; + + node = ngx_js_dict_lookup(dict, key); + + if (node != NULL && dict->timeout && now >= node->expire.key) { + ngx_js_dict_expire_unlink(dict, node); + ngx_rbtree_delete(&dict->sh->rbtree, &node->sn.node); + ngx_js_dict_node_free(dict, node); + return NULL; + } + + return node; +} + + static ngx_int_t ngx_js_dict_set(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key, njs_value_t *value, ngx_msec_t timeout, unsigned flags) @@ -1468,7 +1486,7 @@ ngx_js_dict_set(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key, ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_js_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { if (flags & NGX_JS_DICT_FLAG_MUST_EXIST) { @@ -1482,10 +1500,8 @@ ngx_js_dict_set(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key, } else { if (flags & NGX_JS_DICT_FLAG_MUST_NOT_EXIST) { - if (!dict->timeout || now < node->expire.key) { - ngx_rwlock_unlock(&dict->sh->rwlock); - return NGX_DECLINED; - } + ngx_rwlock_unlock(&dict->sh->rwlock); + return NGX_DECLINED; } if (ngx_js_dict_update(vm, dict, node, value, timeout, now) != NGX_OK) { @@ -1668,36 +1684,28 @@ ngx_js_dict_delete(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key, ngx_int_t rc; ngx_msec_t now; ngx_time_t *tp; - ngx_rbtree_key_t expire; ngx_js_dict_node_t *node; + tp = ngx_timeofday(); + now = tp->sec * 1000 + tp->msec; + ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_js_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { ngx_rwlock_unlock(&dict->sh->rwlock); return NGX_DECLINED; } - expire = 0; - if (dict->timeout) { - expire = ngx_js_dict_expire_unlink(dict, node); + ngx_js_dict_expire_unlink(dict, node); } ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node); if (retval != NULL) { - tp = ngx_timeofday(); - now = tp->sec * 1000 + tp->msec; - - if (!dict->timeout || now < expire) { - rc = ngx_js_dict_copy_value_locked(vm, dict, node, retval); - - } else { - rc = NGX_DECLINED; - } + rc = ngx_js_dict_copy_value_locked(vm, dict, node, retval); } else { rc = NGX_OK; @@ -1730,7 +1738,7 @@ ngx_js_dict_incr(njs_vm_t *vm, ngx_js_dict_t *dict, ngx_str_t *key, ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_js_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { njs_value_number_set(init, njs_value_number(init) @@ -4141,36 +4149,28 @@ ngx_qjs_dict_delete(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key, JSValue ret; ngx_msec_t now; ngx_time_t *tp; - ngx_rbtree_key_t expire; ngx_js_dict_node_t *node; + tp = ngx_timeofday(); + now = tp->sec * 1000 + tp->msec; + ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_qjs_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { ngx_rwlock_unlock(&dict->sh->rwlock); - return JS_UNDEFINED; + return retval ? JS_UNDEFINED : JS_FALSE; } - expire = 0; - if (dict->timeout) { - expire = ngx_js_dict_expire_unlink(dict, node); + ngx_js_dict_expire_unlink(dict, node); } ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node); if (retval) { - tp = ngx_timeofday(); - now = tp->sec * 1000 + tp->msec; - - if (!dict->timeout || now < expire) { - ret = ngx_qjs_dict_copy_value_locked(cx, dict, node); - - } else { - ret = JS_UNDEFINED; - } + ret = ngx_qjs_dict_copy_value_locked(cx, dict, node); } else { ret = JS_TRUE; @@ -4242,7 +4242,7 @@ ngx_qjs_dict_incr(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key, ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_qjs_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { value = JS_NewFloat64(cx, init + delta); @@ -4291,7 +4291,7 @@ ngx_qjs_dict_set(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key, ngx_rwlock_wlock(&dict->sh->rwlock); - node = ngx_qjs_dict_lookup(dict, key); + node = ngx_js_dict_lookup_write(dict, key, now); if (node == NULL) { if (flags & NGX_JS_DICT_FLAG_MUST_EXIST) { @@ -4306,10 +4306,8 @@ ngx_qjs_dict_set(JSContext *cx, ngx_js_dict_t *dict, ngx_str_t *key, } else { if (flags & NGX_JS_DICT_FLAG_MUST_NOT_EXIST) { - if (!dict->timeout || now < node->expire.key) { - ngx_rwlock_unlock(&dict->sh->rwlock); - return JS_FALSE; - } + ngx_rwlock_unlock(&dict->sh->rwlock); + return JS_FALSE; } if (ngx_qjs_dict_update(cx, dict, node, value, timeout, now) diff --git a/nginx/t/js_shared_dict.t b/nginx/t/js_shared_dict.t index 414becf5..f759a8d5 100644 --- a/nginx/t/js_shared_dict.t +++ b/nginx/t/js_shared_dict.t @@ -384,7 +384,7 @@ EOF $t->try_run('no js_shared_dict_zone'); -$t->plan(64); +$t->plan(77); ############################################################################### @@ -432,10 +432,18 @@ like(http_get('/replace?dict=foo&key=FOO2&value=aaa'), qr/true/, like(http_get('/replace?dict=foo&key=NOT_EXISTING&value=aaa'), qr/false/, 'failed replace foo.NOT_EXISTING'); +http_get('/set?dict=foo&key=DELETE&value=xxx'); +http_get('/set?dict=foo&key=POP&value=xxx'); + select undef, undef, undef, 2.1; -like(http_get('/get?dict=foo&key=FOO'), qr/undefined/, 'get expired foo.FOO'); -like(http_get('/pop?dict=foo&key=FOO'), qr/undefined/, 'pop expired foo.FOO'); +like(http_get('/get?dict=foo&key=FOO2'), qr/undefined/, + 'get expired foo.FOO2'); +like(http_get('/pop?dict=foo&key=POP'), qr/undefined/, 'pop expired foo.POP'); +like(http_get('/has?dict=foo&key=FOO3'), qr/false/, + 'has expired foo.FOO3'); +like(http_get('/ttl?dict=foo&key=FOO3'), qr/undefined/, + 'ttl expired foo.FOO3'); TODO: { local $TODO = 'not yet' unless has_version('0.8.1'); @@ -444,6 +452,8 @@ like(http_get('/keys?dict=foo'), qr/empty/, 'foo keys after expire'); like(http_get('/keys?dict=bar'), qr/FOO\,FOO2/, 'bar keys after a delay'); like(http_get('/size?dict=foo'), qr/size: 0/, 'no of items in foo after expire'); +like(http_get('/items?dict=foo'), qr/\x0d\x0a\x0d\x0a$/, + 'foo items after expire'); like(http_get('/items?dict=bar'), qr/FOO,zzz|FOO2,aaa/, 'bar items'); like(http_get('/items?dict=waka'), qr/FOO,47|FOO2,7779|FOO3,3338/, @@ -451,6 +461,20 @@ like(http_get('/items?dict=waka'), qr/FOO,47|FOO2,7779|FOO3,3338/, } +TODO: { +local $TODO = 'not yet' unless has_version('1.0.3'); + +like(http_get('/replace?dict=foo&key=FOO2&value=aaa'), qr/false/, + 'replace expired foo.FOO2'); +like(http_get('/get?dict=foo&key=FOO2'), qr/undefined/, + 'failed replace does not revive foo.FOO2'); +like(http_get('/delete?dict=foo&key=DELETE'), qr/false/, + 'delete expired foo.DELETE'); +like(http_get('/delete?dict=foo&key=DELETE'), qr/false/, + 'delete missing foo.DELETE'); + +} + TODO: { local $TODO = 'not yet' unless has_version('0.8.5'); @@ -508,6 +532,38 @@ my $ttl_incr_def = get_ttl('/ttl?dict=waka&key=TTL_INCR_DEF'); ok($ttl_incr_def >= 900000 && $ttl_incr_def <= 1000000, 'incr without timeout uses directive default'); +http_get('/set?dict=waka&key=EXPIRED&value=100&timeout=100'); +select undef, undef, undef, 0.2; + +TODO: { +local $TODO = 'not yet' unless has_version('1.0.3'); + +like(http_get('/incr?dict=waka&key=EXPIRED&by=3&def=7'), qr/^10$/m, + 'incr expired entry uses init'); +like(http_get('/get?dict=waka&key=EXPIRED'), qr/^10$/m, + 'incr expired entry creates visible value'); +my $ttl_expired = get_ttl('/ttl?dict=waka&key=EXPIRED'); +ok($ttl_expired >= 900000 && $ttl_expired <= 1000000, + 'incr expired entry uses directive timeout'); + +} + +http_get('/set?dict=waka&key=EXPIRED_EXPLICIT&value=100&timeout=100'); +select undef, undef, undef, 0.2; + +TODO: { +local $TODO = 'not yet' unless has_version('1.0.3'); + +like(http_get('/incr?dict=waka&key=EXPIRED_EXPLICIT&by=3&def=7&timeout=30000'), + qr/^10$/m, 'incr expired entry with explicit timeout uses init'); +like(http_get('/get?dict=waka&key=EXPIRED_EXPLICIT'), qr/^10$/m, + 'incr expired entry with explicit timeout creates visible value'); +my $ttl_explicit = get_ttl('/ttl?dict=waka&key=EXPIRED_EXPLICIT'); +ok($ttl_explicit >= 25000 && $ttl_explicit <= 30000, + 'incr expired entry uses explicit timeout'); + +} + like(http_get('/pop?dict=bar&key=FOO'), qr/zzz/, 'pop bar.FOO'); like(http_get('/pop?dict=bar&key=FOO'), qr/undefined/, 'pop deleted bar.FOO'); -- 2.47.3