Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngx.req.raw_header will emit some pre-read body in some edge cases #1200

Closed
tokers opened this issue Nov 30, 2017 · 8 comments
Closed

ngx.req.raw_header will emit some pre-read body in some edge cases #1200

tokers opened this issue Nov 30, 2017 · 8 comments

Comments

@tokers
Copy link
Contributor

tokers commented Nov 30, 2017

Hi!

I found that ngx.req.raw_header() will emit some pre-read body, you can reproduce the problem by the following configuration:

server {
	listen       7106;
	server_name  _;

	location /foo {
		content_by_lua_block {
			ngx.req.read_body()
			local raw_header = ngx.req.raw_header()
			ngx.log(ngx.ERR, raw_header)
			return ngx.exit(200)
		}
	}
 
	location / {
		content_by_lua_block {
			local sock = ngx.socket.tcp()
			local ok, err = sock:connect("127.0.0.1", 7106)
			if not ok then
				ngx.say("connect failed ", err)
				return
			end
 
			ok, err = sock:send("GET /foo HTTP/1.1\nHost:localhost\nContent-Length:7\n\nhe\r\nllo")
			if not ok then
				ngx.say("send failed ", err)
				return
			end
 
			ok, err = sock:close()
			if not ok then
				ngx.say("close failed ", err)
				return
			end
 
			ngx.say("see the error.log")
		}
	}
}

And with curl:

curl -v http://127.0.0.1:7106/

After it, we can find this in the error.log:

2017/11/30 10:41:44 [error] 7710#7710: *4 [lua] content_by_lua(nginx.conf:75):4: GET /foo HTTP/1.1
Host:localhost
Content-Length:7^M
he

Because the nginx core's ngx_http_parse_header_line also thinks the request header lines are terminated when two "\n" are found. So i think the ngx_http_lua_ngx_req_raw_header should also be compatible with this situation?

My local machine:

uname -a
Linux Fedora26-64 4.13.13-200.fc26.x86_64 #1 SMP Wed Nov 15 15:46:36 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

My OpenResty Version:

./nginx/sbin/nginx -V
nginx version: openresty/1.13.6.1
built by gcc 7.2.1 20170915 (Red Hat 7.2.1-2) (GCC)
built with OpenSSL 1.0.2k 26 Jan 2017
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt='-O2 -DNGX_LUA_ABORT_AT_PANIC -I/usr/local/openresty/zlib/include -I/usr/local/openresty/pcre/include -I/usr/local/openresty/openssl/include
' --add-module=../ngx_devel_kit-0.3.0 --add-module=../echo-nginx-module-0.61 --add-module=../xss-nginx-module-0.05 --add-module=../ngx_coolkit-0.2rc3 --add-module=../set-misc-nginx-module-0.31 --add-module=../fo
rm-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.07 --add-module=../srcache-nginx-module-0.31 --add-module=../ngx_lua-0.10.11 --add-module=../ngx_lua_upstream-0.07 --add-module=../head
ers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.18 --add-module=../redis2-nginx-module-0.14 --add-module=../redis-nginx-module-0.3.7 --add-module=../ngx
_stream_lua-0.0.3 --with-ld-opt='-Wl,-rpath,/usr/local/openresty/luajit/lib -L/usr/local/openresty/zlib/lib -L/usr/local/openresty/pcre/lib -L/usr/local/openresty/openssl/lib -Wl,-rpath,/usr/local/openresty/zlib
/lib:/usr/local/openresty/pcre/lib:/usr/local/openresty/openssl/lib' --with-pcre-jit --with-stream --with-stream_ssl_module --with-http_v2_module --without-mail_pop3_module --without-mail_imap_module --without-m
ail_smtp_module --with-http_stub_status_module --with-http_realip_module --with-http_addition_module --with-http_auth_request_module --with-http_secure_link_module --with-http_random_index_module --with-http_gzi
p_static_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-threads --with-file-aio --with-dtrace-probes --with-stream --with-stre
am_ssl_module --with-http_ssl_module

@agentzh
Copy link
Member

agentzh commented Nov 30, 2017

@tokers That part is complicated and hacky. I don't really want to touch it myself. Pull requests welcome :)

@tokers
Copy link
Contributor Author

tokers commented Nov 30, 2017

@agentzh OK, i will try :)

@tokers
Copy link
Contributor Author

tokers commented Dec 4, 2017

@agentzh I found it is difficult to distinguish the pre-read body when the b->pos pointer was moved, because it seems that there is no reliable benchmark we can refer. But i found another problem when troubleshooting this problem.

When the delimiter is the single LF, the first part headers will be discarded (with large headers). Pull request is here: #1204

@agentzh
Copy link
Member

agentzh commented Dec 4, 2017

@tokers Thanks for the investigations! Will have a look.

@spacewander
Copy link
Member

@agentzh
Consider it resolved?

@agentzh
Copy link
Member

agentzh commented Feb 5, 2018

@spacewander I'd like to see what @tokers wants to say about this issue.

@tokers
Copy link
Contributor Author

tokers commented Feb 6, 2018

@spacewander @agentzh There are two problems:

When the delimiter is the single LF and the request has large headers, ngx.req.raw_header will discard the first part headers.

This part had been resolved by #1204

ngx.req.raw_header will emit some pre-read body

This is hard to fix due to the nginx core's design(such as positioning the pre-read body is difficult when the b->pos is moved).

Since this case is relatively rare, i think we can close this issue now :).

@agentzh
Copy link
Member

agentzh commented Feb 6, 2018

@tokers Thanks for the explanation! I'm closing this.

@agentzh agentzh closed this as completed Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants