Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Received-SPF: pass (athena.apache.org: local policy)
Message-ID: <48DBA4EB.3040203@kippdata.de>
Date: Thu, 25 Sep 2008 16:49:15 +0200
From: Rainer Jung <rainer.jung@kippdata.de>
User-Agent: Thunderbird 2.0.0.6 (X11/20070802)
MIME-Version: 1.0
To: Tomcat Developers List <dev@tomcat.apache.org>
Subject: Re: svn commit: r698960 - /tomcat/tc6.0.x/trunk/STATUS.txt
References: <20080925132115.C6F1223889FC@eris.apache.org>
In-Reply-To: <20080925132115.C6F1223889FC@eris.apache.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

>  * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45026
>    Never use empty reason phrase.
>    http://svn.apache.org/viewvc?rev=697183&view=rev
>    +1: rjung, mturk, markt
> -  -1: 
> +  -1: remm (I think HttpMessages.getMessage should return something rather than null, 
> +            most likely something like sc.ZZZ like you are doing, otherwise you need to fix
> +            the APR implementation as well; 
> +            if I understand correctly, trouble will occur with AJP if status is something 467,
> +            with no message set)

So you propose to move the fix simply to HttpMessages.getMessage, which 
should return with status code as string, whenever the StringManager 
doesn't find a reason phrase. Correct?

>  * Allow huge request body packets for AJP13.
>    This was already applied to connectors, but never
> @@ -167,32 +171,34 @@
>    http://svn.apache.org/viewvc?rev=697192&view=rev
>    Original change: http://svn.apache.org/viewvc?rev=486217&view=rev
>    +1: rjung, mturk, markt
> -  -1: 
> +  -1: remm (- bodyMsg.appendInt(AjpConstants.MAX_READ_SIZE + packetSize - AjpConstants.MAX_PACKET_SIZE); looks wrong

Before the change it was simply MAX_READ_SIZE. After the change, if our 
actual configured maximum packet size (A=packetSize) is bigger or 
smaller than the default one (B=MAX_PACKET_SIZE) we adjust the read size 
accordingly by the delta A-B.

Phrased diffferentyl:

    MAX_READ_SIZE = MAX_PACKET_SIZE - H_SIZE - 2;

So

    MAX_READ_SIZE + packetSize - MAX_PACKET_SIZE =
    MAX_PACKET_SIZE - H_SIZE - 2 + packetSize - MAX_PACKET_SIZE =
                    packetSize -H_SIZE - 2

> +            - also partially applies to the two other AJP connectors

I'll look for that.

> +            - not sure why forcing AjpConstants.MAX_PACKET_SIZE; either this shouldn't be done or the constant name should change)

The constant MAX_PACKET_SIZE is equal to the previous value of 8192. If 
one wants to use another size, there is another constructor, which 
inlcudes the size and sets it correctly. This one here is deprecated, 
but for me the reason why the 8192 was there, was that it's the usual 
AJP packet size. Since we already had a constant for that, I replaced it.

Yes the name of the constant doesn't reflect it's use now in all places. 
It is the DEFAULT_MAX_PACKET_SIZE. At the moment there seems to be no 
maximum enforced (with the patch), but there is one (implementation 
dependant) on the client (native) side, and if you use non-default 
values, you need to keep them in sync on the two sides.

Do you think we should rename MAX_PACKET_SIZE to DEFAULT_MAX_PACKET_SIZE?

Regards,

Rainer


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