diff --git a/README.md b/README.md index d6c888c..1b01b7e 100644 --- a/README.md +++ b/README.md @@ -57,8 +57,8 @@ Description This module provides array typed nginx variables to `nginx.conf`. -Under the hood, this module just "abuses" the nginx string values to hold binary pointers -to C data structures (NGINX core's `ngx_array_t` struct on the C land). +Under the hood, this module uses nginx string values to hold tagged references +to NGINX core's `ngx_array_t` structures. The array type gives `nginx.onf` wonderful capabilities of handling value lists. Nowadays, however, you are highly recommended to use the [ngx_lua](https://github.com/openresty/lua-nginx-module) module diff --git a/src/ngx_http_array_var_module.c b/src/ngx_http_array_var_module.c index 76c4df7..32e93ca 100644 --- a/src/ngx_http_array_var_module.c +++ b/src/ngx_http_array_var_module.c @@ -17,6 +17,12 @@ static ngx_str_t ngx_http_array_it_key = ngx_string("array_it"); +typedef struct { + ngx_module_t *tag; + ngx_array_t *array; +} ngx_http_array_var_ref_t; + + typedef struct { ngx_uint_t nargs; } ngx_http_array_split_data_t; @@ -128,6 +134,55 @@ ngx_module_t ngx_http_array_var_module = { }; +static ngx_array_t * +ngx_http_array_var_create(ngx_http_request_t *r, ngx_uint_t n, ngx_str_t *res) +{ + ngx_array_t *array; + ngx_http_array_var_ref_t *a_ref; + + if (n > NGX_MAX_SIZE_T_VALUE / sizeof(ngx_str_t)) { + return NULL; + } + + array = ngx_array_create(r->pool, n, sizeof(ngx_str_t)); + if (array == NULL) { + return NULL; + } + + a_ref = ngx_palloc(r->pool, sizeof(ngx_http_array_var_ref_t)); + if (a_ref == NULL) { + return NULL; + } + + a_ref->tag = &ngx_http_array_var_module; + a_ref->array = array; + res->data = (u_char *) a_ref; + res->len = sizeof(ngx_http_array_var_ref_t); + + return array; +} + + +static ngx_array_t * +ngx_http_array_var_get(ngx_http_variable_value_t *v) +{ + ngx_http_array_var_ref_t a_ref; + + if (v->len != sizeof(ngx_http_array_var_ref_t)) { + return NULL; + } + + /* NDK copies values into buffers that need not be aligned. */ + ngx_memcpy(&a_ref, v->data, sizeof(ngx_http_array_var_ref_t)); + + if (a_ref.tag != &ngx_http_array_var_module) { + return NULL; + } + + return a_ref.array; +} + + static char * ngx_http_array_split(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { @@ -432,7 +487,7 @@ ngx_http_array_var_split(ngx_http_request_t *r, ngx_str_t *res, max = ngx_atosz(v[2].data, v[2].len); if (max == NGX_ERROR) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "array_split: invalid max items: \"%V\"", + "array_split: invalid max items: \"%v\"", &v[2]); return NGX_ERROR; @@ -445,7 +500,7 @@ ngx_http_array_var_split(ngx_http_request_t *r, ngx_str_t *res, len = max; } - array = ngx_array_create(r->pool, len, sizeof(ngx_str_t)); + array = ngx_http_array_var_create(r, len, res); if (array == NULL) { return NGX_ERROR; } @@ -515,9 +570,6 @@ ngx_http_array_var_split(ngx_http_request_t *r, ngx_str_t *res, dd("split: array size: %d", (int) array->nelts); dd("split array ptr: %p", array); - res->data = (u_char *) array; - res->len = sizeof(ngx_array_t); - return NGX_OK; } @@ -540,16 +592,15 @@ ngx_http_array_var_map(ngx_http_request_t *r, ngx_str_t *res, return NGX_OK; } - if (v[0].len != sizeof(ngx_array_t)) { + array = ngx_http_array_var_get(&v[0]); + if (array == NULL) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "array_join: invalid array variable value in the 2nd " - "argument: \"%.*s\"", &v[0]); + "array_map: invalid array variable value in the 2nd " + "argument: \"%v\"", &v[0]); return NGX_ERROR; } - array = (ngx_array_t *) v[0].data; - value = array->elts; array_it = ngx_http_get_indexed_variable(r, conf->array_it_index); @@ -559,10 +610,11 @@ ngx_http_array_var_map(ngx_http_request_t *r, ngx_str_t *res, if (conf->in_place) { new_array = array; + res->data = v[0].data; + res->len = v[0].len; } else { - new_array = ngx_array_create(r->pool, array->nelts, - sizeof(ngx_str_t)); + new_array = ngx_http_array_var_create(r, array->nelts, res); if (new_array == NULL) { return NGX_ERROR; } @@ -600,9 +652,6 @@ ngx_http_array_var_map(ngx_http_request_t *r, ngx_str_t *res, array_it->not_found = 1; array_it->valid = 0; - res->data = (u_char *) new_array; - res->len = sizeof(ngx_array_t); - return NGX_OK; } @@ -632,24 +681,24 @@ ngx_http_array_var_map_op(ngx_http_request_t *r, return NGX_ERROR; } - if (v[1].len != sizeof(ngx_array_t)) { + array = ngx_http_array_var_get(&v[1]); + if (array == NULL) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "array_map_op: invalid array variable value in the 2nd " - "argument: \"%.*s\"", &v[0]); + "argument: \"%v\"", &v[1]); return NGX_ERROR; } - array = (ngx_array_t *) v[1].data; - value = array->elts; if (conf->in_place) { new_array = array; + res->data = v[1].data; + res->len = v[1].len; } else { - new_array = ngx_array_create(r->pool, array->nelts, - sizeof(ngx_str_t)); + new_array = ngx_http_array_var_create(r, array->nelts, res); if (new_array == NULL) { return NGX_ERROR; } @@ -677,9 +726,6 @@ ngx_http_array_var_map_op(ngx_http_request_t *r, } } - res->data = (u_char *) new_array; - res->len = sizeof(ngx_array_t); - return NGX_OK; } @@ -699,16 +745,15 @@ ngx_http_array_var_join(ngx_http_request_t *r, dd("sep %.*s", sep->len, sep->data); - if (v[1].len != sizeof(ngx_array_t)) { + array = ngx_http_array_var_get(&v[1]); + if (array == NULL) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "array_join: invalid array variable value in the " - "2nd argument: \"%V\"", &v[1]); + "2nd argument: \"%v\"", &v[1]); return NGX_ERROR; } - array = (ngx_array_t *) v[1].data; - dd("join array ptr %p", array); dd("array->nelts: %d", (int) array->nelts); diff --git a/t/invalid-array.t b/t/invalid-array.t new file mode 100644 index 0000000..91ab84f --- /dev/null +++ b/t/invalid-array.t @@ -0,0 +1,169 @@ +# vi:filetype= + +use lib 'lib'; +use Config (); +use Test::Nginx::Socket; + +repeat_each(1); + +plan tests => repeat_each() * 3 * blocks(); + +no_long_string(); + +run_tests(); + +__DATA__ + +=== TEST 1: array map reports the scalar value after a successful array operation +--- config + location /foo { + error_page 500 = @invalid_array; + array_split ',' 'a,b' to=$array; + set $scalar 'invalid'; + array_map '[$array_it]' $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- grep_error_log eval +qr/array_map: invalid array variable value in the 2nd argument: "[^"]*"/ +--- grep_error_log_out +array_map: invalid array variable value in the 2nd argument: "invalid" + + + +=== TEST 2: array map op reports the scalar value rather than the directive name +--- config + location /foo { + error_page 500 = @invalid_array; + array_split ',' 'a,b' to=$array; + set $scalar 'invalid'; + array_map_op set_quote_sql_str $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- grep_error_log eval +qr/array_map_op: invalid array variable value in the 2nd argument: "[^"]*"/ +--- grep_error_log_out +array_map_op: invalid array variable value in the 2nd argument: "invalid" + + + +=== TEST 3: array join reports the scalar value +--- config + location /foo { + error_page 500 = @invalid_array; + array_split ',' 'a,b' to=$array; + set $scalar 'invalid'; + array_join ',' $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- grep_error_log eval +qr/array_join: invalid array variable value in the 2nd argument: "[^"]*"/ +--- grep_error_log_out +array_join: invalid array variable value in the 2nd argument: "invalid" + + + +=== TEST 4: array join rejects a scalar the same size as an array reference +--- config eval +my $scalar = 'a' x (2 * $Config::Config{ptrsize}); +sprintf q{ + location /foo { + error_page 500 = @invalid_array; + set $scalar '%s'; + array_join ',' $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +}, $scalar; +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- error_log +array_join: invalid array variable value in the 2nd argument: + + + +=== TEST 5: array map op rejects an array reference with a null tag +--- config eval +my $scalar = '0' x (4 * $Config::Config{ptrsize}); +sprintf q{ + location /foo { + error_page 500 = @invalid_array; + array_split ',' 'a,b' to=$array; + set_decode_hex $scalar '%s'; + array_map_op set_quote_sql_str $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +}, $scalar; +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- error_log +array_map_op: invalid array variable value in the 2nd argument: + + + +=== TEST 6: array map rejects an invalid tag with a valid array pointer +--- config eval +my $digits = 2 * $Config::Config{ptrsize}; +sprintf q{ + location /foo { + error_page 500 = @invalid_array; + array_split ',' 'a,b' to=$array; + set_encode_hex $encoded $array; + if ($encoded ~ "^.{%d}(.{%d})$") { + set $bad_hex "%s$1"; + } + set_decode_hex $scalar $bad_hex; + array_map '[$array_it]' $scalar; + echo 'unexpected success'; + } + + location @invalid_array { + return 500 "invalid array\n"; + } +}, $digits, $digits, '0' x $digits; +--- request +GET /foo +--- error_code: 500 +--- response_body +invalid array +--- error_log +array_map: invalid array variable value in the 2nd argument: diff --git a/t/sanity.t b/t/sanity.t index 0db489a..ab68817 100644 --- a/t/sanity.t +++ b/t/sanity.t @@ -369,3 +369,125 @@ GET /foo?names=bob,marry,nomas --- response_body "bob"+"marry"+"nomas" + + +=== TEST 25: copied array reference +--- config + location /foo { + array_split ',' 'a,b,c' to=$items; + set $alias "$items"; + array_join ':' $alias to=$out; + echo $out; + } +--- request +GET /foo +--- response_body +a:b:c + + + +=== TEST 26: internal redirect preserves an array reference +--- config + location /foo { + array_split ',' 'a,b,c' to=$items; + rewrite ^ /finish last; + } + + location /finish { + array_join ':' $items to=$out; + echo $out; + } +--- request +GET /foo +--- response_body +a:b:c + + + +=== TEST 27: named location preserves an array reference +--- config + location /foo { + error_page 418 = @finish; + array_split ',' 'a,b,c' to=$items; + return 418; + } + + location @finish { + array_map '[$array_it]' $items; + array_join ':' $items to=$out; + echo $out; + } +--- request +GET /foo +--- response_body +[a]:[b]:[c] + + + +=== TEST 28: parent and subrequest arrays share a pool +--- config + location /foo { + array_split ',' 'a,b,c' to=$items; + array_join ':' $items to=$parent_out; + echo_location /child; + echo $parent_out; + } + + location /child { + array_split ',' 'd,e' to=$items; + rewrite ^ /child-finish last; + } + + location /child-finish { + array_join ':' $items to=$out; + echo $out; + } +--- request +GET /foo +--- response_body +d:e +a:b:c + + + +=== TEST 29: mapped arrays and the original remain valid +--- config + location /foo { + array_split ',' 'a,b,c' to=$items; + array_map '[$array_it]' $items to=$mapped; + array_map_op set_quote_sql_str $mapped to=$quoted; + array_join ':' $items to=$original; + array_join ':' $mapped to=$mapped_out; + array_join ':' $quoted to=$quoted_out; + echo "$original|$mapped_out|$quoted_out"; + } +--- request +GET /foo +--- response_body +a:b:c|[a]:[b]:[c]|'[a]':'[b]':'[c]' + + + +=== TEST 30: a copied array reference survives creation of many arrays +--- config eval +my $config = q! + location /foo { + array_split ',' 'first,second' to=$array; + set $alias "$array"; +!; + +for (1 .. 512) { + $config .= q! array_split ',' 'x,y' to=$scratch; +!; +} + +$config . q! + array_map '[$array_it]' $alias; + array_join ':' $array to=$out; + echo $out; + } +!; +--- request +GET /foo +--- response_body +[first]:[second] diff --git a/util/build.sh b/util/build.sh index c9b2873..aa24568 100755 --- a/util/build.sh +++ b/util/build.sh @@ -8,6 +8,9 @@ version=$1 force=$2 ngx-build $force $version \ + --with-ld-opt="-L$OPENSSL_LIB -L$PCRE_LIB -Wl,-rpath,$OPENSSL_LIB:$PCRE_LIB:$LIBDRIZZLE_LIB:/usr/local/lib" \ + --with-cc-opt="-DDEBUG_MALLOC -I $OPENSSL_INC -I $PCRE_INC" \ + --with-http_ssl_module \ --without-mail_pop3_module \ --without-mail_imap_module \ --without-mail_smtp_module \ @@ -18,11 +21,11 @@ ngx-build $force $version \ --without-http_autoindex_module \ --without-http_auth_basic_module \ --without-http_userid_module \ - --add-module=$root/../echo-nginx-module \ - --add-module=$root/../ndk-nginx-module \ - --add-module=$root/../set-misc-nginx-module \ - --add-module=$root $opts \ - --with-debug || exit 1 + --add-module=$root/../echo-nginx-module \ + --add-module=$root/../ndk-nginx-module \ + --add-module=$root/../set-misc-nginx-module \ + --add-module=$root $opts \ + --with-debug || exit 1 #--add-module=$home/work/ndk \ #--without-http_ssi_module # we cannot disable ssi because echo_location_async depends on it (i dunno why?!)