ERL-1234: setting {packet,raw} after arriving http_eoh breaks gen_tcp:recv: {error, closed} is return

[OTP-Maintainer]

Original reporter: maxlapshin
Affected version: OTP-23.0-rc2
Fixed in version: OTP-23.0
Component: inet
Migrated from: https://bugs.erlang.org/browse/ERL-1234

---

# call gen_tcp:connect with binary,\{active,false},\{packet,http}
 # loop in gen_tcp:recv while not received http_eoh
 # then setopts  \{packet,raw}
 # call gen_tcp:recv  with exact size received from Content-Length
 # it must return \{ok, Bin}, but with  \{inet_backend, socket} it returns \{error, closed}

 

[https://gist.github.com/maxlapshin/cdf03656ac76431d9bede314f279b6b3]

 

Minimal example of error:

 

 
{code:java}
1> {ok, Socket} = gen_tcp:connect("demo.flussonic.com", 80, [binary, {active,false}, {packet,http},inet]),
1> ok = gen_tcp:send(Socket, "GET /crossdomain.xml HTTP/1.0\r\n\r\n"),
1> 
1> LoopHeaders = fun LoopHeaders(Len) ->
1>   case gen_tcp:recv(Socket, 0) of
1>     {ok, http_eoh} ->
1>       inet:setopts(Socket, [{packet,raw}]),
1>       Len;
1>     {ok, {http_header,_,'Content-Length',_, Len_}} ->
1>       LoopHeaders(list_to_integer(Len_));
1>     {ok, _} ->
1>       LoopHeaders(Len)
1>   end
1> end,
1> Length = LoopHeaders(undefined),
1> {ok, Bin} = gen_tcp:recv(Socket, Length),
1> io:format("~s\n", [Bin]),
1> gen_tcp:close(Socket).
<?xml version="1.0"?> 
<!DOCTYPE cross-domain-policy SYSTEM "http://www.adobe.com/xml/dtds/cross-domain-policy.dtd"> 
<cross-domain-policy>
  <allow-access-from domain="*" to-ports="*"/>
  <site-control permitted-cross-domain-policies="all"/>
  <allow-http-request-headers-from domain="*" headers="*" />
</cross-domain-policy>


ok
2> f().
ok
3> {ok, Socket} = gen_tcp:connect("demo.flussonic.com", 80, [{inet_backend,socket},binary, {active,false}, {packet,http},inet]),
3> ok = gen_tcp:send(Socket, "GET /crossdomain.xml HTTP/1.0\r\n\r\n"),
3> 
3> LoopHeaders = fun LoopHeaders(Len) ->
3>   case gen_tcp:recv(Socket, 0) of
3>     {ok, http_eoh} ->
3>       inet:setopts(Socket, [{packet,raw}]),
3>       Len;
3>     {ok, {http_header,_,'Content-Length',_, Len_}} ->
3>       LoopHeaders(list_to_integer(Len_));
3>     {ok, _} ->
3>       LoopHeaders(Len)
3>   end
3> end,
3> Length = LoopHeaders(undefined),
3> {ok, Bin} = gen_tcp:recv(Socket, Length),
3> io:format("~s\n", [Bin]),
3> gen_tcp:close(Socket).
** exception error: no match of right hand side value {error,closed}
4> 
{code}
 

 

[OTP-Maintainer]

raimo said:

Thank you for a making it easy to reproduce!

The code simply ditched buffered data when receiving a specified number of bytes.  This diff should fix it.  I will try to squeeze it into OTP-23.0.
{code:erlang}
@@ -1227,15 +1227,15 @@ handle_event(
   {call, From}, {recv, Length, Timeout}, State, {P, D}) ->
     case State of
         'connected' ->
-            handle_recv(
-              P, recv_start(D, From, Length),
-              [{{timeout, recv}, Timeout, recv}]);
+            handle_recv_start(P, D, From, Length, Timeout);
         #recv{} ->
             %% Receive in progress
             {keep_state_and_data,
              [postpone]}
     end;
 
 %% State: #recv{}
 %%
 %% Handle select done - try recv again
@@ -1352,6 +1352,33 @@ handle_connected(P, D, ActionsR) ->
             handle_recv(P, recv_start(D), ActionsR)
     end.
 
+handle_recv_start(
+  P, #{packet := Packet, buffer := Buffer} = D, From, Length, Timeout)
+  when Packet =:= raw, 0 < Length;
+       Packet =:= 0, 0 < Length ->
+    Size = iolist_size(Buffer),
+    if
+        Length =< Size ->
+            {Data, NewBuffer} =
+                split_binary(condense_buffer(Buffer), Length),
+            handle_recv_deliver(
+              P,
+              D#{recv_length => Length, % Redundant
+                 recv_from => From,
+                 buffer := NewBuffer},
+              [], Data);
+        true ->
+            N = Length - Size,
+            handle_recv(
+              P, D#{recv_length => N, recv_from => From},
+              [{{timeout, recv}, Timeout, recv}])
+    end;
+handle_recv_start(P, D, From, _Length, Timeout) ->
+    handle_recv(
+      P, D#{recv_length => 0, recv_from => From},
+      [{{timeout, recv}, Timeout, recv}]).
+
 handle_recv(P, #{packet := Packet, recv_length := Length} = D, ActionsR) ->
     if
         0 < Length ->
@@ -1661,16 +1688,6 @@ cleanup_recv_reply(
      end}.
 
 %% Initialize packet recv state
-recv_start(#{packet := Packet} = D, From, Length) ->
-    %%
-    D#{recv_length =>
-           case Packet of
-               raw -> Length;
-               0 -> Length;
-               _ -> 0
-           end,
-       recv_from => From}.
-
 recv_start(D) ->
     D#{recv_length => 0}.

{code}

[OTP-Maintainer]

raimo said:

That was incomplete.  I wrote a test case and found out this was missing:
{code:erlang}
@@ -1713,6 +1713,10 @@ recv_data_deliver(
              [{reply, From, {ok, DeliverData}},
               {{timeout, recv}, cancel}
               | ActionsR]};
+        #{active := false} ->
+            D_1 = D#{buffer := unrecv_buffer(Data, maps:get(buffer, D))},
+            {recv_stop(next_packet(D_1, Packet, Data)),
+             ActionsR};
         #{active := Active} ->
             ModuleSocket = module_socket(P),
             Owner !
@@ -1781,6 +1785,16 @@ catbin(Bin, <<>>) when is_binary(Bin) -> Bin;
 catbin(Bin1, Bin2) when is_binary(Bin1), is_binary(Bin2) ->
     <<Bin1/binary, Bin2/binary>>.
 
+unrecv_buffer(Data, Buffer) ->
+    case Buffer of
+        <<>> ->
+            Data;
+        _ when is_binary(Buffer) ->
+            [Data, Buffer];
+        _ ->
+            [Data | Buffer]
+    end.
+
 condense_buffer([Bin]) when is_binary(Bin) -> Bin;
 condense_buffer(Buffer) ->
     iolist_to_binary(reverse_improper(Buffer, [])).
{code}

[OTP-Maintainer]

raimo said:

Fixed in 23.0