Skip to content

Commit

Permalink
bugfix: ensured arguments of APIs mutating response headers do not co…
Browse files Browse the repository at this point in the history
…ntain '\r' and '\n' characters.

Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>
  • Loading branch information
doujiang24 authored and thibaultcha committed Jul 25, 2019
1 parent 246ec8a commit 881bf91
Show file tree
Hide file tree
Showing 7 changed files with 257 additions and 2 deletions.
12 changes: 12 additions & 0 deletions README.markdown
Expand Up @@ -4186,6 +4186,11 @@ to be returned when reading `ngx.header.Foo`.

Note that `ngx.header` is not a normal Lua table and as such, it is not possible to iterate through it using the Lua `ipairs` function.

Note: `HEADER` and `VALUE` will be truncated if they
contain the `\r` or `\n` characters. The truncated values
will contain all characters up to (and excluding) the first occurrence of
`\r` or `\n`.

For reading *request* headers, use the [ngx.req.get_headers](#ngxreqget_headers) function instead.

[Back to TOC](#nginx-api-for-lua)
Expand Down Expand Up @@ -5151,6 +5156,13 @@ ngx.redirect

Issue an `HTTP 301` or `302` redirection to `uri`.

Notice: the `uri` should not contains `\r` or `\n`, otherwise, the characters after `\r` or `\n` will be truncated, including the `\r` or `\n` bytes themself.

The `uri` argument will be truncated if it contains the
`\r` or `\n` characters. The truncated value will contain
all characters up to (and excluding) the first occurrence of `\r` or
`\n`.

The optional `status` parameter specifies the HTTP status code to be used. The following status codes are supported right now:

* `301`
Expand Down
12 changes: 12 additions & 0 deletions doc/HttpLuaModule.wiki
Expand Up @@ -3468,6 +3468,11 @@ to be returned when reading <code>ngx.header.Foo</code>.
Note that <code>ngx.header</code> is not a normal Lua table and as such, it is not possible to iterate through it using the Lua <code>ipairs</code> function.
Note: <code>HEADER</code> and <code>VALUE</code> will be truncated if they
contain the <code>\r</code> or <code>\n</code> characters. The truncated values
will contain all characters up to (and excluding) the first occurrence of
<code>\r</code> or <code>\n</code>.
For reading ''request'' headers, use the [[#ngx.req.get_headers|ngx.req.get_headers]] function instead.
== ngx.resp.get_headers ==
Expand Down Expand Up @@ -4302,6 +4307,13 @@ It is recommended that a coding style that combines this method call with the <c
Issue an <code>HTTP 301</code> or <code>302</code> redirection to <code>uri</code>.
Notice: the <code>uri</code> should not contains <code>\r</code> or <code>\n</code>, otherwise, the characters after <code>\r</code> or <code>\n</code> will be truncated, including the <code>\r</code> or <code>\n</code> bytes themself.
The <code>uri</code> argument will be truncated if it contains the
<code>\r</code> or <code>\n</code> characters. The truncated value will contain
all characters up to (and excluding) the first occurrence of <code>\r</code> or
<code>\n</code>.
The optional <code>status</code> parameter specifies the HTTP status code to be used. The following status codes are supported right now:
* <code>301</code>
Expand Down
2 changes: 2 additions & 0 deletions src/ngx_http_lua_control.c
Expand Up @@ -248,6 +248,8 @@ ngx_http_lua_ngx_redirect(lua_State *L)
"the headers");
}

len = ngx_http_lua_safe_header_value_len(p, len);

uri = ngx_palloc(r->pool, len);
if (uri == NULL) {
return luaL_error(L, "no memory");
Expand Down
4 changes: 4 additions & 0 deletions src/ngx_http_lua_headers_out.c
Expand Up @@ -491,6 +491,10 @@ ngx_http_lua_set_output_header(ngx_http_request_t *r, ngx_http_lua_ctx_t *ctx,

dd("set header value: %.*s", (int) value.len, value.data);

key.len = ngx_http_lua_safe_header_value_len(key.data, key.len);

value.len = ngx_http_lua_safe_header_value_len(value.data, value.len);

hv.hash = ngx_hash_key_lc(key.data, key.len);
hv.key = key;

Expand Down
15 changes: 15 additions & 0 deletions src/ngx_http_lua_util.h
Expand Up @@ -260,6 +260,21 @@ void ngx_http_lua_set_sa_restart(ngx_log_t *log);
}


static ngx_inline size_t
ngx_http_lua_safe_header_value_len(u_char *str, size_t len)
{
size_t i;

for (i = 0; i < len; i++, str++) {
if (*str == '\r' || *str == '\n') {
return i;
}
}

return len;
}


static ngx_inline void
ngx_http_lua_init_ctx(ngx_http_request_t *r, ngx_http_lua_ctx_t *ctx)
{
Expand Down
138 changes: 137 additions & 1 deletion t/016-resp-header.t
Expand Up @@ -8,7 +8,7 @@ use Test::Nginx::Socket::Lua;

repeat_each(2);

plan tests => repeat_each() * (blocks() * 3 + 59);
plan tests => repeat_each() * (blocks() * 3 + 73);

#no_diff();
no_long_string();
Expand Down Expand Up @@ -1963,3 +1963,139 @@ foo
Content-Type: application/json
--- no_error_log
[error]



=== TEST 87: truncates value after '\r'
--- config
location = /t {
content_by_lua_block {
ngx.header.header = "value\rfoo:bar\nbar:foo"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header: value
foo:
bar:
--- no_error_log
[error]



=== TEST 88: truncates value after '\n'
--- config
location = /t {
content_by_lua_block {
ngx.header.header = "value\nfoo:bar\rbar:foo"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header: value
foo:
bar:
--- no_error_log
[error]



=== TEST 89: truncates key after '\r'
--- config
location = /t {
content_by_lua_block {
ngx.header["header: value\rfoo:bar\nbar:foo"] = "xx"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header: value: xx
foo:
bar:
--- no_error_log
[error]



=== TEST 90: truncates key after '\n'
--- config
location = /t {
content_by_lua_block {
ngx.header["header: value\nfoo:bar\rbar:foo"] = "xx"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header: value: xx
foo:
bar:
--- no_error_log
[error]



=== TEST 91: truncates key after '\r' as the first character
--- config
location = /t {
content_by_lua_block {
ngx.header["\rheader: value\rfoo:bar\nbar:foo"] = "xx"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header:
foo:
bar:
--- no_error_log
[error]



=== TEST 92: truncates key after '\n' as the first character
--- config
location = /t {
content_by_lua_block {
ngx.header["\nheader: value\nfoo:bar\rbar:foo"] = "xx"
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
header:
foo:
bar:
--- no_error_log
[error]



=== TEST 93: truncates multiple values if they contain '\r' or '\n'
--- config
location = /t {
content_by_lua_block {
ngx.header["foo"] = {
"foo\nxx:bar",
"bar\rxxx:foo",
}
ngx.say("foo")
}
}
--- request
GET /t
--- response_headers
foo: foo, bar
xx:
xxx:
--- no_error_log
[error]
76 changes: 75 additions & 1 deletion t/022-redirect.t
Expand Up @@ -9,7 +9,7 @@ use Test::Nginx::Socket::Lua;
repeat_each(2);
#repeat_each(1);

plan tests => repeat_each() * (blocks() * 3 + 2);
plan tests => repeat_each() * (blocks() * 3 + 8);

#no_diff();
#no_long_string();
Expand Down Expand Up @@ -320,3 +320,77 @@ GET /read
--- response_headers
Location: http://agentzh.org/foo?a=b&c=d
--- error_code: 308



=== TEST 18: truncates uri after '\r'
--- config
location = /t {
content_by_lua_block {
ngx.redirect("http://agentzh.org/foo\rfoo:bar\nbar:foo");
ngx.say("hi")
}
}
--- request
GET /t
--- response_headers
Location: http://agentzh.org/foo
foo:
bar:
--- response_body_like: 302 Found
--- error_code: 302



=== TEST 19: truncates uri after '\n'
--- config
location = /t {
content_by_lua_block {
ngx.redirect("http://agentzh.org/foo\nfoo:bar\rbar:foo");
ngx.say("hi")
}
}
--- request
GET /t
--- response_headers
Location: http://agentzh.org/foo
foo:
bar:
--- response_body_like: 302 Found
--- error_code: 302



=== TEST 20: truncates uri with '\n' as the first character
--- config
location = /t {
content_by_lua_block {
ngx.redirect("\nfoo:http://agentzh.org/foo");
ngx.say("hi")
}
}
--- request
GET /t
--- response_headers
Location:
foo:
--- response_body_like: 302 Found
--- error_code: 302



=== TEST 21: truncates uri with '\r' as the first character
--- config
location = /t {
content_by_lua_block {
ngx.redirect("\rfoo:http://agentzh.org/foo");
ngx.say("hi")
}
}
--- request
GET /t
--- response_headers
Location:
foo:
--- response_body_like: 302 Found
--- error_code: 302

0 comments on commit 881bf91

Please sign in to comment.