From 5859037e073bcac0080f8c2392515993cbbd4eef Mon Sep 17 00:00:00 2001 From: Yuji Yamano Date: Tue, 18 Aug 2020 21:16:44 +0900 Subject: [PATCH 1/2] Add tests for #473. --- test/conf/nginx.conf | 20 ++++++++++++++++++++ test/t/ngx_mruby.rb | 13 +++++++++++++ 2 files changed, 33 insertions(+) diff --git a/test/conf/nginx.conf b/test/conf/nginx.conf index ea9ce044..89f89ab0 100644 --- a/test/conf/nginx.conf +++ b/test/conf/nginx.conf @@ -936,6 +936,26 @@ http { Nginx.rputs h["X-Foo-Bar"] '; } + + location /issue-473/in { + mruby_content_handler_code ' + h = Nginx::Headers_in.new + h["X-Foo"] = nil + h["X-Bar"] = nil + Nginx.rputs h.all.sort.map {|k,v| "#{k}"}.join(",") + '; + } + + location /issue-473/out { + mruby_content_handler_code ' + h = Nginx::Headers_out.new + h["X-Foo"] = "Foo" + h["X-Foo"] = nil + h["X-Bar"] = nil + h["X-Baz"] = "Baz" + Nginx.rputs h.all.sort.map {|k,v| "#{k}"}.join(",") + '; + } } server { diff --git a/test/t/ngx_mruby.rb b/test/t/ngx_mruby.rb index f9f58739..4f805b30 100644 --- a/test/t/ngx_mruby.rb +++ b/test/t/ngx_mruby.rb @@ -785,6 +785,19 @@ def _run res = HttpRequest.new.get base + '/issue-471', nil, {"X-Foo" => "foo", "X-Foo-Bar" => "bar"} t.assert_equal 'foobar', res["body"] end + + t.assert('ngx_mruby - BUG: header issue 473', 'location /issue-473/in') do + res = HttpRequest.new.get base + '/issue-473/in', nil, {'X-Foo' => 'foo'} + t.assert_equal 'Accept,Connection,Host', res['body'] + end + + t.assert('ngx_mruby - BUG: header issue 473', 'location /issue-473/out') do + res = HttpRequest.new.get base + '/issue-473/out', nil, {'XX-Foo' => 'foo'} + t.assert_equal 'Server,X-Baz,hoge', res['body'] + t.assert_equal nil, res['x-foo'] + t.assert_equal nil, res['x-bar'] + t.assert_equal 'Baz', res['x-baz'] + end end From 8f37b5f4b166a1fcc43bf9558419f99ffd0841b2 Mon Sep 17 00:00:00 2001 From: Yuji Yamano Date: Tue, 18 Aug 2020 22:02:26 +0900 Subject: [PATCH 2/2] Fix segmentation fault when set nil to Nginx::Headers_{in,out}. Fixed #473. --- src/http/ngx_http_mruby_request.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/http/ngx_http_mruby_request.c b/src/http/ngx_http_mruby_request.c index 6ee19719..ae7a5dbf 100644 --- a/src/http/ngx_http_mruby_request.c +++ b/src/http/ngx_http_mruby_request.c @@ -227,18 +227,28 @@ static ngx_int_t ngx_mrb_set_request_header(mrb_state *mrb, ngx_list_t *headers, ngx_http_request_t *r = ngx_mrb_get_request(); key_len = (size_t)RSTRING_LEN(mrb_key); - val_len = (size_t)RSTRING_LEN(mrb_val); - key = ngx_pnalloc(pool, key_len); if (key == NULL) { return NGX_ERROR; } + ngx_memcpy(key, (u_char *)RSTRING_PTR(mrb_key), key_len); + + /* TODO:optimize later(linear-search is slow) */ + if (update || mrb_nil_p(mrb_val)) { + while (!mrb_nil_p(ngx_mrb_get_request_header(mrb, headers, (char *)key, key_len))) { + ngx_mrb_del_request_header(mrb, headers, (char *)key, key_len); + } + } + if (mrb_nil_p(mrb_val)) { + /* If the value is nil, just remove the key from the headers. */ + return NGX_OK; + } + + val_len = (size_t)RSTRING_LEN(mrb_val); val = ngx_pnalloc(pool, val_len); if (val == NULL) { return NGX_ERROR; } - - ngx_memcpy(key, (u_char *)RSTRING_PTR(mrb_key), key_len); ngx_memcpy(val, (u_char *)RSTRING_PTR(mrb_val), val_len); switch (ngx_mruby_builtin_header_lookup_token(key, key_len)) { @@ -260,13 +270,6 @@ static ngx_int_t ngx_mrb_set_request_header(mrb_state *mrb, ngx_list_t *headers, break; } - /* TODO:optimize later(linear-search is slow) */ - if (update) { - while (!mrb_nil_p(ngx_mrb_get_request_header(mrb, headers, (char *)key, key_len))) { - ngx_mrb_del_request_header(mrb, headers, (char *)key, key_len); - } - } - new_header = ngx_list_push(headers); if (new_header == NULL) { return NGX_ERROR;