Skip to content

Commit 09484e8

Browse files
wangfakangthibaultcha
wangfakang
authored and
thibaultcha
committed
bugfix: ensured 'init_worker_by_lua*' does not mutate another NGINX module's main_conf.
Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>
1 parent b687530 commit 09484e8

File tree

5 files changed

+235
-1
lines changed

5 files changed

+235
-1
lines changed

‎src/ngx_http_lua_initworkerby.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ ngx_http_lua_init_worker(ngx_cycle_t *cycle)
8686
}
8787

8888
conf_ctx = (ngx_http_conf_ctx_t *) cycle->conf_ctx[ngx_http_module.index];
89-
http_ctx.main_conf = conf_ctx->main_conf;
9089

9190
top_clcf = conf_ctx->loc_conf[ngx_http_core_module.ctx_index];
9291
top_llcf = conf_ctx->loc_conf[ngx_http_lua_module.ctx_index];
@@ -197,6 +196,12 @@ ngx_http_lua_init_worker(ngx_cycle_t *cycle)
197196
return NGX_ERROR;
198197
}
199198

199+
http_ctx.main_conf = ngx_pcalloc(conf.pool,
200+
sizeof(void *) * ngx_http_max_module);
201+
if (http_ctx.main_conf == NULL) {
202+
return NGX_ERROR;
203+
}
204+
200205
#if defined(nginx_version) && nginx_version >= 1009011
201206
modules = cycle->modules;
202207
#else
@@ -210,6 +215,21 @@ ngx_http_lua_init_worker(ngx_cycle_t *cycle)
210215

211216
module = modules[i]->ctx;
212217

218+
if (module->create_main_conf) {
219+
cur = module->create_main_conf(&conf);
220+
if (cur == NULL) {
221+
return NGX_ERROR;
222+
}
223+
224+
if (ngx_modules[i]->index == ngx_http_lua_module.index) {
225+
ngx_memcpy(cur,
226+
conf_ctx->main_conf[ngx_http_lua_module.ctx_index],
227+
sizeof(ngx_http_lua_main_conf_t));
228+
}
229+
230+
http_ctx.main_conf[modules[i]->ctx_index] = cur;
231+
}
232+
213233
if (module->create_srv_conf) {
214234
cur = module->create_srv_conf(&conf);
215235
if (cur == NULL) {

‎t/124-init-worker.t

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,3 +955,25 @@ qr/lua close the global Lua VM ([0-9A-F]+)$/,
955955
--- no_error_log
956956
[error]
957957
start privileged agent process
958+
959+
960+
961+
=== TEST 25: ensure it does not mutate another module's main_conf (github issue #1553)
962+
https://github.com/openresty/lua-nginx-module/issues/1553
963+
--- http_config
964+
init_worker_by_lua_block {
965+
return
966+
}
967+
--- config
968+
location /t {
969+
content_by_lua_block {
970+
ngx.say("fake_var = ", ngx.var.fake_var)
971+
}
972+
}
973+
--- request
974+
GET /t
975+
--- response_body
976+
fake_var = 1
977+
--- no_error_log
978+
[error]
979+
[alert]

‎t/data/fake-merge-module/config

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
ngx_addon_name=ngx_http_fake_merge_module
2+
HTTP_MODULES="$HTTP_MODULES ngx_http_fake_merge_module"
3+
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/ngx_http_fake_merge_module.c"

‎t/data/fake-merge-module/ngx_http_fake_merge_module.c

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/*
2+
* This fake module was used to reproduce a bug in ngx_lua's init_worker_by_lua
3+
* implementation. The bug would cause this module's main_conf->a to be reset
4+
* to 0 due to init_worker_by_lua invoking merge_loc_conf with a brand new
5+
* loc_conf, whose a member would be 0.
6+
* The test case for this bug is in 124-init-worker.t.
7+
*/
8+
9+
10+
#include <ngx_config.h>
11+
#include <ngx_core.h>
12+
#include <ngx_http.h>
13+
#include <nginx.h>
14+
15+
16+
typedef struct {
17+
ngx_flag_t a;
18+
} ngx_http_fake_merge_main_conf_t;
19+
20+
21+
typedef struct {
22+
ngx_flag_t a;
23+
} ngx_http_fake_merge_loc_conf_t;
24+
25+
26+
static ngx_int_t ngx_http_fake_merge_var(ngx_http_request_t *r,
27+
ngx_http_variable_value_t *v, uintptr_t data);
28+
static ngx_int_t ngx_http_fake_merge_add_variables(ngx_conf_t *cf);
29+
static ngx_int_t ngx_http_fake_merge_init(ngx_conf_t *cf);
30+
static void *ngx_http_fake_merge_create_main_conf(ngx_conf_t *cf);
31+
static void *ngx_http_fake_merge_create_loc_conf(ngx_conf_t *cf);
32+
static char *ngx_http_fake_merge_merge_loc_conf(ngx_conf_t *cf, void *prev,
33+
void *conf);
34+
35+
36+
static ngx_http_module_t ngx_http_fake_merge_module_ctx = {
37+
ngx_http_fake_merge_init, /* preconfiguration */
38+
NULL, /* postconfiguration */
39+
40+
ngx_http_fake_merge_create_main_conf, /* create main configuration */
41+
NULL, /* init main configuration */
42+
43+
NULL, /* create server configuration */
44+
NULL, /* merge server configuration */
45+
46+
ngx_http_fake_merge_create_loc_conf, /* create location configuration */
47+
ngx_http_fake_merge_merge_loc_conf /* merge location configuration */
48+
};
49+
50+
51+
ngx_module_t ngx_http_fake_merge_module = {
52+
NGX_MODULE_V1,
53+
&ngx_http_fake_merge_module_ctx, /* module context */
54+
NULL, /* module directives */
55+
NGX_HTTP_MODULE, /* module type */
56+
NULL, /* init master */
57+
NULL, /* init module */
58+
NULL, /* init process */
59+
NULL, /* init thread */
60+
NULL, /* exit thread */
61+
NULL, /* exit process */
62+
NULL, /* exit master */
63+
NGX_MODULE_V1_PADDING
64+
};
65+
66+
67+
static ngx_http_variable_t ngx_http_fake_merge_variables[] = {
68+
69+
{ ngx_string("fake_var"), NULL,
70+
ngx_http_fake_merge_var, 0,
71+
NGX_HTTP_VAR_NOCACHEABLE, 0 },
72+
73+
{ ngx_null_string, NULL, NULL, 0, 0, 0 }
74+
};
75+
76+
77+
static ngx_int_t
78+
ngx_http_fake_merge_var(ngx_http_request_t *r, ngx_http_variable_value_t *v,
79+
uintptr_t data)
80+
{
81+
static char *str[] = {"0", "1"};
82+
ngx_http_fake_merge_main_conf_t *fmcf;
83+
84+
fmcf = ngx_http_get_module_main_conf(r, ngx_http_fake_merge_module);
85+
if (fmcf == NULL) {
86+
return NGX_ERROR;
87+
}
88+
89+
v->len = 1;
90+
v->data = (u_char *) str[fmcf->a];
91+
v->valid = 1;
92+
v->no_cacheable = 0;
93+
v->not_found = 0;
94+
95+
return NGX_OK;
96+
}
97+
98+
99+
static ngx_int_t
100+
ngx_http_fake_merge_add_variables(ngx_conf_t *cf)
101+
{
102+
ngx_http_variable_t *var, *v;
103+
104+
for (v = ngx_http_fake_merge_variables; v->name.len; v++) {
105+
var = ngx_http_add_variable(cf, &v->name, v->flags);
106+
if (var == NULL) {
107+
return NGX_ERROR;
108+
}
109+
110+
var->get_handler = v->get_handler;
111+
var->data = v->data;
112+
}
113+
114+
return NGX_OK;
115+
}
116+
117+
118+
static ngx_int_t
119+
ngx_http_fake_merge_init(ngx_conf_t *cf)
120+
{
121+
ngx_http_fake_merge_loc_conf_t *flcf;
122+
123+
flcf = ngx_http_conf_get_module_loc_conf(cf, ngx_http_fake_merge_module);
124+
if (flcf == NULL) {
125+
return NGX_ERROR;
126+
}
127+
128+
if (ngx_http_fake_merge_add_variables(cf) != NGX_OK) {
129+
return NGX_ERROR;
130+
}
131+
132+
flcf->a = 1;
133+
134+
return NGX_OK;
135+
}
136+
137+
138+
static void *
139+
ngx_http_fake_merge_create_main_conf(ngx_conf_t *cf)
140+
{
141+
ngx_http_fake_merge_main_conf_t *fmcf;
142+
143+
fmcf = ngx_pcalloc(cf->pool, sizeof(ngx_http_fake_merge_main_conf_t));
144+
if (fmcf == NULL) {
145+
return NULL;
146+
}
147+
148+
fmcf->a = NGX_CONF_UNSET;
149+
150+
return fmcf;
151+
}
152+
153+
154+
static void *
155+
ngx_http_fake_merge_create_loc_conf(ngx_conf_t *cf)
156+
{
157+
ngx_http_fake_merge_loc_conf_t *flcf;
158+
159+
flcf = ngx_pcalloc(cf->pool, sizeof(ngx_http_fake_merge_loc_conf_t));
160+
if (flcf == NULL) {
161+
return NULL;
162+
}
163+
164+
flcf->a = NGX_CONF_UNSET;
165+
166+
return flcf;
167+
}
168+
169+
170+
static char *
171+
ngx_http_fake_merge_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
172+
{
173+
ngx_http_fake_merge_loc_conf_t *conf = child;
174+
ngx_http_fake_merge_loc_conf_t *prev = parent;
175+
ngx_http_fake_merge_main_conf_t *fmcf;
176+
177+
ngx_conf_merge_value(conf->a, prev->a, 0);
178+
179+
fmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_fake_merge_module);
180+
if (fmcf == NULL) {
181+
return NGX_CONF_ERROR;
182+
}
183+
184+
fmcf->a = conf->a;
185+
186+
return NGX_CONF_OK;
187+
}

‎util/build.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ force=$2
2323
#--with-http_spdy_module \
2424

2525
add_fake_shm_module="--add-module=$root/t/data/fake-shm-module"
26+
add_fake_merge_module="--add-module=$root/t/data/fake-merge-module"
2627

2728
time ngx-build $force $version \
2829
--with-pcre-jit \
@@ -58,6 +59,7 @@ time ngx-build $force $version \
5859
--add-module=$root/../stream-lua-nginx-module \
5960
--add-module=$root/t/data/fake-module \
6061
$add_fake_shm_module \
62+
$add_fake_merge_module \
6163
--add-module=$root/t/data/fake-delayed-load-module \
6264
--with-http_gunzip_module \
6365
--with-http_dav_module \

0 commit comments

Comments
 (0)