Skip to content

Commit 81e0f64

Browse files
michaelklishinmichaelklishin
committed
Refactor to use only one decompose_from_binary/1 variant
Plus address cosmetic review feedback.
1 parent 35661ad commit 81e0f64

4 files changed

Lines changed: 36 additions & 42 deletions

File tree

‎deps/rabbit/src/rabbit_pid_codec.erl‎

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
-export([decompose/1,
1111
decompose_from_binary/1,
12-
decompose_from_binary_safe/1,
1312
recompose/1,
1413
recompose_to_binary/1]).
1514

@@ -33,36 +32,31 @@
3332
-spec decompose(pid()) -> decomposed_pid().
3433
decompose(Pid) ->
3534
Bin = term_to_binary(Pid, [{minor_version, 2}]),
36-
decompose_from_binary(Bin).
35+
{ok, #{node := NodeBin} = Parts} = decompose_from_binary(Bin),
36+
%% Safe: the input is a real local pid, so its node atom always exists.
37+
Parts#{node := binary_to_existing_atom(NodeBin, utf8)}.
3738

38-
-spec decompose_from_binary(binary()) -> decomposed_pid().
39-
decompose_from_binary(Bin) ->
40-
<<?TTB_PREFIX, ?NEW_PID_EXT, PidData/binary>> = Bin,
41-
{Node, Rest} = case PidData of
42-
<<?ATOM_UTF8_EXT, Len:16/integer, Node0:Len/binary, Rest1/binary>> ->
43-
{Node0, Rest1};
44-
<<?SMALL_ATOM_UTF8_EXT, Len/integer, Node0:Len/binary, Rest1/binary>> ->
45-
{Node0, Rest1}
46-
end,
47-
<<ID:32/integer, Serial:32/integer, Creation:32/integer>> = Rest,
48-
#{node => binary_to_atom(Node, utf8),
49-
id => ID,
50-
serial => Serial,
51-
creation => Creation}.
52-
53-
%% Returns the node as a binary and returns 'error' on malformed input.
54-
-spec decompose_from_binary_safe(binary()) -> {ok, decomposed_pid_bin()} | error.
55-
decompose_from_binary_safe(<<?TTB_PREFIX, ?NEW_PID_EXT,
56-
?ATOM_UTF8_EXT, Len:16/integer, Node:Len/binary,
57-
ID:32/integer, Serial:32/integer,
58-
Creation:32/integer>>) ->
59-
{ok, #{node => Node, id => ID, serial => Serial, creation => Creation}};
60-
decompose_from_binary_safe(<<?TTB_PREFIX, ?NEW_PID_EXT,
61-
?SMALL_ATOM_UTF8_EXT, Len:8/integer, Node:Len/binary,
62-
ID:32/integer, Serial:32/integer,
63-
Creation:32/integer>>) ->
64-
{ok, #{node => Node, id => ID, serial => Serial, creation => Creation}};
65-
decompose_from_binary_safe(_) ->
39+
%% Returns the node name as a binary and 'error' on malformed input.
40+
-spec decompose_from_binary(binary()) -> {ok, decomposed_pid_bin()} | error.
41+
decompose_from_binary(<<?TTB_PREFIX, ?NEW_PID_EXT,
42+
?ATOM_UTF8_EXT, Len:16/integer, Node:Len/binary,
43+
ID:32/integer, Serial:32/integer,
44+
Creation:32/integer>>) ->
45+
{ok,
46+
#{node => Node,
47+
id => ID,
48+
serial => Serial,
49+
creation => Creation}};
50+
decompose_from_binary(<<?TTB_PREFIX, ?NEW_PID_EXT,
51+
?SMALL_ATOM_UTF8_EXT, Len:8/integer, Node:Len/binary,
52+
ID:32/integer, Serial:32/integer,
53+
Creation:32/integer>>) ->
54+
{ok,
55+
#{node => Node,
56+
id => ID,
57+
serial => Serial,
58+
creation => Creation}};
59+
decompose_from_binary(_) ->
6660
error.
6761

6862
-spec recompose_to_binary(decomposed_pid()) -> binary().

‎deps/rabbit/src/rabbit_volatile_queue.erl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ pid_from_name(<<?PREFIX, Bin/binary>>, CandidateNodes) ->
377377
try
378378
[PidBase64, _KeyBase64] = binary:split(Bin, Cp),
379379
PidBin = base64:decode(PidBase64),
380-
case rabbit_pid_codec:decompose_from_binary_safe(PidBin) of
380+
case rabbit_pid_codec:decompose_from_binary(PidBin) of
381381
{ok, #{node := ShortenedNodename} = PidParts0} ->
382382
NodeHash = node_hash_from_binary(ShortenedNodename),
383383
case maps:get(NodeHash, CandidateNodes, undefined) of

‎deps/rabbit/test/pid_codec_SUITE.erl‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ end_per_suite(_Config) -> ok.
3535
safe_decoder_roundtrip_self(_) ->
3636
Pid = self(),
3737
Bin = term_to_binary(Pid, [{minor_version, 2}]),
38-
{ok, Parts} = rabbit_pid_codec:decompose_from_binary_safe(Bin),
38+
{ok, Parts} = rabbit_pid_codec:decompose_from_binary(Bin),
3939
Recomposed = rabbit_pid_codec:recompose(Parts#{node := node()}),
4040
?assertEqual(Pid, Recomposed).
4141

4242
safe_decoder_returns_binary_node(_) ->
4343
Bin = term_to_binary(self(), [{minor_version, 2}]),
44-
{ok, #{node := NodeBin}} = rabbit_pid_codec:decompose_from_binary_safe(Bin),
44+
{ok, #{node := NodeBin}} = rabbit_pid_codec:decompose_from_binary(Bin),
4545
?assert(is_binary(NodeBin)),
4646
?assertEqual(atom_to_binary(node()), NodeBin).
4747

@@ -67,20 +67,20 @@ safe_decoder_rejects_malformed(_) ->
6767
<<?TTB_PREFIX, ?NEW_PID_EXT,
6868
?SMALL_ATOM_UTF8_EXT, 1:8, "x", 0:32, 0:32, 0:24>>
6969
],
70-
[?assertEqual(error, rabbit_pid_codec:decompose_from_binary_safe(C))
70+
[?assertEqual(error, rabbit_pid_codec:decompose_from_binary(C))
7171
|| C <- Cases].
7272

7373
safe_decoder_no_atoms_for_unknown_node_names(_) ->
7474
Inputs = [{fresh_node_bytes(I), I} || I <- lists:seq(1, 1000)],
7575
Bins = [{Name, make_pid_bin(Name, I, I, I)} || {Name, I} <- Inputs],
7676
%% Warm up before sampling the atom count.
7777
{_, B0} = hd(Bins),
78-
{ok, _} = rabbit_pid_codec:decompose_from_binary_safe(B0),
78+
{ok, _} = rabbit_pid_codec:decompose_from_binary(B0),
7979
Before = erlang:system_info(atom_count),
8080
lists:foreach(
8181
fun({Name, B}) ->
8282
{ok, #{node := Got}} =
83-
rabbit_pid_codec:decompose_from_binary_safe(B),
83+
rabbit_pid_codec:decompose_from_binary(B),
8484
?assertEqual(Name, Got)
8585
end,
8686
Bins),
@@ -91,7 +91,7 @@ safe_decoder_handles_long_node_names(_) ->
9191
Long = binary:copy(<<"a">>, 65535),
9292
Bin = make_pid_bin(Long, 1, 2, 3),
9393
{ok, #{node := Got, id := 1, serial := 2, creation := 3}} =
94-
rabbit_pid_codec:decompose_from_binary_safe(Bin),
94+
rabbit_pid_codec:decompose_from_binary(Bin),
9595
?assertEqual(Long, Got).
9696

9797
pid_from_name_no_atoms_for_crafted_input(_) ->

‎deps/rabbit/test/pid_codec_prop_SUITE.erl‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ prop_safe_decoder_returns_ok_or_error(_) ->
4141

4242
prop_safe_decoder_returns_ok_or_error_() ->
4343
?FORALL(Bin, binary(),
44-
case rabbit_pid_codec:decompose_from_binary_safe(Bin) of
44+
case rabbit_pid_codec:decompose_from_binary(Bin) of
4545
error -> true;
4646
{ok, #{node := N, id := I, serial := S, creation := C}} ->
4747
is_binary(N)
@@ -63,7 +63,7 @@ prop_safe_decoder_accepts_arbitrary_node_bytes_() ->
6363
Bin = make_pid_bin(NodeBytes, ID, Serial, Creation),
6464
{ok, #{node := Got, id := GotID,
6565
serial := GotS, creation := GotC}} =
66-
rabbit_pid_codec:decompose_from_binary_safe(Bin),
66+
rabbit_pid_codec:decompose_from_binary(Bin),
6767
Got =:= NodeBytes
6868
andalso GotID =:= ID
6969
andalso GotS =:= Serial
@@ -85,7 +85,7 @@ prop_safe_decoder_roundtrip_numeric_fields_() ->
8585
Bin = rabbit_pid_codec:recompose_to_binary(Parts),
8686
{ok, #{node := NB, id := GotID,
8787
serial := GotS, creation := GotC}} =
88-
rabbit_pid_codec:decompose_from_binary_safe(Bin),
88+
rabbit_pid_codec:decompose_from_binary(Bin),
8989
NB =:= NodeBin
9090
andalso GotID =:= ID
9191
andalso GotS =:= Serial
@@ -94,15 +94,15 @@ prop_safe_decoder_roundtrip_numeric_fields_() ->
9494

9595
prop_safe_decoder_no_atom_growth(_) ->
9696
%% Warm up before sampling the atom count.
97-
_ = rabbit_pid_codec:decompose_from_binary_safe(<<>>),
97+
_ = rabbit_pid_codec:decompose_from_binary(<<>>),
9898
rabbit_ct_proper_helpers:run_proper(
9999
fun prop_safe_decoder_no_atom_growth_/0, [], ?PROP_ITERATIONS).
100100

101101
prop_safe_decoder_no_atom_growth_() ->
102102
?FORALL(Bin, binary(),
103103
begin
104104
Before = erlang:system_info(atom_count),
105-
_ = rabbit_pid_codec:decompose_from_binary_safe(Bin),
105+
_ = rabbit_pid_codec:decompose_from_binary(Bin),
106106
erlang:system_info(atom_count) =:= Before
107107
end).
108108

0 commit comments

Comments
 (0)
⚡