[erlang-questions] Any tcp related changes in R15B* that might cause this?
Anthony Molinaro
anthonym@REDACTED
Tue Jan 15 07:40:28 CET 2013
Hi Sverker,
I cannot get {exit_on_close, false}, to actually change the behavior
of my test. I set it as an options to gen_tcp:listen/2 but gen_tcp:send/2
still returns {error,closed}. I also tried setting it at various other
points with inet:setopts/2, but it never seems to make a difference.
I'm using R15B02, but I'm wondering if you can modify my example, such
that the gen_tcp:send/2 doesn't fail with {error,closed}.
I reattached it without any modifications. You should be able to
just compile it and run
gen_tcp_test:test(10000).
To see it fail.
Hopefully you can show me the error of my ways ;)
-Anthony
On Tue, Jan 08, 2013 at 07:46:43PM +0100, Sverker Eriksson wrote:
> Anthony Molinaro wrote:
> >I agree that the new behavior of returning an emsgsize tcp_error is
> >probably correct, however the new behavior also closes the socket.
> >
> >Before your changes mochiweb and my test could return a 400 error so
> >the client would get a response even if they were sending in a bad
> >request. After the change the client just gets a remote side closed
> >connection message.
> >
> >So I guess the question is, why is the connection being closed? Is
> >that also something the other packet protocols do?
> >
> Yes all packets protocols act the same in this sense.
>
> You can use socket option {exit_on_close, false} to only do a
> half-close and still allow writing to the socket.
>
> /Sverker
>
> >Thanks for looking at it,
> >
> >-Anthony
> >
> >On Mon, Jan 07, 2013 at 05:26:09PM +0100, Sverker Eriksson wrote:
> >>Thanks for the clarification, guys.
> >>I've look at dc5f7190 and it seems like the change in error
> >>handling was more or less intentional (by me).
> >>
> >>Old behavior was
> >>1. Truncate a too long http line and return it as if it was ok.
> >>2. Continue parsing rest of the line as if it were a new http line
> >>which probably (and hopefully) results in a http_error.
> >>
> >>I didn't like that; as it returned a partially parsed line as if it
> >>was ok, reported the error too late and confusing and also (I guess)
> >>constituted a security bug that could be abused to trick the http
> >>parsing with long lines.
> >>
> >>So I changed it to reject too long lines and return tcp_error, which
> >>is what happens for all packet protocols when the length of the
> >>packet is in some way invalid.
> >>
> >>The caller has to handle tcp_error's anyway, so it's a case of bad
> >>fault identification.
> >>I guess it would be nicer to return {http_error, "Line too long:
> >>X-Randomaaaaaaaaaaaaaaaaaa..."}
> >>or something like that.
> >>I don't have time to fix that now, but don't think it would be too
> >>hard do if anyone is eager...
> >>
> >>
> >>/Sverker, Erlang/OTP
> >>
> >>
> >>
> >>Steve Vinoski wrote:
> >>>Same results with this test: works on commits prior to dc5f7190, fails
> >>>on dc5f7190.
> >>>
> >>>--steve
> >>>
> >>>On Thu, Jan 3, 2013 at 1:16 AM, Anthony Molinaro <
> >>>anthonym@REDACTED> wrote:
> >>>
> >>>>Attached is a test which shows the difference using only gen_tcp calls.
> >>>>
> >>>>Here's the output from both
> >>>>
> >>>>Erlang R14B04 (erts-5.8.5) [source] [64-bit] [smp:2:2] [rq:2]
> >>>>[async-threads:0]
> >>>>[hipe] [kernel-poll:false]
> >>>>
> >>>>Eshell V5.8.5 (abort with ^G)
> >>>>1> gen_tcp_test:test(1000).
> >>>>gen_tcp:send ok
> >>>>{{"HTTP/1.1",200,"OK"},
> >>>>[{"date","Wed, 02 Jan 2013 21:01:08 GMT"},
> >>>> {"server","Test"},
> >>>> {"content-length","31"},
> >>>> {"content-type","text/html"}],
> >>>>"<html><body>Hello</body></html>"}
> >>>>2> gen_tcp_test:test(10000).
> >>>>http_error - R14
> >>>>gen_tcp:send ok
> >>>>{{"HTTP/1.1",400,"Bad Request"},
> >>>>[{"date","Wed, 02 Jan 2013 21:03:24 GMT"},
> >>>> {"server","Test"},
> >>>> {"content-length","0"}],
> >>>> []}
> >>>>3>
> >>>>
> >>>>and for R15
> >>>>
> >>>>Erlang R15B02 (erts-5.9.2) [source] [64-bit] [smp:2:2] [async-threads:0]
> >>>>[hipe]
> >>>>[kernel-poll:false]
> >>>>
> >>>>Eshell V5.9.2 (abort with ^G)
> >>>>1> gen_tcp_test:test(1000).
> >>>>gen_tcp:send ok
> >>>>{{"HTTP/1.1",200,"OK"},
> >>>>[{"date","Wed, 02 Jan 2013 21:01:08 GMT"},
> >>>> {"server","Test"},
> >>>> {"content-length","31"},
> >>>> {"content-type","text/html"}],
> >>>>"<html><body>Hello</body></html>"}
> >>>>2> gen_tcp_test:test(10000).
> >>>>emsgsize - R15
> >>>>gen_tcp:send failed with {error,closed}
> >>>>socket_closed_remotely
> >>>>3>
> >>>>
> >>>>Hope it helps,
> >>>>
> >>>>-Anthony
> >>>>
> >>>>On Wed, Jan 02, 2013 at 12:34:56PM -0500, Steve Vinoski wrote:
> >>>>>Hi Anthony and Sverker, as I mentioned in a previous email, using
> >>>>Anthony's
> >>>>>test case a few weeks ago I bisected between R14B04 and HEAD and found
> >>>>>commit dc5f7190 to be the first commit where the test case fails.
> >>>>>
> >>>>>--steve
> >>>>>
> >>>>>On Wed, Jan 2, 2013 at 12:28 PM, Anthony Molinaro <
> >>>>>anthonym@REDACTED> wrote:
> >>>>>
> >>>>>>Hi,
> >>>>>>
> >>>>>> The holidays went and interrupted everything, so I'm sorry about the
> >>>>>>late response.
> >>>>>>It's unclear if the commit dc5f7190 did it or not, but sometime between
> >>>>>>R14B04 and R15B02 the behavior did change. I've not isolated it
> >>>>outside
> >>>>>>of mochiweb, but I don't think its a mochiweb problem.
> >>>>>>
> >>>>>>Here's the pull request which contains the short program I used to
> >>>>>>reproduce with mochiweb (and work around the new behavior)
> >>>>>>
> >>>>>>https://github.com/mochi/mochiweb/pull/91
> >>>>>>
> >>>>>>I'll see if I can't isolate it to just OTP sometime in the next
> >>>>>>couple of days.
> >>>>>>
> >>>>>>-Anthony
> >>>>>>
> >>>>>>On Mon, Dec 17, 2012 at 04:10:55PM +0100, Sverker Eriksson wrote:
> >>>>>>>Have I understood it correctly so far if I say that my commit
> >>>>>>>dc5f7190 seems to cause a
> >>>>>>>
> >>>>>>>{tcp_error,Port,emsgsize}
> >>>>>>>
> >>>>>>>while earlier versions caused a
> >>>>>>>
> >>>>>>>{http,Port,{http_error,"..."}}
> >>>>>>>
> >>>>>>>
> >>>>>>>for the same scenario with some sort of very long http lines?
> >>>>>>>
> >>>>>>>/Sverker, Erlang/OTP
> >>>>>>>
> >>>>>>>Anthony Molinaro wrote:
> >>>>>>>>On Fri, Dec 14, 2012 at 11:32:11AM -0500, Steve Vinoski wrote:
> >>>>>>>>>Hi Anthony, based on your example, it looks like a follow-up commit
> >>>>>>that
> >>>>>>>>>modified some of the changes introduced by my patch introduced the
> >>>>>>issue
> >>>>>>>>>you're seeing: commit dc5f7190. You might want to report it over on
> >>>>>>>>>erlang-bugs.
> >>>>>>>>Nice catch, I guess that it's actually not enough to return error
> >>>>>>>>for backward compatibility ;)
> >>>>>>>>
> >>>>>>>>I think the main thing I'm still not sure of is if the decode_packet
> >>>>>>changes
> >>>>>>>>are actually the cause. I was able to call decode_packet with long
> >>>>>>headers
> >>>>>>>>and not actually see an issue.
> >>>>>>>>
> >>>>>>>>This
> >>>>>>>>
> >>>>>>>>erlang:decode_packet ( httph,
> >>>>>>>> list_to_binary ([ "X-Random: ",
> >>>>>>>> [$a || _ <-
> >>>>>>lists:seq(1,10000)],
> >>>>>>>> "\r\n\r\n"
> >>>>>>>> ]),
> >>>>>>>> []).
> >>>>>>>>
> >>>>>>>>Returns the same thing in both R14B04 and R15B02. And as far as I
> >>>>am
> >>>>>>able
> >>>>>>>>to tell that is the only call made to decode_packet in mochiweb.
> >>>>>>>>
> >>>>>>>>I think the actual error is around recvbuf sizing (mochiweb uses
> >>>>8192 as
> >>>>>>>>the default, so 10000 is definitely larger than that, however, the
> >>>>>>standard
> >>>>>>>>libraries must be doing something different as now it returns
> >>>>emsgsize
> >>>>>>>>where it did no in the past). I really wish there was a way to
> >>>>search
> >>>>>>>>commits on github as then I could search for recvbuf or something
> >>>>like
> >>>>>>>>that to see what might have changed.
> >>>>>>>>
> >>>>>>>>Before I go over to bugs I'll probably try to detangle mochiweb
> >>>>enough
> >>>>>>>>to create a small reproducible test case.
> >>>>>>>>
> >>>>>>>>I also have a pull request into mochiweb which prevents the crash
> >>>>but
> >>>>>>>>has the annoying behavior at the moment that it returns an error to
> >>>>the
> >>>>>>>>client about disconnecting version the 400 error it returned before.
> >>>>>>>>
> >>>>>>>>https://github.com/mochi/mochiweb/pull/91
> >>>>>>>>
> >>>>>>>>But since the standard library seems to be the one closing the
> >>>>socket
> >>>>>>>>there's not much that can be done other than the fix I put in place.
> >>>>>>>>
> >>>>>>>>Thanks for your help Steve,
> >>>>>>>>
> >>>>>>>>-Anthony
> >>>>>>>>
> >>>>>>>_______________________________________________
> >>>>>>>erlang-questions mailing list
> >>>>>>>erlang-questions@REDACTED
> >>>>>>>http://erlang.org/mailman/listinfo/erlang-questions
> >>>>>>--
> >>>>>>
> >>>>------------------------------------------------------------------------
> >>>>>>Anthony Molinaro <
> >>>>anthonym@REDACTED>
> >>>>>>_______________________________________________
> >>>>>>erlang-questions mailing list
> >>>>>>erlang-questions@REDACTED
> >>>>>>http://erlang.org/mailman/listinfo/erlang-questions
> >>>>>>
> >>>>--
> >>>>------------------------------------------------------------------------
> >>>>Anthony Molinaro <anthonym@REDACTED>
> >>>>
> >
>
--
------------------------------------------------------------------------
Anthony Molinaro <anthonym@REDACTED>
-------------- next part --------------
-module (gen_tcp_test).
-export ([start/0,
start_one/0,
test/1]).
start () ->
application:start (inets),
spawn (gen_tcp_test, start_one, []).
start_one () ->
{ok, LSock} = gen_tcp:listen (5678, [ binary,
{reuseaddr,true},
{packet,0},
{active,false}
]),
{ok, Sock} = gen_tcp:accept (LSock),
ok = inet:setopts (Sock, [{packet, http}]),
Resp = req (Sock),
case gen_tcp:send (Sock, Resp) of
ok -> io:format ("gen_tcp:send ok~n");
E -> io:format ("gen_tcp:send failed with ~p~n",[E])
end,
close (Sock).
close (Socket) ->
gen_tcp:close (Socket),
exit (normal).
req (Socket) ->
ok = inet:setopts (Socket, [{active, once}]),
receive
{_, _, {http_request, _, _, _}} ->
ok = inet:setopts(Socket, [{packet, httph}]),
headers (Socket)
end.
headers (Socket) ->
ok = inet:setopts(Socket, [{active, once}]),
receive
{_, _, http_eoh} ->
ok = inet:setopts(Socket, [{packet, raw}]),
[<<"HTTP/1.1 ">>, ["200",[" ",79,75]], <<"\r\n">>,
"Server",<<": ">>, "Test",<<"\r\n">>,
"Date",<<": ">>,"Wed, 02 Jan 2013 21:01:08 GMT",<<"\r\n">>,
"Content-Type",<<": ">>,"text/html",<<"\r\n">>,
"Content-Length",<<": ">>,"31",<<"\r\n">>,<<"\r\n">>,
"<html><body>Hello</body></html>"];
{_,_, {http_header, _, _, _, _}} ->
headers (Socket);
{_,_, {http_error, _}} ->
io:format("http_error - R14~n"),
invalid (Socket);
{tcp_error, _, emsgsize} ->
io:format("emsgsize - R15~n"),
invalid (Socket)
end.
invalid (_Socket) ->
[ <<"HTTP/1.1 ">>,
[ "400",[" ",66,97,100,32,82,101,113,117,101,115,116]],<<"\r\n">>,
"Server",<<": ">>, "Test",<<"\r\n">>,
"Date",<<": ">>,"Wed, 02 Jan 2013 21:03:24 GMT",<<"\r\n">>,
"Content-Length",<<": ">>,"0",<<"\r\n">>,<<"\r\n">>
].
test (Len) ->
start (),
case httpc:request (get,
{ "http://127.0.0.1:5678/",
[{"X-Random", [$a || _ <- lists:seq(1,Len)]}]
},
[],
[]) of
{ok, Resp} -> Resp;
{error, Err} -> Err
end.
More information about the erlang-questions
mailing list