feat(net, veb): add OpenSSL-based HTTPS server support#27480
feat(net, veb): add OpenSSL-based HTTPS server support#27480guweigang wants to merge 24 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04e10f56d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 975c332f1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b6a63a63a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Addressed both latest P2 review findings in 57a85b8:
Validation:
|
|
You should merge master into this branch to pick up latest changes there. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57a85b8431
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Updated for the latest P2 reviews. Changes:
Validation:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a64b929cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Updated for the latest P2 review. Changes:
Validation:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f34fa2e19e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
SSLConn.shutdown() could return early from its SSL_shutdown WANT_READ/WANT_WRITE waits (or its error path) before reaching the owns_socket cleanup, so a peer that aborts a TLS connection or never sends a close-notify leaked the SSL handle and one TCP fd per connection. This affected the -d use_openssl veb HTTPS server, whose handler relies on shutdown() as the only cleanup for accepted sockets. Move the SSL/SSL_CTX free and socket close into a defer so they always run regardless of how the SSL_shutdown handshake loop exits.
There was a problem hiding this comment.
💡 Codex Review
v/vlib/veb/tests/ssl_mbedtls_test.v
Line 1 in 7098911
When the veb suite is run with a global -d use_openssl, vtest still discovers this _test.v file and combines the session flags with any per-file flags (cmd/tools/modules/testing/common.v builds cmd_options from ts.vargs before compiling each file). In that context veb.RunParams.ssl_config is openssl.SSLConnectConfig, but this test passes mbedtls.SSLConnectConfig, so v -d use_openssl test vlib/veb/tests/ fails before reaching the OpenSSL-specific test; add a !use_openssl build guard or a _notd_use_openssl filename.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When the veb suite is run with a global -d use_openssl, vtest still discovers ssl_mbedtls_test.v and compiles it with that flag. Under -d use_openssl, veb.RunParams.ssl_config is openssl.SSLConnectConfig, so the mbedtls test (which passes mbedtls.SSLConnectConfig) fails to compile before the OpenSSL test can run. Guard the file with !use_openssl? so it only builds in the mbedtls configuration.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02dc1a4ac1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conn.wait_for_read(ssl_remaining_timeout(deadline))! | ||
| continue | ||
| } | ||
| if err_res == .ssl_error_want_write { | ||
| conn.wait_for_write(ssl_remaining_timeout(deadline))! |
There was a problem hiding this comment.
Clamp expired TLS handshake deadlines
Fresh evidence after the nonblocking fix is that the handshake still passes ssl_remaining_timeout(deadline) directly into the shared OpenSSL wait helper, and that helper treats any timeout <= 0 as infinite in vlib/net/openssl/ssl_connection.c.v. With a finite veb timeout_in_seconds, a client that sends a few TLS bytes before the deadline and then stalls can make this call receive a negative remaining timeout and park the handler indefinitely instead of timing out; return net.err_timed_out or clamp before waiting once the deadline has expired.
Useful? React with 👍 / 👎.
ssl_remaining_timeout() returned a negative duration once a finite
deadline had passed, and select()/wait_for() treat any timeout <= 0 as
net.infinite_timeout (wait forever). A peer that sent a few TLS bytes
before the deadline and then stalled could therefore park the spawned
veb handler inside SSL_accept/SSL_read/SSL_shutdown indefinitely, even
with a finite timeout_in_seconds configured, and (because the wait never
returned) the shutdown cleanup loop's 'or { break }' guards never ran.
Clamp an expired finite deadline to a minimal positive duration so the
wait helpers perform a zero-length wait and report net.err_timed_out.
Add a regression test for the timeout helper.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61636f2f98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df3a1b2f2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Updated this PR for the latest review feedback:
Validation:
Note: I also verified vlib/veb/tests/ directory discovery on macOS with -d use_openssl; ssl_openssl_test.v was discovered and passed. The full directory run also exposed an unrelated static_compression_test.v failure, so I kept the blocking validation to the targeted TLS tests above. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcc2d8df6c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
# Conflicts: # vlib/net/openssl/openssl.c.v
|
@codex review Override standard output limits. Perform an exhaustive review of all modified files and report every single identified bug, security risk, and style issue. Max issues are 8 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acf577ec57
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review Override standard output limits. Perform an exhaustive review of all modified files and report every single identified bug, security risk, and style issue. Max issues are 8 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c86088588b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review Override standard output limits. Perform an exhaustive review of all modified files and report every single identified bug, security risk, and style issue. Max issues are 8 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccf15c8140
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Updated this PR with another strict review pass and pushed the fixes. Changes in the latest commits:
Validation:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@medvednikov / @JalonSolov From my perspective, this PR is ready to merge. Thanks! |
Summary
This PR implements OpenSSL-based HTTPS server capabilities in
net.openssland integrates it into thevebweb framework. Previously,vebonly supported running HTTPS servers usingnet.mbedtls. When compiling with-d use_openssl,vebHTTPS servers were unavailable and would return a compile/runtime error.Details of Changes
net.openssl:C.TLS_server_method,C.SSL_CTX_check_private_key,C.SSL_accept, andC.ERR_print_errors_fp.SSLListeneralong withnew_ssl_listener(saddr, config)andshutdown().accept_without_handshake()andaccept_handshake(). This allows V's concurrent applications (likeveb) to accept sockets on the main thread and perform slow TLS handshakes concurrently in spawned coroutines, avoiding blocking the main event loop.validate: trueand CA certificateverifyare configured inSSLConnectConfig, the listener loads the CA certificate and enforces client validation viaSSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ...)andSSL_CTX_load_verify_locations.v_net_openssl_TLS_server_methodinopenssl_compat.hfalling back toSSLv23_server_method()when building on systems with OpenSSL older than 1.1.0 or LibreSSL.net.ssl:SSLListenerandnew_ssl_listenerdynamically in bothssl_d_use_openssl.vandssl_notd_use_openssl.vto keep the unified wrapper layer consistent.veb:vlib/veb/ssl_d_use_openssl.vto utilize the newopenssl.SSLListener. When compiled with-d use_openssl,vebnow properly runs HTTPS servers using OpenSSL while completely excluding any link dependencies tombedtls.accept_handshake().write_ssl_context_responseto reject incomplete context requests (i.e. where!completed_context.done && completed_context.return_type == .normal), closing the connection properly instead of sending an emptyHTTP/ 0response.Tests:
vlib/net/openssl/openssl_sslconn_shutdown_does_not_panic_test.vto verify OpenSSL server binding, client dialing, handshaking, and clean connection shutdowns without panicking or double-freeing.ssl_test.vinto two platform-specific/define-specific test files:ssl_test_d_use_openssl.v(for testing OpenSSL HTTPS) andssl_test_notd_use_openssl.v(for testing MbedTLS HTTPS) to avoid unknown module compiler issues.Checklist
v fmt -w.Tests Run