[erlang-questions] Any tcp related changes in R15B* that might cause this?

Sverker Eriksson sverker.eriksson@REDACTED
Thu Feb 21 19:30:31 CET 2013 

This patch for {exit_on_close,false} will be included in R16B.

/Sverker, Erlang/OTP

Sverker Eriksson wrote:
> Sorry for the late answer, been kind of busy with R16A.
>
> I think you have found a bug. I did this modification in your test
>
> ok = inet:setopts (Sock, [{packet, http}, {exit_on_close,false}]),
>
> and it still close the socket.
>
> Here is a patch that seems to solve the problem:
>
> git fetch git://github.com/sverker/otp.git sverk/tcp-exit_on_close-false
>
> I'm not 100% sure that this fix doesn't has some other unexpected 
> effect. The inet driver is quite a complex piece of software.
> It would be good if you could test this out.
> The patch is based on R15B02 but it should be easy to apply on both 
> R15B03 and R16A.
>
> /Sverker, Erlang/OTP
>
>
> Anthony Molinaro wrote:
>> 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>
>>>>>>>
>>
>
> _______________________________________________
> erlang-questions mailing list
> erlang-questions@REDACTED
> http://erlang.org/mailman/listinfo/erlang-questions
>

More information about the erlang-questions mailing list