tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: [VOTE] Release Apache Tomcat 7.0.9
Date Mon, 28 Feb 2011 20:06:06 GMT
On 2/28/2011 12:00 PM, Mark Thomas wrote:
> On 28/02/2011 18:45, Filip Hanik - Dev Lists wrote:
>> On 2/28/2011 11:30 AM, Mark Thomas wrote:
>>> On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:
>>>> [X] Broken - do not release
>>>>
>>>> http://svn.apache.org/viewvc?rev=1075030&view=rev
>>>> http://svn.apache.org/viewvc?rev=1074675&view=rev
>>>>
>>>> Are not completely correct implementations as they don't
>>>> - support a timeout correctly
>>> That is an issue. Should be a relatively easy fix.
>> I got this in my code base,
> Could you commit it then?
>
>> I still recommend pulling out the change
>> until all issues are taken care of.
> We can easily comment out the rehandshake() call if the issues aren't
> fixed by the time we decide to do the next release.
>
>>>> - dont account for when a write is needed on the selector
>>> I can't figure out what circumstances the handshake process would return
>>> OP_WRITE since it shouldn't ever be waiting for some other process to
>>> write data. Could you explain how this might happen?
>> When you want to initiate a renegotiating from the server, you want to
>> write data to the client notifying it of such. This happens when you
>> call beginHandshake.
>> beginHandshake generates the bytes needed to be sent to the client, so
>> you must send them.
>> So we make an attempt to send them. If the attempt to send them fails,
>> meaning no bytes were accepted into the buffer, the handshake(...) calls
>> returns OP_WRITE to let you know that you can be signaled for when this
>> needs to take place.
>> This is most unlikely to happen in a scenario like this, as there TCP
>> buffers should have plenty of space in them and we should be able to
>> take the bytes. However, from a state machine perspective, you still
>> need to account for the state.
> So what is the right way to respond to an OP_WRITE? Select and wait for
> the signal and then call handshakeWrap()?
you don't call handshakeWrap or unwrap, the handshake(boolean,boolean) does everything for
you. You simply call select() with OP_WRITE, and 
when it returns from that, you call handshake(...) again, and it automagically does the state
machine for you
>> The other mistake, is that there is an assumed OP_READ, even if the
>> handshake completes.
> I don't see what you mean here. Could you clarify please.
handshake(true, true);
297 	if (handshakeStatus == HandshakeStatus.NEED_UNWRAP) {
298 	// Block until there is data to read from the client
299 	Selector selector = null;


again, this is ignoring the state machine. What you need to do here, is check the results
of handshake(true,true), not if NEED_UNWRAP is there.

it should be (in pseudo code):
loop:
   int status = handshake(...);
   switch (status): {
     case 0: handshake-completed; return;
     case -1: something went wrong; throw exception;
     default: select.select(status);
end loop




>> The change will require a bit more careful analysis to get right. Not to
>> mention, this code, as you may have noticed, is fragile.
>>
>>>> - they potentially garble a message body larger than the limit of the
>>>> input buffer
>>> This should be handled by the existing code although I didn't test it.
>>> Should be easy to write a unit test to confirm what happens in this case.
>> Try to start sending a 25MB file upload together with your headers.
> Does it need to be that big? I was planning on writing a unit test that
> POSTed 20 bytes to a resource that required CLIENT-CERT and then testing
> with an input buffer of 100 bytes, 20 bytes and 5 bytes (or similar
> numbers).
It would have to be bigger than maxPostSize for the data to remain in the buffers

Filip

> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 10.0.1204 / Virus Database: 1435/3473 - Release Date: 02/28/11
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message