[erlang-questions] Any tcp related changes in R15B* that might cause this?
Anthony Molinaro
anthonym@REDACTED
Tue Jan 8 17:42:31 CET 2013
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?
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>
More information about the erlang-questions
mailing list