Miscellaneous fixes #771
Merged
Miscellaneous fixes #771
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix critical race conditions that could cause process crashes and resource leaks: - stop_async_response/2: Add proper process monitoring to handle stream death - terminate_async_response/2: Replace unsafe monitor pattern with robust termination - Remove unused helper functions that were prone to race conditions
Fix resource leaks that could accumulate over time: - Ensure proper socket cleanup in controlling_process error paths - Fix timer message draining in cancel_timer to prevent mailbox buildup - Add safety timeouts to handle message delivery edge cases
Fix badmatch error in checkout_timeout test caused by connections not being
returned to pool after skip_body completion:
- Ensure connections are properly returned to pool in close_request when done
- Increase checkout_timeout in pool test to allow proper connection cycling
- Prevents {error,checkout_timeout} when pool connections should be available
Prevent basic authentication credentials from being sent over unencrypted HTTP connections to protect against credential interception: - Block basic_auth over HTTP by default with clear error message - Add insecure_basic_auth option to explicitly bypass security check - Update documentation to warn about security implications - Always allow basic_auth over HTTPS connections - Update integration tests to use bypass option for HTTP test scenarios This prevents accidental credential exposure while maintaining backward compatibility for users who explicitly need HTTP basic auth.
Replace ubuntu-20.04 with ubuntu-22.04 for legacy OTP job to avoid GitHub Actions runner retirement scheduled for 2025-04-15. Both ci and legacy jobs now use ubuntu-22.04 LTS for consistency.
Allow users to set insecure_basic_auth globally via application variable
for simplified usage across all requests:
- Support application:set_env(hackney, insecure_basic_auth, true)
- Option takes precedence over application variable if both are set
- Update documentation to explain global setting option
- Update error message to mention application variable
- Add test to verify application variable functionality
Usage:
%% Global setting - affects all requests
application:set_env(hackney, insecure_basic_auth, true),
hackney:get("http://api.example.com", [], [], [{basic_auth, {"user", "pass"}}])
%% Per-request setting - overrides global
hackney:get("http://api.example.com", [], [], [
{basic_auth, {"user", "pass"}},
{insecure_basic_auth, false} % blocks even if global is true
])
Handle gen_tcp:controlling_process/2 errors instead of crashing with match error. When controlling_process fails, close the socket and try the next IP address. Fixes #768 - badarg error at line 149 of hackney_happy.erl
Ensure hostname verification is preserved when custom SSL options are provided by using merge_ssl_opts instead of completely overriding default options. Add test for googleapis.com connection with custom SSL options to verify hostname verification works correctly. Fixes #766 - TLS handshake failure with googleapis.com
Add test that replicates the exact Elixir scenario from issue #766 that was failing in version 1.22.0. Verifies that custom SSL options with hostname verification work correctly.
Properly handle {ssl_closed, _} and {ssl_error, _} messages in
async_recv to prevent them from leaking to user processes.
Closes #464
Ensure redirect responses consume body to properly release pool connections, preventing pool exhaustion. Closes #717
Prevent legitimate timeout messages from being consumed when timer cancellation succeeds, eliminating potential message loss between concurrent timers.
Ensure sockets are properly closed when controlling_process recovery fails, preventing socket leaks in error scenarios.
Add proper error handling to ETS delete operations in hackney_manager.erl to prevent stale entries when deletion fails. Use catch/1 to ensure cleanup continues even if individual ETS operations fail.
Add proper monitor cleanup in terminate_async_response to prevent deadlocks and stale monitor messages. Ensures erlang:demonitor is called in all code paths to clean up process monitors.
Improve atomic state updates in end_stream_body to reduce race conditions when multiple processes access client state. Use consistent variable naming to make state transitions clearer.
Replace infinity timeout with 30 second timeout in gen_server:call to prevent processes from hanging indefinitely when the manager process is unresponsive or overloaded.
Ignore setopts errors in stream handling to prevent crashes when socket operations are called on closed or invalid sockets during async stream processing and state transitions.
Preserve buffer data along with error reason in stream_body_recv_error to prevent loss of potentially important context information when socket errors occur during response body streaming.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
miscellaneous fixes:
** Breaking Change **
The new
insecure_basic_authapplication variable defaults tofalsefor security.If your application relies on insecure basic auth over HTTP, you must explicitly set
application:set_env(hackney, insecure_basic_auth, true)to maintain previous behavior.