validate options before passing the to the socket process

This allows us to throw badarg errors instead of crashing the
socket process.

Author: Andreas Schultz

src/ezmq.erl

@@ -49,9 +49,11 @@
 %% @end
 %%--------------------------------------------------------------------
 
-start_link(Opts) when is_list(Opts) ->
+start_link(RawOpts) when is_list(RawOpts) ->
+    Opts = lists:map(fun validate_opts/1, proplists:unfold(RawOpts)),
     gen_server:start_link(?MODULE, {self(), Opts}, [?SERVER_OPTS]).
-start(Opts) when is_list(Opts) ->
+start(RawOpts) when is_list(RawOpts) ->
+    Opts = lists:map(fun validate_opts/1, proplists:unfold(RawOpts)),
     gen_server:start(?MODULE, {self(), Opts}, [?SERVER_OPTS]).
 
 socket_link(Opts) when is_list(Opts) ->
@@ -93,7 +95,8 @@ recv(Socket) ->
 recv(Socket, Timeout) ->
     gen_server:call(Socket, {recv, Timeout}, infinity).
 
-setopts(Socket, Opts) ->
+setopts(Socket, RawOpts) ->
+    Opts = lists:map(fun validate_opts/1, proplists:unfold(RawOpts)),
     gen_server:call(Socket, {setopts, Opts}).
 
 sockname(Socket) ->
@@ -227,7 +230,7 @@ init_socket(Owner, Type, Opts) ->
     process_flag(trap_exit, true),
     MqSState0 = #ezmq_socket{owner = Owner, mode = passive, recv_q = orddict:new(),
                                 connecting = orddict:new(), listen_trans = orddict:new(), transports = [], remote_ids = orddict:new()},
-    MqSState1 = lists:foldl(fun do_setopts/2, MqSState0, proplists:unfold(Opts)),
+    MqSState1 = lists:foldl(fun do_setopts/2, MqSState0, Opts),
     ezmq_socket_fsm:init(Type, Opts, MqSState1).
 
 %%--------------------------------------------------------------------
@@ -307,7 +310,7 @@ handle_call({send, Msg}, From, State) ->
     end;
 
 handle_call({setopts, Opts}, _From, State) ->
-    NewState = lists:foldl(fun do_setopts/2, State, proplists:unfold(Opts)),
+    NewState = lists:foldl(fun do_setopts/2, State, Opts),
     {reply, ok, NewState};
 
 handle_call(sockname, _From, #ezmq_socket{listen_trans = ListenTrans, transports = Transports} = State) ->
@@ -653,25 +656,50 @@ do_dequeue(Transport, Q) ->
 do_setopts({type, Type}, MqSState) ->
     MqSState#ezmq_socket{type = Type};
 do_setopts({identity, Id}, MqSState) ->
-    MqSState#ezmq_socket{identity = iolist_to_binary(Id)};
+    MqSState#ezmq_socket{identity = Id};
 do_setopts({active, once}, MqSState) ->
     run_recv_q(MqSState#ezmq_socket{mode = active_once});
 do_setopts({active, true}, MqSState) ->
     run_recv_q(MqSState#ezmq_socket{mode = active});
 do_setopts({active, false}, MqSState) ->
     MqSState#ezmq_socket{mode = passive};
-do_setopts({need_events, NeedEvents}, MqSState) when is_boolean(NeedEvents) ->
+do_setopts({need_events, NeedEvents}, MqSState) ->
     MqSState#ezmq_socket{need_events = NeedEvents};
 do_setopts({version, Version}, MqSState) ->
+    MqSState#ezmq_socket{version = Version};
+
+do_setopts(_, MqSState) ->
+    MqSState.
+
+validate_opts(Opt = {type, Type})
+  when Type == req;    Type == rep;
+       Type == dealer; Type == router;
+       Type == pub;    Type == sub ->
+    Opt;
+
+validate_opts(Opt = {identity, Id}) when is_binary(Id) ->
+    Opt;
+validate_opts({identity, Id}) when is_list(Id) ->
+    {identity, iolist_to_binary(Id)};
+
+validate_opts(Opt = {active, once}) -> Opt;
+validate_opts(Opt = {active, true}) -> Opt;
+validate_opts(Opt = {active, false}) -> Opt;
+
+validate_opts(Opt = {need_events, NeedEvents}) when is_boolean(NeedEvents) ->
+    Opt;
+
+validate_opts(Opt = {version, Version}) ->
     case lists:member(Version, ?SUPPORTED_VERSIONS) of
         true ->
-            MqSState#ezmq_socket{version = Version};
+	    Opt;
         _ ->
-            erlang:error(badargs, [{version, Version}])
+            erlang:error(badargs, [Opt])
     end;
 
-do_setopts(_, MqSState) ->
-    MqSState.
+validate_opts(Opt) ->
+    erlang:error(badarg, [Opt]).
+
 
 do_sockname(Transport, _Opts, Acc) ->
     case ezmq_tcp_socket:sockname(Transport) of

test/ezmq_SUITE.erl

@@ -28,13 +28,13 @@ req_tcp_connect_close(Config) ->
     TcpOpts = tcp_opts(Config),
     IP = proplists:get_value(localhost, Config, {127,0,0,1}),
     {ok, S} = ezmq_client_socket([{type, req}, {active, false}]),
-    ok = ezmq:connect(S, tcp, IP, 5555, [TcpOpts]),
+    ok = ezmq:connect(S, tcp, IP, 5555, TcpOpts),
     ezmq:close(S).
 
 req_tcp_connect_fail(Config) ->
     TcpOpts = tcp_opts(Config),
-    {ok, S} = ezmq_client_socket([{type, req}, {active, false}|TcpOpts]),
-    {error,nxdomain} = ezmq:connect(S, tcp, "undefined.undefined", 5555, []),
+    {ok, S} = ezmq_client_socket([{type, req}, {active, false}]),
+    {error,nxdomain} = ezmq:connect(S, tcp, "undefined.undefined", 5555, TcpOpts),
     ezmq:close(S).
 
 req_tcp_connect_timeout(Config) ->
@@ -69,7 +69,7 @@ dealer_tcp_connect_close(Config) ->
     TcpOpts = tcp_opts(Config),
     IP = proplists:get_value(localhost, Config, {127,0,0,1}),
     {ok, S} = ezmq_client_socket([{type, dealer}, {active, false}]),
-    ok = ezmq:connect(S, tcp, IP, 5555, [TcpOpts]),
+    ok = ezmq:connect(S, tcp, IP, 5555, TcpOpts),
     ezmq:close(S).
 
 dealer_tcp_connect_timeout(Config) ->
@@ -363,7 +363,7 @@ create_bound_pair(Config, Type1, Type2, Mode) ->
     ok = ezmq:bind(S1, tcp, 0, [{reuseaddr, true}|TcpOpts]),
     {ok, [{_, IP, Port}|_]} = ezmq:sockname(S1),
     ct:pal("IP: ~p, Port: ~p~n", [IP, Port]),
-    ok = ezmq:connect(S2, tcp, IP, Port, [TcpOpts]),
+    ok = ezmq:connect(S2, tcp, IP, Port, TcpOpts),
     {S1, S2}.
 
 %% assert that message queue is empty....

test/ezmq_event_SUITE.erl

@@ -34,8 +34,8 @@ ezmq_server_events_t(Contig) ->
     Port = 5555,
     ClientIdentity = <<"client">>,
     ServerIdentity = <<"server">>,
-    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {reuseaddr, true}, {identity, ServerIdentity}, {need_events, true}]),
-    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}])),
+    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {identity, ServerIdentity}, {need_events, true}]),
+    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}, {reuseaddr, true}])),
     receive
         {zmq_event, ServerSocket, Event} ->
             ?debugFmt("ERROR: No connections. Unhandled events: ~p~n", [Event]),
@@ -92,8 +92,8 @@ ezmq_client_events_t(Contig) ->
     Port = 5555,
     ClientIdentity = <<"client">>,
     ServerIdentity = <<"server">>,
-    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {reuseaddr, true}, {identity, ServerIdentity}]),
-    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}])),
+    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {identity, ServerIdentity}]),
+    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}, {reuseaddr, true}])),
     receive
         {zmq_event, ServerSocket, Event} ->
             ?debugFmt("ERROR: No connections. Unhandled events: ~p~n", [Event]),
@@ -150,8 +150,8 @@ no_events_t(Contig) ->
     Port = 5555,
     ClientIdentity = <<"client">>,
     ServerIdentity = <<"server">>,
-    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {identity, ServerIdentity}, {reuseaddr, true}]),
-    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}])),
+    {ok, ServerSocket} = ezmq:socket([{type, router}, {active, true}, {identity, ServerIdentity}]),
+    ?assertEqual(ok, ezmq:bind(ServerSocket, tcp, Port, [{ip, IP}, {reuseaddr, true}])),
     %% Start client
     {ok, ClientSocket} = ezmq:start([{type, dealer}, {identity, ClientIdentity}]),
     ezmq:connect(ClientSocket, tcp, IP, Port, []),
@@ -178,4 +178,4 @@ clear_mailbox() ->
     after 0 ->
         ok
     end.
-%%++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+%%++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++