Merge branch 'sverk/packet_size-http/OTP-9389'

* sverk/packet_size-http/OTP-9389:
  erts: Remove truncation of http packet parsing and return error instead
  honor packet_size for http packet parsing to fix OTP-9389

7 files changed, 180 insertions, 24 deletions

diff --git a/erts/doc/src/erlang.xml b/erts/doc/src/erlang.xml
index eac7db5eaa..a603d5c2b8 100644
--- a/erts/doc/src/erlang.xml
+++ b/erts/doc/src/erlang.xml
@@ -724,9 +724,12 @@ false</pre>
                 size limit.</p>
               </item>
               <tag><c>{line_length, integer()}</c></tag>
-              <item><p>Applies only to line oriented protocols
-                (<c>line</c>, <c>http</c>). Lines longer than this
-                will be truncated.</p>
+              <item><p>For packet type <c>line</c>, truncate lines longer
+	      than the indicated length.</p>
+	      <p>Option <c>line_length</c> also applies to <c>http*</c> 
+	      packet types as an alias for option <c>packet_size</c> in the
+	      case when <c>packet_size</c> itself is not set. This usage is
+	      only intended for backward compatibility.</p>
               </item>
             </taglist>
         <pre>
diff --git a/erts/emulator/beam/packet_parser.c b/erts/emulator/beam/packet_parser.c
index a66d60aa22..4d4b6ea196 100644
--- a/erts/emulator/beam/packet_parser.c
+++ b/erts/emulator/beam/packet_parser.c
@@ -301,7 +301,11 @@ int packet_get_length(enum PacketParseType htype,
         /* TCP_PB_LINE_LF:  [Data ... \n]  */
         const char* ptr2;
         if ((ptr2 = memchr(ptr, '\n', n)) == NULL) {
-            if (n >= trunc_len && trunc_len!=0) { /* buffer full */
+            if (n > max_plen && max_plen != 0) { /* packet full */
+                DEBUGF((" => packet full (no NL)=%d\r\n", n));
+                goto error;
+            }
+            else if (n >= trunc_len && trunc_len!=0) { /* buffer full */
                 DEBUGF((" => line buffer full (no NL)=%d\r\n", n));
                 return trunc_len;
             }
@@ -309,6 +313,10 @@ int packet_get_length(enum PacketParseType htype,
         }
         else {
             int len = (ptr2 - ptr) + 1; /* including newline */
+            if (len > max_plen && max_plen!=0) {
+                DEBUGF((" => packet_size %d exceeded\r\n", max_plen));
+                goto error;
+            }
             if (len > trunc_len && trunc_len!=0) {
                 DEBUGF((" => truncated line=%d\r\n", trunc_len));
                 return trunc_len;
@@ -397,33 +405,50 @@ int packet_get_length(enum PacketParseType htype,
             const char* ptr1 = ptr;
             int   len = plen;
             
+	    if (!max_plen) {
+		/* This is for backward compatibility with old user of decode_packet
+		 * that might use option 'line_length' to limit accepted length of
+		 * http lines.
+		 */
+		max_plen = trunc_len;
+	    }
+
             while (1) {
                 const char* ptr2 = memchr(ptr1, '\n', len);
                 
                 if (ptr2 == NULL) {
-                    if (n >= trunc_len && trunc_len!=0) { /* buffer full */
-                        plen = trunc_len;
-                        goto done;
+                    if (max_plen != 0) {
+                        if (n >= max_plen) /* packet full */
+                            goto error;
                     }
                     goto more;
                 }
                 else {
                     plen = (ptr2 - ptr) + 1;
-                    
-                    if (*statep == 0)
+
+                    if (*statep == 0) {
+                        if (max_plen != 0 && plen > max_plen)
+                            goto error;
                         goto done;
-                    
+                    }
+
                     if (plen < n) {
                         if (SP(ptr2+1) && plen>2) {
                             /* header field value continue on next line */
                             ptr1 = ptr2+1;
                             len = n - plen;
                         }
-                        else
+                        else {
+                            if (max_plen != 0 && plen > max_plen)
+                                goto error;
                             goto done;
+                        }
                     }
-                    else
+                    else {
+                        if (max_plen != 0 && plen > max_plen)
+                            goto error;
                         goto more;
+                    }
                 }
             }
         }
diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c
index e0d869f328..45089dcc2f 100644
--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -8729,8 +8729,15 @@ static int tcp_remain(tcp_descriptor* desc, int* len)
     else if (tlen == 0) { /* need unknown more */
         *len = 0;
         if (nsz == 0) {
-            if (nfill == n)
-                goto error;
+            if (nfill == n) {
+                if (desc->inet.psize != 0 && desc->inet.psize > nfill) {
+                    if (tcp_expand_buffer(desc, desc->inet.psize) < 0)
+                        return -1;
+                    return desc->inet.psize;
+                }
+                else
+                    goto error;
+            }
             DEBUGF((" => restart more=%d\r\n", nfill - n));
             return nfill - n;
         }
diff --git a/erts/emulator/test/decode_packet_SUITE.erl b/erts/emulator/test/decode_packet_SUITE.erl
index c0499554eb..4acbe8c6e0 100644
--- a/erts/emulator/test/decode_packet_SUITE.erl
+++ b/erts/emulator/test/decode_packet_SUITE.erl
@@ -26,12 +26,14 @@
 -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, 
 	 init_per_group/2,end_per_group/2,
 	 init_per_testcase/2,end_per_testcase/2,
-	 basic/1, packet_size/1, neg/1, http/1, line/1, ssl/1, otp_8536/1]).
+	 basic/1, packet_size/1, neg/1, http/1, line/1, ssl/1, otp_8536/1,
+         otp_9389/1, otp_9389_line/1]).
 
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
-    [basic, packet_size, neg, http, line, ssl, otp_8536].
+    [basic, packet_size, neg, http, line, ssl, otp_8536,
+     otp_9389, otp_9389_line].
 
 groups() -> 
     [].
@@ -251,6 +253,28 @@ packet_size(Config) when is_list(Config) ->
 			  ?line {error,_} = decode_pkt(4,<<Size:32,Packet/binary>>)
 		  end, 
 		  lists:seq(-10,-1)),
+
+    %% Test OTP-9389, long HTTP header lines.
+    Opts = [{packet_size, 128}],
+    Pkt = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /",
+                          string:chars($Y, 64), "\r\n\r\n"]),
+    <<Pkt1:50/binary, Pkt2/binary>> = Pkt,
+    ?line {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest1} =
+        erlang:decode_packet(http, Pkt1, Opts),
+    ?line {ok, {http_header,_,'Host',_,"localhost"}, Rest2} =
+        erlang:decode_packet(httph, Rest1, Opts),
+    ?line {more, undefined} = erlang:decode_packet(httph, Rest2, Opts),
+    ?line {ok, {http_header,_,"Link",_,_}, _} =
+        erlang:decode_packet(httph, list_to_binary([Rest2, Pkt2]), Opts),
+
+    Pkt3 = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /",
+                           string:chars($Y, 129), "\r\n\r\n"]),
+    ?line {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest3} =
+        erlang:decode_packet(http, Pkt3, Opts),
+    ?line {ok, {http_header,_,'Host',_,"localhost"}, Rest4} =
+        erlang:decode_packet(httph, Rest3, Opts),
+    ?line {error, invalid} = erlang:decode_packet(httph, Rest4, Opts),
+
     ok.
 
 
@@ -557,3 +581,35 @@ decode_pkt(Type,Bin,Opts) ->
     %%io:format(" -> ~p\n",[Res]),
     Res.
 
+otp_9389(doc) -> ["Verify line_length works correctly for HTTP headers"];
+otp_9389(suite) -> [];
+otp_9389(Config) when is_list(Config) ->
+    Opts = [{packet_size, 16384}, {line_length, 3000}],
+    Pkt = list_to_binary(["GET / HTTP/1.1\r\nHost: localhost\r\nLink: /",
+                          string:chars($X, 8192),
+                          "\r\nContent-Length: 0\r\n\r\n"]),
+    <<Pkt1:5000/binary, Pkt2/binary>> = Pkt,
+    {ok, {http_request,'GET',{abs_path,"/"},{1,1}}, Rest1} =
+        erlang:decode_packet(http, Pkt1, Opts),
+    {ok, {http_header,_,'Host',_,"localhost"}, Rest2} =
+        erlang:decode_packet(httph, Rest1, Opts),
+    {more, undefined} = erlang:decode_packet(httph, Rest2, Opts),
+    {ok, {http_header,_,"Link",_,Link}, Rest3} =
+        erlang:decode_packet(httph, list_to_binary([Rest2, Pkt2]), Opts),
+    true = (length(Link) > 8000),
+    {ok, {http_header,_,'Content-Length',_,"0"}, <<"\r\n">>} =
+        erlang:decode_packet(httph, Rest3, Opts),
+    ok.
+
+otp_9389_line(doc) -> ["Verify packet_size works correctly for line mode"];
+otp_9389_line(suite) -> [];
+otp_9389_line(Config) when is_list(Config) ->
+    Opts = [{packet_size, 20}],
+    Line1 = <<"0123456789012345678\n">>,
+    Line2 = <<"0123456789\n">>,
+    Line3 = <<"01234567890123456789\n">>,
+    Pkt = list_to_binary([Line1, Line2, Line3]),
+    ?line {ok, Line1, Rest1} = erlang:decode_packet(line, Pkt, Opts),
+    ?line {ok, Line2, Rest2} = erlang:decode_packet(line, Rest1, Opts),
+    ?line {error, invalid} = erlang:decode_packet(line, Rest2, Opts),
+    ok.
diff --git a/lib/kernel/doc/src/inet.xml b/lib/kernel/doc/src/inet.xml
index fad5af85bb..1a05b4ba99 100644
--- a/lib/kernel/doc/src/inet.xml
+++ b/lib/kernel/doc/src/inet.xml
@@ -573,6 +573,10 @@ fe80::204:acff:fe17:bf38
               is longer than the max allowed length, the packet is
               considered invalid. The same happens if the packet header
               is too big for the socket receive buffer.</p>
+	    <p>For line oriented protocols (<c>line</c>,<c>http*</c>),
+	    option <c>packet_size</c> also guarantees that lines up to the
+	    indicated length are accepted and not considered invalid due
+	    to internal buffer limitations.</p>
           </item>
           <tag><c>{read_packets, Integer}</c>(UDP sockets)</tag>
           <item>
diff --git a/lib/kernel/test/gen_tcp_echo_SUITE.erl b/lib/kernel/test/gen_tcp_echo_SUITE.erl
index fffaaf4c45..5bbaeb02ad 100644
--- a/lib/kernel/test/gen_tcp_echo_SUITE.erl
+++ b/lib/kernel/test/gen_tcp_echo_SUITE.erl
@@ -167,8 +167,12 @@ echo_test_1(SockOpts, EchoFun, Config0) ->
 		      [{type, {cdr, little}}|Config]),
     ?line case lists:keymember(packet_size, 1, SockOpts) of
 	      false ->
-		  ?line echo_packet([{packet, line}|SockOpts],
-				    EchoFun, Config);
+		  % This is cheating, we should test that packet_size
+		  % also works for line and http.
+		  echo_packet([{packet, line}|SockOpts], EchoFun, Config),
+		  echo_packet([{packet, http}|SockOpts], EchoFun, Config),
+		  echo_packet([{packet, http_bin}|SockOpts], EchoFun, Config);
+
 	      true -> ok
 	  end,
     ?line echo_packet([{packet, tpkt}|SockOpts], EchoFun, Config),
@@ -183,9 +187,6 @@ echo_test_1(SockOpts, EchoFun, Config0) ->
 		      [{type, {asn1, short, LongTag}}|Config]),
     ?line echo_packet([{packet, asn1}|SockOpts], EchoFun,
 		      [{type, {asn1, long, LongTag}}|Config]),
-
-    ?line echo_packet([{packet, http}|SockOpts], EchoFun, Config),
-    ?line echo_packet([{packet, http_bin}|SockOpts], EchoFun, Config),
     ok.
 
 echo_packet(SockOpts, EchoFun, Opts) ->
diff --git a/lib/kernel/test/gen_tcp_misc_SUITE.erl b/lib/kernel/test/gen_tcp_misc_SUITE.erl
index b1ef8826d5..c1c5ff8b81 100644
--- a/lib/kernel/test/gen_tcp_misc_SUITE.erl
+++ b/lib/kernel/test/gen_tcp_misc_SUITE.erl
@@ -40,7 +40,8 @@
 	 accept_timeouts_in_order3/1,accept_timeouts_mixed/1, 
 	 killing_acceptor/1,killing_multi_acceptors/1,killing_multi_acceptors2/1,
 	 several_accepts_in_one_go/1,active_once_closed/1, send_timeout/1, send_timeout_active/1, 
-	 otp_7731/1, zombie_sockets/1, otp_7816/1, otp_8102/1]).
+	 otp_7731/1, zombie_sockets/1, otp_7816/1, otp_8102/1,
+         otp_9389/1]).
 
 %% Internal exports.
 -export([sender/3, not_owner/1, passive_sockets_server/2, priority_server/1, 
@@ -72,7 +73,7 @@ all() ->
      killing_acceptor, killing_multi_acceptors,
      killing_multi_acceptors2, several_accepts_in_one_go,
      active_once_closed, send_timeout, send_timeout_active, otp_7731,
-     zombie_sockets, otp_7816, otp_8102].
+     zombie_sockets, otp_7816, otp_8102, otp_9389].
 
 groups() -> 
     [].
@@ -2479,4 +2480,63 @@ otp_8102_do(LSocket, PortNum, {Bin,PType}) ->
     io:format("Got error msg, ok.\n",[]),
     gen_tcp:close(SSocket),    
     gen_tcp:close(RSocket).
-    
+
+otp_9389(doc) -> ["Verify packet_size handles long HTTP header lines"];
+otp_9389(suite) -> [];
+otp_9389(Config) when is_list(Config) ->
+    ?line {ok, LS} = gen_tcp:listen(0, []),
+    ?line {ok, {_, PortNum}} = inet:sockname(LS),
+    io:format("Listening on ~w with port number ~p\n", [LS, PortNum]),
+    OrigLinkHdr = "/" ++ string:chars($S, 8192),
+    _Server = spawn_link(
+                fun() ->
+                        ?line {ok, S} = gen_tcp:accept(LS),
+                        ?line ok = inet:setopts(S, [{packet_size, 16384}]),
+                        ?line ok = otp_9389_loop(S, OrigLinkHdr),
+                        ?line ok = gen_tcp:close(S)
+                end),
+    ?line {ok, S} = gen_tcp:connect("localhost", PortNum,
+                                    [binary, {active, false}]),
+    Req = "GET / HTTP/1.1\r\n"
+        ++ "Host: localhost\r\n"
+        ++ "Link: " ++ OrigLinkHdr ++ "\r\n\r\n",
+    ?line ok = gen_tcp:send(S, Req),
+    ?line ok = inet:setopts(S, [{packet, http}]),
+    ?line {ok, {http_response, {1,1}, 200, "OK"}} = gen_tcp:recv(S, 0),
+    ?line ok = inet:setopts(S, [{packet, httph}, {packet_size, 16384}]),
+    ?line {ok, {http_header, _, 'Content-Length', _, "0"}} = gen_tcp:recv(S, 0),
+    ?line {ok, {http_header, _, "Link", _, LinkHdr}} = gen_tcp:recv(S, 0),
+    ?line true = (LinkHdr == OrigLinkHdr),
+    ok = gen_tcp:close(S),
+    ok = gen_tcp:close(LS),
+    ok.
+
+otp_9389_loop(S, OrigLinkHdr) ->
+    ?line ok = inet:setopts(S, [{active,once},{packet,http}]),
+    receive
+        {http, S, {http_request, 'GET', _, _}} ->
+            ?line ok = otp_9389_loop(S, OrigLinkHdr, undefined)
+    after
+        3000 ->
+            ?line error({timeout,request_line})
+    end.
+otp_9389_loop(S, OrigLinkHdr, ok) ->
+    ?line Resp = "HTTP/1.1 200 OK\r\nContent-length: 0\r\n" ++
+        "Link: " ++ OrigLinkHdr ++ "\r\n\r\n",
+    ?line ok = gen_tcp:send(S, Resp);
+otp_9389_loop(S, OrigLinkHdr, State) ->
+    ?line ok = inet:setopts(S, [{active,once}, {packet,httph}]),
+    receive
+        {http, S, http_eoh} ->
+            ?line otp_9389_loop(S, OrigLinkHdr, ok);
+        {http, S, {http_header, _, "Link", _, LinkHdr}} ->
+            ?line LinkHdr = OrigLinkHdr,
+            ?line otp_9389_loop(S, OrigLinkHdr, State);
+        {http, S, {http_header, _, _Hdr, _, _Val}} ->
+            ?line otp_9389_loop(S, OrigLinkHdr, State);
+        {http, S, {http_error, Err}} ->
+            ?line error({error, Err})
+    after
+        3000 ->
+            ?line error({timeout,header})
+    end.