[erlang-questions] Any tcp related changes in R15B* that might cause this?
Sverker Eriksson
sverker.eriksson@REDACTED
Tue Jan 8 19:46:43 CET 2013
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>
>>>>
>>>>
>
>
More information about the erlang-questions
mailing list