inets: Fix broken httpc options handling

- Add support for setting socket options per request. - Add http_ipv6 test group. Change-Id: Ia2aca37c0b5fe64a41995c79ae3399434b17ab8a
erlang · Apr 19, 2018 · 2dc08b4 · 2dc08b4
1 parent 6fad272
commit 2dc08b4
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 8 deletions.
diff --git a/lib/inets/doc/src/httpc.xml b/lib/inets/doc/src/httpc.xml
@@ -450,17 +450,22 @@
 
           <tag><c><![CDATA[socket_opts]]></c></tag>
           <item>
-            <p>Socket options to be used for this and subsequent 
-	    requests.</p>
+            <p>Socket options to be used for this request.</p>
 	    <p>Overrides any value set by function
 	    <seealso marker="#set_options-1">set_options</seealso>.</p>
             <p>The validity of the options is <em>not</em> checked by
             the HTTP client they are assumed to be correct and passed
             on to ssl application and inet driver, which may reject
-            them if they are not correct. Note that the current
-            implementation assumes the requests to the same host, port
-            combination will use the same socket options.
+            them if they are not correct.
 	    </p>
+	    <note>
+	      <p>
+		Persistent connections are not supported when setting the
+		<c>socket_opts</c> option. When <c>socket_opts</c> is not
+		set the current implementation assumes the requests to the
+		same host, port combination will use the same socket options.
+	      </p>
+	    </note>
 
             <p>By default the socket options set by function
 	    <seealso marker="#set_options-1">set_options/[1,2]</seealso> 
@@ -633,8 +638,11 @@
 	  to complete. The HTTP/1.1 specification suggests a
 	  limit of two persistent connections per server, which is the
 	  default value of option <c>max_sessions</c>.</p>
+	  <p>
+            The current implementation assumes the requests to the same host, port
+            combination will use the same socket options.
+	  </p>
 	</note>
-
         <marker id="get_options"></marker>
       </desc>
     </func>

diff --git a/lib/inets/src/http_client/httpc_manager.erl b/lib/inets/src/http_client/httpc_manager.erl
@@ -750,8 +750,26 @@ handle_request(#request{settings =
     start_handler(NewRequest#request{headers = NewHeaders}, State),
     {reply, {ok, NewRequest#request.id}, State};
 
-handle_request(Request, State = #state{options = Options}) ->
+%% Simple socket options handling (ERL-441).
+%%
+%% TODO: Refactor httpc to enable sending socket options in requests
+%%       using persistent connections. This workaround opens a new
+%%       connection for each request with non-empty socket_opts.
+handle_request(Request0 = #request{socket_opts = SocketOpts},
+               State0 = #state{options = Options0})
+  when is_list(SocketOpts) andalso length(SocketOpts) > 0 ->
+    Request = handle_cookies(generate_request_id(Request0), State0),
+    Options = convert_options(SocketOpts, Options0),
+    State = State0#state{options = Options},
+    Headers =
+	(Request#request.headers)#http_request_h{connection
+						    = "close"},
+    %% Reset socket_opts to avoid setopts failure.
+    start_handler(Request#request{headers = Headers, socket_opts = []}, State),
+    %% Do not change the state
+    {reply, {ok, Request#request.id}, State0};
 
+handle_request(Request, State = #state{options = Options}) ->
     NewRequest = handle_cookies(generate_request_id(Request), State),
     SessionType = session_type(Options),
     case select_session(Request#request.method,
@@ -775,6 +793,18 @@ handle_request(Request, State = #state{options = Options}) ->
     {reply, {ok, NewRequest#request.id}, State}.
 
 
+%% Convert Request options to State options
+convert_options([], Options) ->
+    Options;
+convert_options([{ipfamily, Value}|T], Options) ->
+    convert_options(T, Options#options{ipfamily = Value});
+convert_options([{ip, Value}|T], Options) ->
+    convert_options(T, Options#options{ip = Value});
+convert_options([{port, Value}|T], Options) ->
+    convert_options(T, Options#options{port = Value});
+convert_options([Option|T], Options = #options{socket_opts = SocketOpts}) ->
+    convert_options(T, Options#options{socket_opts = SocketOpts ++ [Option]}).
+
 start_handler(#request{id   = Id, 
 		       from = From} = Request, 
 	      #state{profile_name = ProfileName, 

diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
@@ -53,6 +53,7 @@ suite() ->
 all() ->
     [
      {group, http},
+     {group, http_ipv6},
      {group, sim_http},
      {group, http_internal},
      {group, http_unix_socket},
@@ -64,6 +65,10 @@ all() ->
 groups() ->
     [
      {http, [], real_requests()},
+     {http_ipv6, [], [request_options]},
+     %% process_leak_on_keepalive is depending on stream_fun_server_close
+     %% and it shall be the last test case in the suite otherwise cookie
+     %% will fail.
      {sim_http, [], only_simulated() ++ [process_leak_on_keepalive]},
      {http_internal, [], real_requests_esi()},
      {http_unix_socket, [], simulated_unix_socket()},
@@ -201,6 +206,16 @@ init_per_group(http_unix_socket = Group, Config0) ->
             Port = server_start(Group, server_config(Group, Config)),
             [{port, Port} | Config]
     end;
+init_per_group(http_ipv6 = Group, Config0) ->
+    case is_ipv6_supported() of
+        true ->
+            start_apps(Group),
+            Config = proplists:delete(port, Config0),
+            Port = server_start(Group, server_config(Group, Config)),
+            [{port, Port} | Config];
+        false ->
+            {skip, "Host does not support IPv6"}
+     end;
 init_per_group(Group, Config0) ->
     start_apps(Group),
     Config = proplists:delete(port, Config0),
@@ -250,6 +265,16 @@ end_per_testcase(persistent_connection, _Config) ->
 end_per_testcase(_Case, _Config) ->
     ok.
 
+is_ipv6_supported() ->
+    case gen_udp:open(0, [inet6]) of
+        {ok, Socket} ->
+            gen_udp:close(Socket),
+            true;
+        _ ->
+            false
+    end.
+
+
 %%--------------------------------------------------------------------
 %% Test Cases --------------------------------------------------------
 %%--------------------------------------------------------------------
@@ -1264,6 +1289,15 @@ unix_domain_socket(Config) when is_list(Config) ->
     {ok, {{_,200,_}, [_ | _], _}}
         = httpc:request(get, {URL, []}, [], []).
 
+%%--------------------------------------------------------------------
+request_options() ->
+    [{doc, "Test http get request with socket options against local server (IPv6)"}].
+request_options(Config) when is_list(Config) ->
+    Request  = {url(group_name(Config), "/dummy.html", Config), []},
+    {ok, {{_,200,_}, [_ | _], _ = [_ | _]}} = httpc:request(get, Request, [],
+                                                            [{socket_opts,[{ipfamily, inet6}]}]),
+    {error,{failed_connect,_ }} = httpc:request(get, Request, [], []).
+
 
 
 %%--------------------------------------------------------------------
@@ -1353,6 +1387,9 @@ url(http, End, Config) ->
     Port = proplists:get_value(port, Config),
     {ok,Host} = inet:gethostname(),
     ?URL_START ++ Host ++ ":" ++ integer_to_list(Port) ++ End;
+url(http_ipv6, End, Config) ->
+    Port = proplists:get_value(port, Config),
+    ?URL_START ++ "[::1]" ++ ":" ++ integer_to_list(Port) ++ End;
 url(https, End, Config) ->
     Port = proplists:get_value(port, Config),
     {ok,Host} = inet:gethostname(),
@@ -1397,7 +1434,11 @@ server_start(http_unix_socket, Config) ->
     {_Pid, Port} = http_test_lib:dummy_server(unix_socket, Inet, [{content_cb, ?MODULE},
                                                                   {unix_socket, Socket}]),
     Port;
-
+server_start(http_ipv6, HttpdConfig) ->
+    {ok, Pid} = inets:start(httpd, HttpdConfig),
+    Serv = inets:services_info(),
+    {value, {_, _, Info}} = lists:keysearch(Pid, 2, Serv),
+    proplists:get_value(port, Info);
 server_start(_, HttpdConfig) ->
     {ok, Pid} = inets:start(httpd, HttpdConfig),
     Serv = inets:services_info(),
@@ -1416,6 +1457,17 @@ server_config(http, Config) ->
      {mime_type, "text/plain"},
      {script_alias, {"/cgi-bin/", filename:join(ServerRoot, "cgi-bin") ++ "/"}}
     ];
+server_config(http_ipv6, Config) ->
+    ServerRoot = proplists:get_value(server_root, Config),
+    [{port, 0},
+     {server_name,"httpc_test"},
+     {server_root, ServerRoot},
+     {document_root, proplists:get_value(doc_root, Config)},
+     {bind_address, {0,0,0,0,0,0,0,1}},
+     {ipfamily, inet6},
+     {mime_type, "text/plain"},
+     {script_alias, {"/cgi-bin/", filename:join(ServerRoot, "cgi-bin") ++ "/"}}
+    ];
 server_config(http_internal, Config) ->
     ServerRoot = proplists:get_value(server_root, Config),
     [{port, 0},