Skip to content

Commit

Permalink
gen_smtp_client: stop filtering gen_tcp and ssl options, fixes #271
Browse files Browse the repository at this point in the history
A lot of new useful options were added to gen_tcp and ssl in the recent
OTP releases, but `smtp_socket` module (that backs gen_smtp_client) used
to only allow a smal set of options and discarded all the not defined in
the whitelist.
We think that it's better to let programmer decide which options they
want to pass - it should e programmer's responsibility if they provide
invalid options.
  • Loading branch information
seriyps committed Aug 20, 2021
1 parent 18ad70d commit 3cfd421
Showing 1 changed file with 6 additions and 22 deletions.
28 changes: 6 additions & 22 deletions src/smtp_socket.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,13 @@
{keyfile, "server.key"},
{packet, line},
{reuse_sessions, false},
{reuseaddr, true},
{ssl_imp, new}]).
{reuseaddr, true}]).
-define(SSL_CONNECT_OPTIONS,[ {active, false},
{depth, 0},
{packet, line},
{ip, {0,0,0,0}},
{versions, ['tlsv1', 'tlsv1.1', 'tlsv1.2']},
{port, 0}]).

-define(SSL_CONNECT_OPTIONS_KEYS, [ciphers]).

-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
-endif.
Expand Down Expand Up @@ -320,12 +316,8 @@ proplist_merge(PrimaryList, DefaultList) ->
lists:keysort(1, PrimaryTuples),
lists:keysort(1, DefaultTuples)
),
MergedOther = lists:merge(lists:sort(PrimaryOther), lists:sort(DefaultOther)),

%% remove all the values that don't belong here
[Option || Option = {Key, _} <- MergedTuples,
lists:member(Key, ?SSL_CONNECT_OPTIONS_KEYS) or proplists:is_defined(Key, DefaultList)]
++ [Option || Option <- MergedOther, Option == inet6 ].
MergedOther = lists:umerge(lists:sort(PrimaryOther), lists:sort(DefaultOther)),
MergedTuples ++ MergedOther.

parse_address(Options) ->
case proplists:get_value(ip, Options) of
Expand Down Expand Up @@ -558,11 +550,6 @@ active_once_test_() ->

option_test_() ->
[
{"options removes bogus values",
fun() ->
?assertEqual(lists:sort([list|?TCP_LISTEN_OPTIONS]), lists:sort(tcp_listen_options([{notvalid,whatever}])))
end
},
{"tcp_listen_options has defaults",
fun() ->
?assertEqual(lists:sort([list|?TCP_LISTEN_OPTIONS]), lists:sort(tcp_listen_options([])))
Expand Down Expand Up @@ -637,8 +624,7 @@ option_test_() ->
{keyfile, "server.key"},
{packet, 2},
{reuse_sessions, false},
{reuseaddr, true},
{ssl_imp, new}])],
{reuseaddr, true}])],
ssl_listen_options([{active, true},{packet,2}])),
?assertEqual([list|lists:keysort(1, [{active, false},
{backlog, 30},
Expand All @@ -648,8 +634,7 @@ option_test_() ->
{keyfile, "../server.key"},
{packet, line},
{reuse_sessions, false},
{reuseaddr, true},
{ssl_imp, new}])],
{reuseaddr, true}])],
ssl_listen_options([{certfile, "../server.crt"}, {keyfile, "../server.key"}]))
end
},
Expand All @@ -659,8 +644,7 @@ option_test_() ->
{depth, 0},
{ip, {0,0,0,0}},
{port, 0},
{packet, 2},
{versions, [tlsv1,'tlsv1.1','tlsv1.2']}]),
{packet, 2}]),
lists:sort(ssl_connect_options([{active, true},{packet,2}])))
end
}
Expand Down

0 comments on commit 3cfd421

Please sign in to comment.