Skip to content

feature: use the actual ngx_connection_t * in ssl_client_hello_by#2416

Closed
climagabriel wants to merge 2 commits intoopenresty:masterfrom
climagabriel:feature_connection_pointer_in_ssl_client_hello_by
Closed

feature: use the actual ngx_connection_t * in ssl_client_hello_by#2416
climagabriel wants to merge 2 commits intoopenresty:masterfrom
climagabriel:feature_connection_pointer_in_ssl_client_hello_by

Conversation

@climagabriel
Copy link
Contributor

@climagabriel climagabriel commented May 8, 2025

Do we really need to create a fake connection here? Sure, the request struct wasn't even created yet but the conneciton does exist.
I personally need it because I need to allocate memory that will live for as long as the connection exists, not until the Lua chunk returns.

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

image

Also, surely there must be a better way to do this.
c = ngx_ssl_get_connection(ssl_conn);
This line already does exist at the very top of ngx_http_lua_ssl_client_hello_handler().

If the real connection exists, can't we make it available in the Lua code itself?
The same way we make the fake request available by resty.core.base.get_request

Do we really need to create a fake connection here?
Sure, the request struct wasn't even created yet but the conneciton does
exist.
I personally need it because I need to allocate memory that will live
for as long as the connection exists, not until the Lua chunk returns.
zhuizhuhaomeng
zhuizhuhaomeng previously approved these changes May 8, 2025
@climagabriel
Copy link
Contributor Author

climagabriel commented May 9, 2025

I read a bit more about openresty and realized that data persisting beyond the lifetime of a request goes against the design principles of the module so I'm closing this PR.

@climagabriel climagabriel deleted the feature_connection_pointer_in_ssl_client_hello_by branch May 9, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants