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

New features for ngx_lua #286

Closed
wants to merge 29 commits into from
Closed

New features for ngx_lua #286

wants to merge 29 commits into from

Conversation

aviramc
Copy link

@aviramc aviramc commented Oct 2, 2013

I've added quite a few patches to ngx_lua, and recently I've merged it with the newest version.
Some of the changes relate to issues, and some to things that we've talked about.
Here's a summary of all the changes and the issues to which they relate:

  • Sockets extensions:
    • Added support for SSL sockets (includes the verification of the remote end's certificate). Related to issue SSL/TLS support for cosocket API? #178
    • Setting different timeouts for receive and send.
    • Now read and write can be done from different Lua threads. Relates to issue Can't read/write to the same cosocket from different threads #241.
    • Added fake_close so that we won't be able to read again from the client socket after it is closed.
    • receive API - added the option 'bsd_read', which enables to receive the maximum number of bytes given (like the BSD API, not LuaSocket API).
  • Made the request socket (client socket) readable also when the request body was not read. This is useful when implementing protocol that require simultaenous reads and writes to and from the client. Relates to issue Advance print function #242.
  • Added functions for killing threads:
    • ngx.thread.kill - kills a thread.
    • ngx.thread.kill_sleeping - for a thread that has a loop with sleep - kills the thread if it's sleeping, otherwise waits for it to end.
    • It seems that when a request is over, if there's a sleeping thread, it doesn't delete its timer. Added a fix for that. Related to issue Not removing sleep timer when deleting a thread? #248.
  • Added APIs to get and set the keepalive property of a request. This is useful for implementing protocols that use the raw request socket. The API is ngx.req.get_keepalive and ngx.req.set_keepalive.
  • Added the directive lua_correct_location_header. When off, this causes Nginx not to correct relative 'Location' headers in responses. Default is on (the current beavior).
  • Added the directive lua_enforce_content_type. When off, if no headers were set in the response, the 'Content-Type' header is not set. Default is on (the current beavior).
  • Subrequest API - the method option can also now be a string. If the string is an unknown method, the subrequest's method field is set to NGX_HTTP_UNKNOWN, and the method_name field is set to the given method name. The previous numeric method still works.
  • Streaming subrequest API - Added the ability to perform a 'streaming' subrequest, using ngx.location.capture_stream, meaning that the main request and the subrequest would run simultaneously. This is still preliminary, thus the feature is protected by a C macro. To enable it on compile time, the option '-DNGX_LUA_CAPTURE_DOWN_STREAMING' should be given to GCC.

I've tried to put each of the changes in a different branch in my fork of the repository, thus if you want to merge only specific features, you can merge a specific branch or cherry pick the relevant commits.

For more information about the streaming subrequest API, go to
https://groups.google.com/forum/?hl=en#!topic/openresty-en/2M6Z9BNtufQ

Aviram and others added 29 commits April 24, 2013 10:36
…header that determines whether or not to replace underscores with hyphens.

Previously, underscores were replaced unconditionally.
Currently each of the functions has another boolean argument. If it's false, underscores would not be touched. If it's true, they would.
The default value of the argument is true.
…ptions), in which the only possible option currently is replace_underscores.
 - Now receive and send operations can be done simultaneous from several threads.
 - A different timeout can be set for receive and send.
 - Added fake_close for the client socket (ngx.req.socket), so that a thread that receives on this socket can be notified that we don't want to read from it anymore.
…s to receive just like BSD's recv call, meaning the maximum number of bytes given.

This will only work only when a number is given as the first parameter for receive.
Note that this differs from the original LuaSocket API.
… the "Location" response header, Nginx won't try to make absolute in case it's relative.

Default is on.
…response headers weren't set, ngx_lua won't enforce setting the content type header.

Default is on, and this causes the default behavior - if the headers aren't set, then the 'Content-Type' header is set to the default content type.
 ngx.thread.kill - kills a thread.
 ngx.thread.kill_sleeping - either kills immediately a sleeping thread, or, if the thread is not sleeping, can wait for it to end. Useful for a thread that has a loop in which it sleeps.
…eter as a string.

If the method string isn't a known Nginx method, it is still passed (and Nginx considers it as unknown).
Conflicts:
	src/ngx_http_lua_common.h
	src/ngx_http_lua_module.c
Conflicts:
	src/ngx_http_lua_common.h
	src/ngx_http_lua_module.c
…ts when using ngx.thread.kill_sleeping.

Now removing the timer.
…request body before using it.

This makes it possible to implement 'streaming' protocols, in which both client and server can read and write at the same time (such as forward HTTPS proxy with the CONNECT method).
@bakins
Copy link
Member

bakins commented Oct 2, 2013

That's a lot of changes in a single pull request. Could you maybe separate and prioritize? Makes it easier to review the individual changes.

@agentzh
Copy link
Member

agentzh commented Oct 2, 2013

@aviramc Great work! Thank you for your massive contributions!

And yeah, just as @bakins has suggested, it'll be easier for us to review and merge if you can send separate pull requests based on your individual git branches :)

agentzh added a commit that referenced this pull request Oct 2, 2013
… for controlling whether to send out a default Content-Type response header (as defined by the "default_type" directive). default on. thanks aviramc for the patch in #286.
@agentzh
Copy link
Member

agentzh commented Oct 2, 2013

@aviramc For the changes in your "content-type" branch, I've renamed your "lua_enforce_content_type" directive to "lua_use_default_type". Also I've made some other tweaks in your code and added some test cases for it. See commit 789e36b and commit 3740962 for details.

@agentzh
Copy link
Member

agentzh commented Oct 2, 2013

@aviramc I've noticed that your "patches" branch cannot even build on my side with nginx 1.4.2:

/home/agentzh/git/lua-nginx-module/src/ngx_http_lua_subrequest.c: In function ‘ngx_http_lua_post_subrequest’:
/home/agentzh/git/lua-nginx-module/src/ngx_http_lua_subrequest.c:1445:20: error: ‘ngx_http_lua_ctx_t’ has no member named ‘async_capture’
/home/agentzh/git/lua-nginx-module/src/ngx_http_lua_subrequest.c:1449:19: error: ‘ngx_http_lua_ctx_t’ has no member named ‘current_subrequest’
/home/agentzh/git/lua-nginx-module/src/ngx_http_lua_subrequest.c:1450:19: error: ‘ngx_http_lua_ctx_t’ has no member named ‘current_subrequest_ctx’
/home/agentzh/git/lua-nginx-module/src/ngx_http_lua_subrequest.c:1510:16: error: ‘ngx_http_lua_ctx_t’ has no member named ‘async_capture’

Am I missing anything here?

@aviramc
Copy link
Author

aviramc commented Oct 3, 2013

@agentzh @bakins Thanks very much for the review!
I'll comment here for the stuff you've reviewed, and create new pull requests for the other features.

@aviramc
Copy link
Author

aviramc commented Oct 3, 2013

@agentzh

Regarding commits 789e36b and ebf9cc8 , it seems like a good patch. Thanks!

Regarding the build errors - it is a problem in the subrequest streaming feature. It is turned off by default, by it seems that I forgot to put safe guards in some places. It will be fixed, and I'll post another pull request for the feature.

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

Successfully merging this pull request may close these issues.

4 participants