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

Sverker Eriksson sverker.eriksson@REDACTED
Mon Jan 7 17:26:09 CET 2013


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