feature: ngx.req.set_uri(): added the 'binary' optional boolean arg to allow arbitrary binary data in the unencoded URI.

Signed-off-by: Yichun Zhang (agentzh) <yichun@openresty.com>

Author: zhuizhuhaomeng

README.markdown

@@ -4431,7 +4431,7 @@ See also [ngx.req.get_method](#ngxreqget_method).
 ngx.req.set_uri
 ---------------
 
-**syntax:** *ngx.req.set_uri(uri, jump?)*
+**syntax:** *ngx.req.set_uri(uri, jump?, binary?)*
 
 **context:** *set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua**
 
@@ -4527,6 +4527,18 @@ or
  ngx.req.set_uri("/foo", true)
 ```
 
+Starting from `0.10.16` of this module, this function accepts an
+optional boolean `binary` argument to allow arbitrary binary URI
+data. By default, this `binary` argument is false and this function
+will throw out a Lua error such as the one below when the `uri`
+argument contains any control characters (ASCII Code 0 ~ 0x08, 0x0A ~ 0x1F and 0x7F).
+
+
+    [error] 23430#23430: *1 lua entry thread aborted: runtime error:
+    content_by_lua(nginx.conf:44):3: ngx.req.set_uri unsafe byte "0x00"
+    in "\x00foo" (maybe you want to set the 'binary' argument?)
+
+
 This interface was first introduced in the `v0.3.1rc14` release.
 
 [Back to TOC](#nginx-api-for-lua)

doc/HttpLuaModule.wiki

@@ -3685,7 +3685,7 @@ See also [[#ngx.req.get_method|ngx.req.get_method]].
 
 == ngx.req.set_uri ==
 
-'''syntax:''' ''ngx.req.set_uri(uri, jump?)''
+'''syntax:''' ''ngx.req.set_uri(uri, jump?, binary?)''
 
 '''context:''' ''set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua*''
 
@@ -3771,6 +3771,18 @@ or
     ngx.req.set_uri("/foo", true)
 </geshi>
 
+Starting from <code>0.10.16</code> of this module, this function accepts an
+optional boolean <code>binary</code> argument to allow arbitrary binary URI
+data. By default, this <code>binary</code> argument is false and this function
+will throw out a Lua error such as the one below when the <code>uri</code>
+argument contains any control characters (ASCII Code 0 ~ 0x08, 0x0A ~ 0x1F and 0x7F).
+
+<geshi lang="text">
+    [error] 23430#23430: *1 lua entry thread aborted: runtime error:
+    content_by_lua(nginx.conf:44):3: ngx.req.set_uri unsafe byte "0x00"
+    in "\x00foo" (maybe you want to set the 'binary' argument?)
+</geshi>
+
 This interface was first introduced in the <code>v0.3.1rc14</code> release.
 
 == ngx.req.set_uri_args ==

src/ngx_http_lua_control.c

@@ -186,9 +186,12 @@ ngx_http_lua_ngx_redirect(lua_State *L)
     int                          n;
     u_char                      *p;
     u_char                      *uri;
+    u_char                       byte;
     size_t                       len;
     ngx_table_elt_t             *h;
     ngx_http_request_t          *r;
+    size_t                       buf_len;
+    u_char                      *buf;
 
     n = lua_gettop(L);
 
@@ -239,8 +242,17 @@ ngx_http_lua_ngx_redirect(lua_State *L)
                           "the headers");
     }
 
-    if (ngx_http_lua_check_unsafe_string(r, p, len, "redirect uri") != NGX_OK) {
-        return luaL_error(L, "attempt to set unsafe redirect uri");
+    if (ngx_http_lua_check_unsafe_uri_bytes(r, p, len, &byte) != NGX_OK) {
+        buf_len = ngx_http_lua_escape_log(NULL, p, len) + 1;
+        buf = ngx_palloc(r->pool, buf_len);
+        if (buf == NULL) {
+            return NGX_ERROR;
+        }
+
+        ngx_http_lua_escape_log(buf, p, len);
+        buf[buf_len - 1] = '\0';
+        return luaL_error(L, "unsafe byte \"0x%02x\" in redirect uri \"%s\"",
+                          byte, buf);
     }
 
     uri = ngx_palloc(r->pool, len);

src/ngx_http_lua_uri.c

@@ -32,14 +32,18 @@ ngx_http_lua_ngx_req_set_uri(lua_State *L)
     ngx_http_request_t          *r;
     size_t                       len;
     u_char                      *p;
+    u_char                       byte;
     int                          n;
     int                          jump = 0;
+    int                          binary = 0;
     ngx_http_lua_ctx_t          *ctx;
+    size_t                       buf_len;
+    u_char                      *buf;
 
     n = lua_gettop(L);
 
-    if (n != 1 && n != 2) {
-        return luaL_error(L, "expecting 1 or 2 arguments but seen %d", n);
+    if (n < 1 || n > 3) {
+        return luaL_error(L, "expecting 1, 2 or 3 arguments but seen %d", n);
     }
 
     r = ngx_http_lua_get_req(L);
@@ -55,8 +59,29 @@ ngx_http_lua_ngx_req_set_uri(lua_State *L)
         return luaL_error(L, "attempt to use zero-length uri");
     }
 
-    if (n == 2) {
+    if (n >= 3) {
+        luaL_checktype(L, 3, LUA_TBOOLEAN);
+        binary = lua_toboolean(L, 3);
+    }
+
+    if (!binary
+        && ngx_http_lua_check_unsafe_uri_bytes(r, p, len, &byte) != NGX_OK)
+    {
+        buf_len = ngx_http_lua_escape_log(NULL, p, len) + 1;
+        buf = ngx_palloc(r->pool, buf_len);
+        if (buf == NULL) {
+            return NGX_ERROR;
+        }
+
+        ngx_http_lua_escape_log(buf, p, len);
+        buf[buf_len - 1] = '\0';
+
+        return luaL_error(L, "unsafe byte \"0x%02x\" in uri \"%s\" "
+                          "(maybe you want to set the 'binary' argument?)",
+                          byte, buf);
+    }
 
+    if (n >= 2) {
         luaL_checktype(L, 2, LUA_TBOOLEAN);
         jump = lua_toboolean(L, 2);
 

src/ngx_http_lua_util.h

@@ -496,17 +496,16 @@ ngx_inet_get_port(struct sockaddr *sa)
 
 
 static ngx_inline ngx_int_t
-ngx_http_lua_check_unsafe_string(ngx_http_request_t *r, u_char *str, size_t len,
-    const char *name)
+ngx_http_lua_check_unsafe_uri_bytes(ngx_http_request_t *r, u_char *str,
+    size_t len, u_char *byte)
 {
-    size_t           i, buf_len;
+    size_t           i;
     u_char           c;
-    u_char          *buf, *src = str;
 
-                     /* %00-%1F, %7F */
+                     /* %00-%08, %0A-%1F, %7F */
 
     static uint32_t  unsafe[] = {
-        0xffffffff, /* 1111 1111 1111 1111  1111 1111 1111 1111 */
+        0xfffffdff, /* 1111 1111 1111 1111  1111 1101 1111 1111 */
 
                     /* ?>=< ;:98 7654 3210  /.-, +*)( '&%$ #"!  */
         0x00000000, /* 0000 0000 0000 0000  0000 0000 0000 0000 */
@@ -526,20 +525,7 @@ ngx_http_lua_check_unsafe_string(ngx_http_request_t *r, u_char *str, size_t len,
     for (i = 0; i < len; i++, str++) {
         c = *str;
         if (unsafe[c >> 5] & (1 << (c & 0x1f))) {
-            buf_len = ngx_http_lua_escape_log(NULL, src, len);
-            buf = ngx_palloc(r->pool, buf_len);
-            if (buf == NULL) {
-                return NGX_ERROR;
-            }
-
-            ngx_http_lua_escape_log(buf, src, len);
-
-            ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
-                          "unsafe byte \"0x%uxd\" in %s \"%*s\"",
-                          (unsigned) c, name, buf_len, buf);
-
-            ngx_pfree(r->pool, buf);
-
+            *byte = c;
             return NGX_ERROR;
         }
     }

t/022-redirect.t

@@ -9,7 +9,7 @@ use Test::Nginx::Socket::Lua;
 repeat_each(2);
 #repeat_each(1);
 
-plan tests => repeat_each() * (blocks() * 3 + 14);
+plan tests => repeat_each() * (blocks() * 3 + 9);
 
 #no_diff();
 #no_long_string();
@@ -339,8 +339,7 @@ Location:
 foo:
 bar:
 --- error_log
-unsafe byte "0xd" in redirect uri "http://agentzh.org/foo\x0Dfoo:bar\x0Abar:foo"
-attempt to set unsafe redirect uri
+unsafe byte "0x0d" in redirect uri "http://agentzh.org/foo\x0Dfoo:bar\x0Abar:foo"
 
 
 
@@ -360,8 +359,7 @@ Location:
 foo:
 bar:
 --- error_log
-unsafe byte "0xa" in redirect uri "http://agentzh.org/foo\x0Afoo:bar\x0Dbar:foo"
-attempt to set unsafe redirect uri
+unsafe byte "0x0a" in redirect uri "http://agentzh.org/foo\x0Afoo:bar\x0Dbar:foo"
 
 
 
@@ -380,8 +378,7 @@ GET /t
 Location:
 foo:
 --- error_log
-unsafe byte "0xa" in redirect uri "\x0Afoo:http://agentzh.org/foo"
-attempt to set unsafe redirect uri
+unsafe byte "0x0a" in redirect uri "\x0Afoo:http://agentzh.org/foo"
 
 
 
@@ -400,8 +397,7 @@ GET /t
 Location:
 foo:
 --- error_log
-unsafe byte "0xd" in redirect uri "\x0Dfoo:http://agentzh.org/foo"
-attempt to set unsafe redirect uri
+unsafe byte "0x0d" in redirect uri "\x0Dfoo:http://agentzh.org/foo"
 
 
 
@@ -420,5 +416,4 @@ GET /t
 Location:
 foo:
 --- error_log
-unsafe byte "0xd" in redirect uri "\x0Dhttp\x5C://\x22agentzh.org\x22/foo"
-attempt to set unsafe redirect uri
+unsafe byte "0x0d" in redirect uri "\x0Dhttp\x5C://\x22agentzh.org\x22/foo"

t/030-uri-args.t

@@ -9,7 +9,9 @@ log_level('warn');
 repeat_each(2);
 #repeat_each(1);
 
-plan tests => repeat_each() * (blocks() * 2 + 21);
+
+plan tests => repeat_each() * (blocks() * 2 + 23);
+
 
 no_root_location();
 
@@ -1579,7 +1581,7 @@ args: foo=%2C%24%40%7C%60&bar=-_.!~*'()
     location /t {
         content_by_lua_block {
             local new_uri = '\0foo'
-            ngx.req.set_uri(new_uri)
+            ngx.req.set_uri(new_uri, false, true)
             ngx.say(ngx.var.uri)
         }
     }
@@ -1684,3 +1686,84 @@ bad argument #1 to 'set_uri_args' (string, number, or table expected, but got ni
 --- error_code: 500
 --- error_log
 bad argument #1 to 'set_uri_args' (string, number, or table expected, but got userdata)
+
+
+
+=== TEST 64: set_uri binary option with unsafe uri
+explict specify binary option to true
+--- config
+    location /t {
+        rewrite_by_lua_block {
+            local new_uri = "/foo\r\nbar"
+            ngx.req.set_uri(new_uri, false, true)
+        }
+
+        proxy_pass http://127.0.0.1:$TEST_NGINX_SERVER_PORT;
+    }
+
+    location /foo {
+        content_by_lua_block {
+            ngx.say("request_uri: ", ngx.var.request_uri)
+            ngx.say("uri: ", ngx.var.uri)
+        }
+    }
+--- request
+    GET /t
+--- response_body eval
+["request_uri: /foo%0D%0Abar\nuri: /foo\r\nbar\n", "request_uri: /foo%0D%0Abar\nuri: /foo\r\nbar\n"]
+--- no_error_log
+[error]
+
+
+
+=== TEST 65: set_uri binary option with unsafe uri
+explict specify binary option to false
+--- config
+    location /t {
+        rewrite_by_lua_block {
+            local new_uri = "/foo\r\nbar"
+            ngx.req.set_uri(new_uri, false, false)
+        }
+
+        proxy_pass http://127.0.0.1:$TEST_NGINX_SERVER_PORT;
+    }
+
+    location /foo {
+        content_by_lua_block {
+            ngx.say("request_uri: ", ngx.var.request_uri)
+            ngx.say("uri: ", ngx.var.uri)
+        }
+    }
+--- request
+    GET /t
+--- error_code: 500
+--- error_log eval
+qr{\[error\] \d+#\d+: \*\d+ lua entry thread aborted: runtime error: rewrite_by_lua\(nginx.conf:\d+\):\d+: unsafe byte "0x0d" in uri "/foo\\x0D\\x0Abar" \(maybe you want to set the 'binary' argument\?\)}
+
+
+
+=== TEST 66: set_uri binary option with safe uri
+explict specify binary option to false
+--- config
+    location /t {
+        rewrite_by_lua_block {
+            local new_uri = "/foo bar"
+            ngx.req.set_uri(new_uri, false, true)
+        }
+
+        proxy_pass http://127.0.0.1:$TEST_NGINX_SERVER_PORT;
+    }
+
+    location /foo {
+        content_by_lua_block {
+            ngx.say("request_uri: ", ngx.var.request_uri)
+            ngx.say("uri: ", ngx.var.uri)
+        }
+    }
+--- request
+    GET /t
+--- response_body
+request_uri: /foo%20bar
+uri: /foo bar
+--- no_error_log
+[error]

t/162-static-module-location.t

@@ -18,7 +18,7 @@ __DATA__
         rewrite_by_lua_block {
             ngx.req.read_body();
             local args, _ = ngx.req.get_post_args();
-            ngx.req.set_uri(args["url"], true);
+            ngx.req.set_uri(args["url"], true, true);
         }
     }
 --- request