Skip to content

Commit 7d0e5b9

Browse files
Piyush0049lukebakken
authored andcommitted
Address maintainer review findings for UDS support
- Fix socket_ends/2 tuple decomposition causing is_loopback failures (Finding #1) - Restore {error, enoent} assertion in uds_connection_failure tests (Finding #2) - Handle the bare atom 'local' in rabbit_misc:ntoa/1 (Finding #3) - Drop overly strict hd(Path) guard from tcp_listener_addresses/1 (Finding #4) - Remove 'string' from tcp and ssl listener schema datatypes (Finding #5) - Restore {error, einval} fallback in rabbit_auth_backend_http - Normalise track_auth_attempt_source tuple extraction in rabbit_core_metrics
1 parent 344b685 commit 7d0e5b9

8 files changed

Lines changed: 25 additions & 13 deletions

File tree

deps/rabbit/priv/schema/rabbit.schema

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
]}.
2424

2525
{mapping, "listeners.tcp.$name", "rabbit.tcp_listeners",[
26-
{datatype, [integer, ip, string, domain_socket]}
26+
{datatype, [integer, ip, domain_socket]}
2727
]}.
2828

2929
{translation, "rabbit.tcp_listeners",
@@ -46,7 +46,7 @@ end}.
4646
]}.
4747

4848
{mapping, "listeners.ssl.$name", "rabbit.ssl_listeners",[
49-
{datatype, [integer, ip, string, domain_socket]}
49+
{datatype, [integer, ip, domain_socket]}
5050
]}.
5151

5252
{translation, "rabbit.ssl_listeners",

deps/rabbit/src/rabbit_networking.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ fix_ssl_options(Config) ->
149149

150150
tcp_listener_addresses({local, Path, _Port}) ->
151151
[{{local, Path}, 0, local}];
152-
tcp_listener_addresses(Path) when is_list(Path) andalso Path =/= [] andalso (hd(Path) =:= $/ orelse hd(Path) =:= $.) ->
152+
tcp_listener_addresses(Path) when is_list(Path) andalso Path =/= [] ->
153153
[{{local, Path}, 0, local}];
154154
tcp_listener_addresses(Port) when is_integer(Port) ->
155155
tcp_listener_addresses_auto(Port);

deps/rabbit/test/unix_domain_socket_SUITE.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ test_uds_connection_failure(Config) ->
129129
file:delete(SocketPath),
130130

131131
ConnParams = #amqp_params_network{host = {local, SocketPath}, port = 0},
132-
?assertMatch({error, _}, amqp_connection:start(ConnParams)).
132+
?assertMatch({error, enoent}, amqp_connection:start(ConnParams)).
133133

134134
%% Verifies finding #1 at the unit level: is_loopback must return true for the
135135
%% {local, _} tuple form that peername returns on an accepted UDS socket.

deps/rabbit_common/src/rabbit_core_metrics.erl

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,16 @@ update_auth_attempt(RemoteAddress, Username, Protocol, Incr) ->
422422
%% It's up to the operator to enable them, and reset it required
423423
_ = case application:get_env(rabbit, track_auth_attempt_source) of
424424
{ok, true} ->
425-
Addr = case inet:is_ip_address(RemoteAddress) of
426-
true ->
425+
Addr = case RemoteAddress of
426+
{local, _Path} ->
427427
list_to_binary(rabbit_misc:ntoa(RemoteAddress));
428-
false ->
429-
rabbit_data_coercion:to_binary(RemoteAddress)
428+
_ ->
429+
case inet:is_ip_address(RemoteAddress) of
430+
true ->
431+
list_to_binary(rabbit_misc:ntoa(RemoteAddress));
432+
false ->
433+
rabbit_data_coercion:to_binary(RemoteAddress)
434+
end
430435
end,
431436
case {Addr, Username} of
432437
{<<>>, <<>>} ->

deps/rabbit_common/src/rabbit_misc.erl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,8 @@
211211
{bad_edge, [digraph:vertex()]}),
212212
digraph:vertex(), digraph:vertex()}).
213213
-spec const(A) -> thunk(A).
214-
-spec ntoa(inet:ip_address()) -> string().
215-
-spec ntoab(inet:ip_address()) -> string().
214+
-spec ntoa(any()) -> string() | {error, einval}.
215+
-spec ntoab(any()) -> binary() | {error, einval}.
216216
-spec is_process_alive(pid()) -> boolean().
217217

218218
-spec pmerge(term(), term(), [term()]) -> [term()].
@@ -801,13 +801,17 @@ const(X) -> fun () -> X end.
801801
%% when IPv6 is enabled but not used (i.e. 99% of the time).
802802
ntoa({local, Path}) ->
803803
rabbit_data_coercion:to_list(Path);
804+
ntoa(local) ->
805+
"local";
804806
ntoa({0,0,0,0,0,16#ffff,AB,CD}) ->
805807
inet_parse:ntoa({AB bsr 8, AB rem 256, CD bsr 8, CD rem 256});
806808
ntoa(IP) ->
807809
inet_parse:ntoa(IP).
808810

809811
ntoab({local, Path}) ->
810812
rabbit_data_coercion:to_binary(Path);
813+
ntoab(local) ->
814+
<<"local">>;
811815
ntoab(IP) ->
812816
Str = ntoa(IP),
813817
case string:str(Str, ":") of

deps/rabbit_common/src/rabbit_net.erl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ is_loopback(Sock) when is_port(Sock) ; ?IS_SSL(Sock) ->
299299
is_loopback({127,_,_,_}) -> true;
300300
is_loopback({0,0,0,0,0,0,0,1}) -> true;
301301
is_loopback({0,0,0,0,0,65535,AB,CD}) -> is_loopback(ipv4(AB, CD));
302+
is_loopback(local) -> true;
302303
is_loopback({local, _}) -> true;
303304
is_loopback(_) -> false.
304305

deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,4 +294,7 @@ join_tags(Tags) ->
294294
parse_peeraddr(unknown) ->
295295
rabbit_data_coercion:to_list(unknown);
296296
parse_peeraddr(PeerAddr) ->
297-
rabbit_misc:ntoa(PeerAddr).
297+
case rabbit_misc:ntoa(PeerAddr) of
298+
{error, einval} -> rabbit_data_coercion:to_list(PeerAddr);
299+
Str -> Str
300+
end.

deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323
send_fun/0]).
2424

2525
-import(rabbit_mqtt_util, [mqtt_to_amqp/1,
26-
amqp_to_mqtt/1,
27-
ip_address_to_binary/1]).
26+
amqp_to_mqtt/1]).
2827
-import(rabbit_misc, [maps_put_truthy/3]).
2928

3029
-include_lib("kernel/include/logger.hrl").

0 commit comments

Comments
 (0)