Miscellaneous fixes

.github/workflows/erlang.yml

@@ -13,8 +13,8 @@ jobs:
     runs-on: ${{matrix.os}}
     strategy:
       matrix:
-        otp: ["25.3", "26.2", "27.2"]
-        rebar3: ['3.22.1']
+        otp: ["27.3", "28.0"]
+        rebar3: ['3.25.0']
         os: [ubuntu-22.04]
     steps:
       - uses: actions/checkout@v4
@@ -37,9 +37,9 @@ jobs:
     runs-on: ${{matrix.os}}
     strategy:
       matrix:
-        otp: ["22.3", "23.3", "24.3"]
-        rebar3: ['3.18.0']
-        os: [ubuntu-20.04]
+        otp: ["24.3", "25.3", "26.2"]
+        rebar3: ['3.22.1']
+        os: [ubuntu-22.04]
     steps:
       - uses: actions/checkout@v4
       - uses: erlef/setup-beam@v1

LICENSE

@@ -1,4 +1,4 @@
-2012-2025 (c) Benoît Chesneau <bchesneau@pm.me>
+2012-2025 (c) Benoît Chesneau <benoitc@enki-multimedia.eu>
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

NEWS.md

@@ -1,5 +1,22 @@
 # NEWS
 
+1.24.0 - 2025-05-26
+-------------------
+
+- security: fix basic auth credential exposure vulnerability
+- security: add application variable support for insecure_basic_auth
+- fix: SSL hostname verification with custom ssl_options and SSL message leak in async streaming
+- fix: pool connections not freed on 307 redirects and multiple pool/timer race conditions
+- fix: socket leaks, process deadlocks, ETS memory leaks, and infinite gen_server calls
+- fix: controlling_process error handling in happy eyeballs and connection pool return
+- improvement: update GitHub Actions to ubuntu-22.04 and bump certifi/mimerl dependencies
+
+** Breaking Change **
+
+The new `insecure_basic_auth` application variable defaults to `false` for 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.
+
 1.23.0 - 2025-02-25
 -------------------
 

README.md

@@ -4,7 +4,7 @@
 
 Copyright (c) 2012-2025 Benoît Chesneau.
 
-__Version:__ 1.23.0
+__Version:__ 1.25.0
 
 # hackney
 
@@ -29,8 +29,7 @@ __Version:__ 1.23.0
 - Optional socket pool
 - REST syntax: `hackney:Method(URL)` (where a method can be get, post, put, delete, ...)
 
-**Supported versions** of Erlang are 22.3 and above. It is
-reported to work with version from 19.3 to 21.3.
+**Supported versions** of Erlang are 25.3 and above. 
 
 > Note: This is a work in progress, see the
 [TODO](http://github.com/benoitc/hackney/blob/master/TODO.md) for more

src/hackney.app.src

@@ -4,7 +4,7 @@
 {application, hackney,
   [
     {description, "simple HTTP client"},
-    {vsn, "1.23.0"},
+    {vsn, "1.24.0"},
     {registered, [hackney_pool]},
     {applications, [kernel,
       stdlib,

src/hackney.erl

@@ -237,7 +237,11 @@ request(Method, URL, Headers, Body) ->
 %%          redirection for a request</li>
 %%          <li>`{force_redirect, boolean}': false by default, to force the
 %%          redirection even on POST</li>
-%%          <li>`{basic_auth, {binary, binary}}`: HTTP basic auth username and password.</li>
+%%          <li>`{basic_auth, {binary, binary}}`: HTTP basic auth username and password. 
+%%          Only allowed over HTTPS unless {insecure_basic_auth, true} is also set.</li>
+%%          <li>`{insecure_basic_auth, boolean}': false by default. When true, allows 
+%%          basic auth over unencrypted HTTP connections (security risk). 
+%%          Can also be set globally via application:set_env(hackney, insecure_basic_auth, true).</li>
 %%          <li>`{proxy, proxy_options()}': to connect via a proxy.</li>
 %%          <li>`insecure': to perform "insecure" SSL connections and
 %%          transfers without checking the certificate</li>
@@ -861,7 +865,7 @@ do_connect(ProxyHost, ProxyPort, {ProxyUser, ProxyPass}, Transport, Host, Port,
 
 maybe_redirect({ok, _}=Resp, _Req) -> Resp;
 maybe_redirect(
-  {ok, S, _H, #client{headers=Headers, follow_redirect=true, retries=Tries}=Client}=Resp,
+  {ok, S, H, #client{headers=Headers, follow_redirect=true, retries=Tries}=Client}=Resp,
   Req
  ) when Tries > 0 ->
   %% check if the given location is an absolute url,
@@ -1097,8 +1101,35 @@ reply_response(
 reply_response({ok, Status, Headers, #client{request_ref=Ref}=NState}, _State) ->
   case NState#client.with_body of
     false ->
-      hackney_manager:update_state(NState),
-      {ok, Status, Headers, Ref};
+      %% For redirect responses with pools, ensure body is consumed to properly clean up connections
+      IsRedirect = lists:member(Status, [301, 302, 303, 307, 308]),
+      IsPool = hackney_connect:is_pool(NState) /= false,
+      case IsRedirect andalso IsPool of
+        true ->
+          %% Skip the body to ensure proper connection cleanup for pool redirects
+          case hackney_response:skip_body(NState) of
+            {skip, CleanNState} ->
+              %% For pools, the connection cleanup should have happened in skip_body
+              %% Check if this breaks existing API by checking pool name
+              PoolName = proplists:get_value(pool, NState#client.options, default),
+              case PoolName of
+                default ->
+                  %% For default pool, preserve existing API compatibility
+                  hackney_manager:update_state(CleanNState),
+                  {ok, Status, Headers, Ref};
+                _ ->
+                  %% For custom pools, use full cleanup to ensure connection release
+                  maybe_update_req(CleanNState),
+                  {ok, Status, Headers, Ref}
+              end;
+            Error ->
+              hackney_manager:handle_error(NState),
+              Error
+          end;
+        false ->
+          hackney_manager:update_state(NState),
+          {ok, Status, Headers, Ref}
+      end;
     true ->
       reply_with_body(Status, Headers, NState)
   end;

src/hackney_connection.erl

@@ -119,7 +119,7 @@ ssl_opts(Host, Options) ->
     [] ->
       ssl_opts_1(Host, Options);
     SSLOpts ->
-      SSLOpts
+      merge_ssl_opts(Host, SSLOpts)
   end.
 
 ssl_opts_1(Host, Options) ->

src/hackney_happy.erl

@@ -146,8 +146,14 @@ try_connect([{IP, Type} | Rest], Port, Opts, Timeout, ServerPid, _LastError) ->
   case gen_tcp:connect(IP, Port, [Type | Opts], Timeout) of
     {ok, Socket} = OK ->
       ?report_trace("success to connect", [{ip, IP}, {type, Type}]),
-      ok = gen_tcp:controlling_process(Socket, ServerPid),
-      exit({happy_connect, OK});
+      case gen_tcp:controlling_process(Socket, ServerPid) of
+        ok ->
+          exit({happy_connect, OK});
+        {error, Reason} ->
+          ?report_trace("controlling_process failed", [{error, Reason}]),
+          _ = gen_tcp:close(Socket),
+          try_connect(Rest, Port, Opts, Timeout, ServerPid, {error, Reason})
+      end;
     Error ->
       try_connect(Rest, Port, Opts, Timeout, ServerPid, Error)
   end.

src/hackney_manager.erl

@@ -111,13 +111,24 @@ close_request(#client{}=Client) ->
 
   %% remove the request
   erase(Ref),
-  ets:delete(?MODULE, Ref),
+  catch ets:delete(?MODULE, Ref),
 
   %% stop to monitor the request
   ok = gen_server:cast(?MODULE, {cancel_request, Ref}),
 
   case Status of
-    done -> ok;
+    done when Socket /= nil ->
+      %% Connection completed successfully, return to pool if using one
+      case Client#client.socket_ref of
+        nil ->
+          %% Not using pool, close the socket
+          catch Transport:close(Socket);
+        SocketRef ->
+          %% Return to pool
+          Handler = Client#client.pool_handler,
+          catch Handler:checkin(SocketRef, Socket)
+      end,
+      ok;
     _ when Socket /= nil ->
         catch Transport:controlling_process(Socket, self()),
         catch Transport:close(Socket),
@@ -179,7 +190,7 @@ start_async_response(Ref) ->
   end.
 
 stop_async_response(Ref) ->
-  gen_server:call(?MODULE, {stop_async_response, Ref, self()}, infinity).
+  gen_server:call(?MODULE, {stop_async_response, Ref, self()}, 30000).
 
 async_response_pid(Ref) ->
   case ets:lookup(?REFS, Ref) of
@@ -213,7 +224,7 @@ get_state(Ref) ->
           %% owner can handle it.
           put(Ref, State),
           %% delete the state, from ets
-          ets:delete(?MODULE, Ref),
+          catch ets:delete(?MODULE, Ref),
           State
       end;
     State ->
@@ -241,10 +252,10 @@ store_state(Ref, NState) ->
 
 take_control(Ref, NState) ->
   %% maybe delete the state from ets
-  ets:delete(?MODULE, Ref),
+  catch ets:delete(?MODULE, Ref),
   %% add the state to the current context
   put(Ref, NState),
-  gen_server:call(?MODULE, {take_control, Ref, NState}, infinity).
+  gen_server:call(?MODULE, {take_control, Ref, NState}, 30000).
 
 handle_error(#client{request_ref=Ref, dynamic=true}) ->
   close_request(Ref);
@@ -338,26 +349,46 @@ handle_call({stop_async_response, Ref, To}, _From, State) ->
       %% there is no async request to handle, just return
       {reply, {ok, Ref}, State};
     [{Ref, {Owner, Stream, Info}}] ->
+      %% Monitor the stream process to avoid race conditions
+      MonitorRef = erlang:monitor(process, Stream),
       %% tell to the stream to stop
       Stream ! {Ref, stop_async, self()},
       receive
         {Ref, ok} ->
-          %% if the stream return, we unlink it and update the
-          %% state. if we stop the async request and want to use it
-          %% in another process, make sure to unlink the old owner
-          %% and link the new one.
+          %% Clean shutdown received
+          erlang:demonitor(MonitorRef, [flush]),
           unlink(Stream),
           ets:insert(?REFS, {Ref, {To, nil, Info}}),
           Pids1 = dict:erase(Stream, State#mstate.pids),
           %% if the owner change we need to track the request for this new pid
+          Pids2 = case To of
+                    Owner -> Pids1;
+                    _ ->
+                      track_owner(To, Ref, untrack_owner(Owner, Ref, Pids1))
+                  end,
+          {reply, {ok, Ref}, State#mstate{pids=Pids2}};
+        {'DOWN', MonitorRef, process, Stream, _Reason} ->
+          %% Stream died before responding, clean up and continue
+          ets:insert(?REFS, {Ref, {To, nil, Info}}),
+          Pids1 = dict:erase(Stream, State#mstate.pids),
           Pids2 = case To of
                     Owner -> Pids1;
                     _ ->
                       track_owner(To, Ref, untrack_owner(Owner, Ref, Pids1))
                   end,
           {reply, {ok, Ref}, State#mstate{pids=Pids2}}
       after 5000 ->
-        {reply, {error, timeout}, State}
+        %% Timeout - force cleanup
+        erlang:demonitor(MonitorRef, [flush]),
+        catch unlink(Stream),
+        ets:insert(?REFS, {Ref, {To, nil, Info}}),
+        Pids1 = dict:erase(Stream, State#mstate.pids),
+        Pids2 = case To of
+                  Owner -> Pids1;
+                  _ ->
+                    track_owner(To, Ref, untrack_owner(Owner, Ref, Pids1))
+                end,
+        {reply, {error, timeout}, State#mstate{pids=Pids2}}
       end
   end;
 
@@ -382,7 +413,7 @@ handle_cast({cancel_request, Ref}, State) ->
     [{Ref, {Owner, nil, #request_info{pool=Pool}=Info}}] ->
       %% no stream just cancel the request and untrack the owner.
       Pids2 = untrack_owner(Owner, Ref, State#mstate.pids),
-      ets:delete(?REFS, Ref),
+      catch ets:delete(?REFS, Ref),
       %% notify the pool that the request have been canceled
       PoolHandler:notify(Pool, {'DOWN', Ref, request, Owner, cancel}),
       %% update metrics
@@ -392,7 +423,7 @@ handle_cast({cancel_request, Ref}, State) ->
       %% unlink the stream and untrack the owner
       unlink(Stream),
       Pids2 = dict:erase(Stream, untrack_owner(Owner, Ref, State#mstate.pids)),
-      ets:delete(?REFS, Ref),
+      catch ets:delete(?REFS, Ref),
       %% notify the pool that the request have been canceled
       _ = PoolHandler:notify(Pool, {'DOWN', Ref, request, Owner, cancel}),
       %% update metrics
@@ -509,8 +540,8 @@ clean_requests([Ref | Rest], Pid, Reason, PoolHandler, State) ->
       %% cleanup socket
       ok = cleanup_socket(Ref),
       %% remove the reference
-      ets:delete(?REFS, Ref),
-      ets:delete(?MODULE, Ref),
+      catch ets:delete(?REFS, Ref),
+      catch ets:delete(?MODULE, Ref),
       %% notify the pool that the request have been canceled
       PoolHandler:notify(Pool, {'DOWN', Ref, request, Pid, Reason}),
       %% update metrics
@@ -525,8 +556,8 @@ clean_requests([Ref | Rest], Pid, Reason, PoolHandler, State) ->
       %% cleanup socket
       ok = cleanup_socket(Ref),
       %% remove the reference
-      ets:delete(?REFS, Ref),
-      ets:delete(?MODULE, Ref),
+      catch ets:delete(?REFS, Ref),
+      catch ets:delete(?MODULE, Ref),
       %% notify the pool that the request have been canceled
       PoolHandler:notify(Pool, {'DOWN', Ref, request, Pid, Reason}),
       %% update metrics
@@ -537,28 +568,29 @@ clean_requests([Ref | Rest], Pid, Reason, PoolHandler, State) ->
 clean_requests([], _Pid, _Reason, _PoolHandler, State) ->
   State.
 
-monitor_child(Pid) ->
-  erlang:monitor(process, Pid),
-  unlink(Pid),
-  receive
-    {'EXIT', Pid, _} ->
-      true
-  after 0 ->
-    true
-  end.
-
 terminate_async_response(StreamPid) ->
   terminate_async_response(StreamPid, shutdown).
 
 terminate_async_response(StreamPid, Reason) ->
-  _ = monitor_child(StreamPid),
+  MonitorRef = erlang:monitor(process, StreamPid),
+  unlink(StreamPid),
   exit(StreamPid, Reason),
-  wait_async_response(StreamPid).
-
-wait_async_response(Stream) ->
   receive
-    {'DOWN', _MRef, process, Stream, _Reason} ->
+    {'DOWN', MonitorRef, process, StreamPid, _} ->
+      erlang:demonitor(MonitorRef, [flush]),
       ok
+  after 5000 ->
+      %% Force kill if not terminated
+      exit(StreamPid, kill),
+      receive
+        {'DOWN', MonitorRef, process, StreamPid, _} ->
+          erlang:demonitor(MonitorRef, [flush]),
+          ok
+      after 1000 ->
+          %% Process should be dead now, clean up monitor
+          erlang:demonitor(MonitorRef, [flush]),
+          ok  %% Give up if still not dead
+      end
   end.