Skip to content

bugfix: remove sslhandshake result assert for pre-handshake errors.#534

Merged
zhuizhuhaomeng merged 1 commit into
openresty:masterfrom
tzssangglass:fix-sslhandshake-result-assert
Jun 6, 2026
Merged

bugfix: remove sslhandshake result assert for pre-handshake errors.#534
zhuizhuhaomeng merged 1 commit into
openresty:masterfrom
tzssangglass:fix-sslhandshake-result-assert

Conversation

@tzssangglass
Copy link
Copy Markdown
Contributor

@tzssangglass tzssangglass commented Jun 5, 2026

When tcpsock:sslhandshake() fails before the actual SSL handshake starts, ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result() may return FFI_OK while still providing the OpenSSL error details.

One example is a client private key setup failure. In that path, rc is FFI_ERROR, but u->error_ret is not set, so ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result would return FFI_OK. The existing assert(res == FFI_ERROR) turns this expected error-reporting path into an assertion failure.

This patch removes the assertion so Lua can return the original SSL error to the caller.

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-resty-core project.

@tzssangglass
Copy link
Copy Markdown
Contributor Author

As a fix for #533

@tzssangglass
Copy link
Copy Markdown
Contributor Author

ping @zhuizhuhaomeng

@tzssangglass tzssangglass force-pushed the fix-sslhandshake-result-assert branch from e330467 to 9a07206 Compare June 5, 2026 16:32
When `tcpsock:sslhandshake()` fails before the actual SSL handshake starts,
`ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result()` may return `FFI_OK`
while still providing the OpenSSL error details.

One example is a client private key setup failure. In that path, `rc` is
`FFI_ERROR`, but `u->error_ret` is not set,
so `ngx_http_lua_ffi_socket_tcp_get_sslhandshake_result` would return `FFI_OK`.
The existing `assert(res == FFI_ERROR)` turns this expected
error-reporting path into an assertion failure.

This patch removes the assertion so Lua can return the original SSL error to
the caller.
@tzssangglass tzssangglass force-pushed the fix-sslhandshake-result-assert branch from 9a07206 to e3d5506 Compare June 5, 2026 16:35
@zhuizhuhaomeng zhuizhuhaomeng merged commit c63009c into openresty:master Jun 6, 2026
3 of 4 checks passed
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.

2 participants