-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feature: Add api to fetch raw nginx ssl pointer #1286
Conversation
349f8cc
to
45c2bed
Compare
Test seems to be failing for unrelated reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, we need some tests in the existing test suite to cover this change.
src/ngx_http_lua_ssl_certby.c
Outdated
@@ -1319,6 +1319,21 @@ ngx_http_lua_ffi_set_priv_key(ngx_http_request_t *r, | |||
} | |||
|
|||
|
|||
int | |||
ngx_http_lua_ffi_get_ssl_pointer(ngx_http_request_t *r, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are moving to the meta-lua-nginx-module:
https://github.com/openresty/meta-lua-nginx-module
So this change deserves supporting other subsystems as well (like the stream
subsystem) if we indeed want to have this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this get added when the stream subsystem gains ssl_certificate_by_lua_block
support?
src/ngx_http_lua_ssl_certby.c
Outdated
{ | ||
if (r->connection == NULL || r->connection->ssl == NULL) { | ||
*err = "bad request"; | ||
return NGX_ERROR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to just return the SSL pointer directly and return NULL in case of errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix.
45c2bed
to
916a243
Compare
e3ca94d
to
2abdfe2
Compare
2abdfe2
to
319e8e0
Compare
Test added. |
Is there anything blocking this PR? |
This pull request is now in conflict :( |
f924579
to
fef2581
Compare
@@ -1319,6 +1319,17 @@ ngx_http_lua_ffi_set_priv_key(ngx_http_request_t *r, | |||
} | |||
|
|||
|
|||
ngx_ssl_conn_t * | |||
ngx_http_lua_ffi_get_ssl_pointer(ngx_http_request_t *r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ngx_http_lua_ffi_get_req_ssl_ptr
is a better name.
merged |
This allows introspection of the
SSL*
object from lua code.I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.