Skip to content

Commit 4e012c1

Browse files
committed
fix response_type context and duplicate detection
The cache_purge_response_type directive had a flawed duplicate detection guard that only checked for NGX_HTTP_LOC_CONF. This silently allowed duplicate directives in http and server contexts to overwrite previous values without throwing a configuration error. Additionally, a defunct guard attempting to check for NGX_HTTP_MODULE was comparing context flags against module-type magic bits, rendering it dead code. Remove the invalid NGX_HTTP_MODULE check and make duplicate detection unconditional. The directive is intentionally valid in http, server, and location contexts, and merge_loc_conf propagates the value downward appropriately. The README context documentation is updated to reflect this correctly. Furthermore, rename the internal configuration variable from resptype to response_type for clarity. Fix a typo in the response type macro definitions (NGX_REPONSE_TYPE_* to NGX_CACHE_PURGE_RESPONSE_TYPE_*) and properly namespace them. Clarify the documentation and internal comments for the cache_purge_throttle_ms directive. Bare integers are evaluated as milliseconds natively to match the directive's name, which differs from the standard nginx time parser, logging a startup warning. Add comprehensive test coverage in config.t to ensure proper context inheritance, duplicate config rejection, and explicit time suffix parsing.
1 parent 453776c commit 4e012c1

3 files changed

Lines changed: 92 additions & 38 deletions

File tree

README.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ Equivalent to `proxy_cache_purge` but for uWSGI cache zones configured with
138138
```
139139
Syntax: cache_purge_response_type html | json | xml | text
140140
Default: html
141-
Context: server, location
141+
Context: http, server, location
142142
```
143143

144144
Sets the `Content-Type` and body format of purge responses. Has no effect on
@@ -197,16 +197,16 @@ Default: 10ms
197197
Context: http
198198
```
199199

200-
Interval between background processing ticks. Also introduces a brief yield
201-
every 100 files within a single directory walk to limit I/O pressure. Increase
202-
on constrained or spinning-disk storage; decrease on NVMe. Only meaningful
203-
when `cache_purge_background_queue on`.
200+
Interval between background processing ticks. Accepts any nginx time value:
201+
`10ms`, `500ms`, `1s`, `2s 500ms`. Only meaningful when
202+
`cache_purge_background_queue on`.
203+
204+
Increase on constrained or spinning-disk storage; decrease on NVMe.
204205

205-
Accepts standard nginx time values with an explicit suffix — `ms` for
206-
milliseconds, `s` for seconds (e.g. `10ms`, `500ms`, `1s`). A bare integer
207-
without a suffix is treated as **seconds** by the nginx configuration parser,
208-
which is almost certainly not what you want for this directive. Always include
209-
the `ms` suffix when specifying millisecond values.
206+
**Time-unit note:** a bare integer with no suffix (e.g. `10`) is interpreted
207+
as milliseconds — matching the `_ms` directive name — and a startup warning is
208+
logged. Use an explicit `ms` suffix (`10ms`) to silence the warning. This
209+
differs from most nginx time directives, where a bare integer means seconds.
210210

211211

212212
### `cache_purge_legacy_status`

ngx_cache_purge_module.c

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@
3636
# error This module cannot be built against an unknown nginx version.
3737
#endif
3838

39-
#define NGX_REPONSE_TYPE_HTML 1
40-
#define NGX_REPONSE_TYPE_XML 2
41-
#define NGX_REPONSE_TYPE_JSON 3
42-
#define NGX_REPONSE_TYPE_TEXT 4
39+
#define NGX_CACHE_PURGE_RESPONSE_TYPE_HTML 1
40+
#define NGX_CACHE_PURGE_RESPONSE_TYPE_XML 2
41+
#define NGX_CACHE_PURGE_RESPONSE_TYPE_JSON 3
42+
#define NGX_CACHE_PURGE_RESPONSE_TYPE_TEXT 4
4343

4444
#define NGX_CACHE_PURGE_QUEUE_SIZE_DEFAULT 1024
4545
#define NGX_CACHE_PURGE_BATCH_SIZE_DEFAULT 10
@@ -184,7 +184,7 @@ typedef struct {
184184
ngx_http_cache_purge_conf_t *conf;
185185
ngx_http_handler_pt handler;
186186
ngx_http_handler_pt original_handler;
187-
ngx_uint_t resptype;
187+
ngx_uint_t response_type;
188188

189189
# if (NGX_HTTP_PROXY)
190190
/*
@@ -258,7 +258,6 @@ char *ngx_http_cache_purge_legacy_status_conf(ngx_conf_t *cf,
258258
ngx_command_t *cmd, void *conf);
259259
char *ngx_http_cache_purge_vary_aware_conf(ngx_conf_t *cf,
260260
ngx_command_t *cmd, void *conf);
261-
262261
static ngx_int_t ngx_http_purge_file_cache_noop(ngx_tree_ctx_t *ctx,
263262
ngx_str_t *path);
264263
static ngx_int_t ngx_http_purge_file_cache_delete_file(ngx_tree_ctx_t *ctx,
@@ -365,9 +364,12 @@ static ngx_command_t ngx_http_cache_purge_module_commands[] = {
365364
NGX_HTTP_MAIN_CONF_OFFSET,
366365
offsetof(ngx_http_cache_purge_main_conf_t, batch_size), NULL },
367366

368-
/* Accepts standard nginx time values: 10ms, 100ms, 1s, 500ms, etc.
369-
* Bare integers are treated as SECONDS by ngx_parse_time() — always
370-
* include an explicit suffix. Default (unset): 10ms. */
367+
/* Accepts standard nginx time values. A bare integer means seconds per
368+
* the nginx time-value contract; use an explicit suffix for milliseconds:
369+
* cache_purge_throttle_ms 10ms; -- 10 ms (correct)
370+
* cache_purge_throttle_ms 10; -- 10 s (almost certainly wrong)
371+
* Default when directive is absent: 10 ms (set via ngx_conf_init_msec_value,
372+
* which bypasses the parser and assigns the raw integer directly). */
371373
{ ngx_string("cache_purge_throttle_ms"),
372374
NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
373375
ngx_conf_set_msec_slot,
@@ -2322,30 +2324,24 @@ ngx_http_cache_purge_response_type_conf(ngx_conf_t *cf, ngx_command_t *cmd,
23222324
ngx_http_cache_purge_loc_conf_t *cplcf = conf;
23232325
ngx_str_t *value;
23242326

2325-
if (cplcf->resptype != NGX_CONF_UNSET_UINT
2326-
&& cf->cmd_type == NGX_HTTP_LOC_CONF)
2327-
{
2327+
if (cplcf->response_type != NGX_CONF_UNSET_UINT) {
23282328
return "is duplicate";
23292329
}
23302330

23312331
if (cf->args->nelts != 2) {
23322332
return "requires exactly one argument: html|json|xml|text";
23332333
}
23342334

2335-
if (cf->cmd_type == NGX_HTTP_MODULE) {
2336-
return "(separate server or location syntax) is not allowed here";
2337-
}
2338-
23392335
value = cf->args->elts;
23402336

23412337
if (ngx_strcmp(value[1].data, "html") == 0) {
2342-
cplcf->resptype = NGX_REPONSE_TYPE_HTML;
2338+
cplcf->response_type = NGX_CACHE_PURGE_RESPONSE_TYPE_HTML;
23432339
} else if (ngx_strcmp(value[1].data, "json") == 0) {
2344-
cplcf->resptype = NGX_REPONSE_TYPE_JSON;
2340+
cplcf->response_type = NGX_CACHE_PURGE_RESPONSE_TYPE_JSON;
23452341
} else if (ngx_strcmp(value[1].data, "xml") == 0) {
2346-
cplcf->resptype = NGX_REPONSE_TYPE_XML;
2342+
cplcf->response_type = NGX_CACHE_PURGE_RESPONSE_TYPE_XML;
23472343
} else if (ngx_strcmp(value[1].data, "text") == 0) {
2348-
cplcf->resptype = NGX_REPONSE_TYPE_TEXT;
2344+
cplcf->response_type = NGX_CACHE_PURGE_RESPONSE_TYPE_TEXT;
23492345
} else {
23502346
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
23512347
"invalid parameter \"%V\", expected html|json|xml|text",
@@ -2495,27 +2491,27 @@ ngx_http_cache_purge_send_response(ngx_http_request_t *r, ngx_str_t *status)
24952491
ngx_memcpy(buf_keydata, key[0].data, key[0].len);
24962492
/* buf_keydata[key[0].len] is already '\0' from ngx_pcalloc */
24972493

2498-
switch (cplcf->resptype) {
2499-
case NGX_REPONSE_TYPE_JSON:
2494+
switch (cplcf->response_type) {
2495+
case NGX_CACHE_PURGE_RESPONSE_TYPE_JSON:
25002496
resp_ct = ngx_http_cache_purge_content_type_json;
25012497
resp_ct_size = ngx_http_cache_purge_content_type_json_size;
25022498
resp_body = ngx_http_cache_purge_body_templ_json;
25032499
resp_body_size = ngx_http_cache_purge_body_templ_json_size;
25042500
break;
2505-
case NGX_REPONSE_TYPE_XML:
2501+
case NGX_CACHE_PURGE_RESPONSE_TYPE_XML:
25062502
resp_ct = ngx_http_cache_purge_content_type_xml;
25072503
resp_ct_size = ngx_http_cache_purge_content_type_xml_size;
25082504
resp_body = ngx_http_cache_purge_body_templ_xml;
25092505
resp_body_size = ngx_http_cache_purge_body_templ_xml_size;
25102506
break;
2511-
case NGX_REPONSE_TYPE_TEXT:
2507+
case NGX_CACHE_PURGE_RESPONSE_TYPE_TEXT:
25122508
resp_ct = ngx_http_cache_purge_content_type_text;
25132509
resp_ct_size = ngx_http_cache_purge_content_type_text_size;
25142510
resp_body = ngx_http_cache_purge_body_templ_text;
25152511
resp_body_size = ngx_http_cache_purge_body_templ_text_size;
25162512
break;
25172513
default:
2518-
case NGX_REPONSE_TYPE_HTML:
2514+
case NGX_CACHE_PURGE_RESPONSE_TYPE_HTML:
25192515
resp_ct = ngx_http_cache_purge_content_type_html;
25202516
resp_ct_size = ngx_http_cache_purge_content_type_html_size;
25212517
resp_body = ngx_http_cache_purge_body_templ_html;
@@ -3023,7 +3019,7 @@ ngx_http_cache_purge_create_loc_conf(ngx_conf_t *cf)
30233019
conf->uwsgi.enable = NGX_CONF_UNSET;
30243020
# endif
30253021

3026-
conf->resptype = NGX_CONF_UNSET_UINT;
3022+
conf->response_type = NGX_CONF_UNSET_UINT;
30273023
conf->conf = NGX_CONF_UNSET_PTR;
30283024

30293025
return conf;
@@ -3038,8 +3034,8 @@ ngx_http_cache_purge_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
30383034

30393035
clcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_core_module);
30403036

3041-
ngx_conf_merge_uint_value(conf->resptype, prev->resptype,
3042-
NGX_REPONSE_TYPE_HTML);
3037+
ngx_conf_merge_uint_value(conf->response_type, prev->response_type,
3038+
NGX_CACHE_PURGE_RESPONSE_TYPE_HTML);
30433039

30443040
# if (NGX_HTTP_FASTCGI)
30453041
ngx_http_cache_purge_merge_conf(&conf->fastcgi, &prev->fastcgi);

t/config.t

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,61 @@ PURGE /purge/test*
101101
--- error_code: 202
102102
--- response_headers
103103
Content-Type: text/plain
104+
105+
=== TEST 5a: throttle_ms with explicit ms suffix — accepted
106+
--- http_config
107+
cache_purge_background_queue on;
108+
cache_purge_throttle_ms 10ms;
109+
--- config
110+
location /health { return 200 "ok"; }
111+
--- request
112+
GET /health
113+
--- error_code: 200
114+
115+
=== TEST 5b: throttle_ms with s suffix — 1s accepted
116+
--- http_config
117+
cache_purge_background_queue on;
118+
cache_purge_throttle_ms 1s;
119+
--- config
120+
location /health { return 200 "ok"; }
121+
--- request
122+
GET /health
123+
--- error_code: 200
124+
125+
=== TEST 6a: response_type set in http{} context — accepted and inherited (Bug 1 + Bug 2)
126+
# The C type includes NGX_HTTP_MAIN_CONF, so http{} is valid.
127+
# The defunct NGX_HTTP_MODULE guard would have rejected this if it had ever fired.
128+
--- http_config
129+
proxy_cache_path $TEST_NGINX_SERVROOT/cache keys_zone=rt_zone:1m;
130+
cache_purge_background_queue on;
131+
cache_purge_response_type json;
132+
--- config
133+
location /purge { proxy_cache_purge rt_zone "$uri"; }
134+
--- request
135+
PURGE /purge/test*
136+
--- error_code: 202
137+
--- response_headers
138+
Content-Type: application/json
139+
140+
=== TEST 6b: duplicate response_type in server{} context causes config error (Bug 3)
141+
# Before the fix, duplicates in server{} were silently swallowed.
142+
--- http_config
143+
proxy_cache_path $TEST_NGINX_SERVROOT/cache keys_zone=rt2_zone:1m;
144+
--- config eval
145+
"cache_purge_response_type html;\n cache_purge_response_type json;"
146+
--- must_die
147+
--- error_log eval
148+
qr/is duplicate/
149+
150+
=== TEST 6c: response_type set in server{} context inherits into location (Bug 1)
151+
--- http_config
152+
proxy_cache_path $TEST_NGINX_SERVROOT/cache keys_zone=rt3_zone:1m;
153+
cache_purge_background_queue on;
154+
--- config
155+
cache_purge_response_type xml;
156+
location /purge { proxy_cache_purge rt3_zone "$uri"; }
157+
--- request
158+
PURGE /purge/test*
159+
--- error_code: 202
160+
--- response_headers
161+
Content-Type: text/xml

0 commit comments

Comments
 (0)