aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSverker Eriksson <[email protected]>2011-12-07 14:20:55 +0100
committerSverker Eriksson <[email protected]>2011-12-07 14:20:55 +0100
commitf7241e210c333b088210a138c0880aa93d1f7780 (patch)
treea8c4620af57b908468881864b48866f5c71500c5
parent2e5639155e153ba4869d7671d1a7becd5e07dc1a (diff)
parentdc5f7190f16cf4552db74fba3f4e0f2d654e2594 (diff)
downloadotp-f7241e210c333b088210a138c0880aa93d1f7780.tar.gz
otp-f7241e210c333b088210a138c0880aa93d1f7780.tar.bz2
otp-f7241e210c333b088210a138c0880aa93d1f7780.zip
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
-rw-r--r--erts/doc/src/erlang.xml9
-rw-r--r--erts/emulator/beam/packet_parser.c43
-rw-r--r--erts/emulator/drivers/common/inet_drv.c11
-rw-r--r--erts/emulator/test/decode_packet_SUITE.erl60
-rw-r--r--lib/kernel/doc/src/inet.xml4
-rw-r--r--lib/kernel/test/gen_tcp_echo_SUITE.erl11
-rw-r--r--lib/kernel/test/gen_tcp_misc_SUITE.erl66
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.