You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rainer Jung <ra...@kippdata.de> on 2018/05/11 07:58:21 UTC
Re: svn commit: r1830547 - in /tomcat/trunk:
java/org/apache/coyote/Response.java
java/org/apache/coyote/http11/Http11InputBuffer.java
webapps/docs/changelog.xml
Am 30.04.2018 um 12:57 schrieb markt@apache.org:
> Author: markt
> Date: Mon Apr 30 10:57:27 2018
> New Revision: 1830547
>
> URL: http://svn.apache.org/viewvc?rev=1830547&view=rev
> Log:
> Correct a regression in the error page handling that prevented error pages from issuing redirects or taking other action that required the response status code to be changed.
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/Response.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
> tomcat/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/trunk/java/org/apache/coyote/Response.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Response.java?rev=1830547&r1=1830546&r2=1830547&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/Response.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/Response.java Mon Apr 30 10:57:27 2018
> @@ -227,10 +227,6 @@ public final class Response {
> * @param status The status value to set
> */
> public void setStatus(int status) {
> - if (this.status > 399) {
> - // Don't overwrite first recorded error status
> - return;
> - }
> this.status = status;
> }
This part of the patch makes TestHttp11InputBuffer fail for me, more
precisely
Testcase: testNewLinesExcessive took 0.12 sec
FAILED
HTTP/1.1 505
junit.framework.AssertionFailedError: HTTP/1.1 505
at
org.apache.coyote.http11.TestHttp11InputBuffer.testNewLinesExcessive(TestHttp11InputBuffer.java:385)
The test expects status 400, but gets 505. The 505 is being set in the
following stack:
at org.apache.coyote.Response.setStatus(Response.java:231)
at
org.apache.coyote.http11.Http11Processor.prepareRequest(Http11Processor.java:588)
at
org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:388)
at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:754)
...
In prepareRequest(), the String protocolMB contains the text "null".
I don't know whether we just need to fix the test expectation, or tested
code.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1830547 - in /tomcat/trunk:
java/org/apache/coyote/Response.java
java/org/apache/coyote/http11/Http11InputBuffer.java
webapps/docs/changelog.xml
Posted by Rainer Jung <ra...@kippdata.de>.
Am 11.05.2018 um 10:31 schrieb Mark Thomas:
> On 11/05/18 08:58, Rainer Jung wrote:
>> Am 30.04.2018 um 12:57 schrieb markt@apache.org:
>>> Author: markt
>>> Date: Mon Apr 30 10:57:27 2018
>>> New Revision: 1830547
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1830547&view=rev
>>> Log:
>>> Correct a regression in the error page handling that prevented error
>>> pages from issuing redirects or taking other action that required the
>>> response status code to be changed.
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/coyote/Response.java
>>> tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
>>> tomcat/trunk/webapps/docs/changelog.xml
>>>
>>> Modified: tomcat/trunk/java/org/apache/coyote/Response.java
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Response.java?rev=1830547&r1=1830546&r2=1830547&view=diff
>>>
>>> ==============================================================================
>>>
>>> --- tomcat/trunk/java/org/apache/coyote/Response.java (original)
>>> +++ tomcat/trunk/java/org/apache/coyote/Response.java Mon Apr 30
>>> 10:57:27 2018
>>> @@ -227,10 +227,6 @@ public final class Response {
>>> * @param status The status value to set
>>> */
>>> public void setStatus(int status) {
>>> - if (this.status > 399) {
>>> - // Don't overwrite first recorded error status
>>> - return;
>>> - }
>>> this.status = status;
>>> }
>>
>> This part of the patch makes TestHttp11InputBuffer fail for me, more
>> precisely
>>
>> Testcase: testNewLinesExcessive took 0.12 sec
>> FAILED
>> HTTP/1.1 505
>> junit.framework.AssertionFailedError: HTTP/1.1 505
>> at
>> org.apache.coyote.http11.TestHttp11InputBuffer.testNewLinesExcessive(TestHttp11InputBuffer.java:385)
>
> I can't repeat this but looking at the test is does look as if it might
> depend on network behaviour.
>
>> The test expects status 400, but gets 505. The 505 is being set in the
>> following stack:
>>
>> at org.apache.coyote.Response.setStatus(Response.java:231)
>> at
>> org.apache.coyote.http11.Http11Processor.prepareRequest(Http11Processor.java:588)
>
> But I can put a break point here and catch this happening.
>
>> at
>> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:388)
>> at
>> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
>>
>> at
>> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:754)
>>
>> ...
>>
>> In prepareRequest(), the String protocolMB contains the text "null".
>>
>> I don't know whether we just need to fix the test expectation, or tested
>> code.
>
> I think something along the lines of the r1830547 changes for
> Http11InputBuffer should do the trick. I should be able to commit
> something shortly.
Thanks Mark, r1831389 fixed it for me.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1830547 - in /tomcat/trunk:
java/org/apache/coyote/Response.java
java/org/apache/coyote/http11/Http11InputBuffer.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 11/05/18 08:58, Rainer Jung wrote:
> Am 30.04.2018 um 12:57 schrieb markt@apache.org:
>> Author: markt
>> Date: Mon Apr 30 10:57:27 2018
>> New Revision: 1830547
>>
>> URL: http://svn.apache.org/viewvc?rev=1830547&view=rev
>> Log:
>> Correct a regression in the error page handling that prevented error
>> pages from issuing redirects or taking other action that required the
>> response status code to be changed.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/Response.java
>> tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/java/org/apache/coyote/Response.java
>> URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Response.java?rev=1830547&r1=1830546&r2=1830547&view=diff
>>
>> ==============================================================================
>>
>> --- tomcat/trunk/java/org/apache/coyote/Response.java (original)
>> +++ tomcat/trunk/java/org/apache/coyote/Response.java Mon Apr 30
>> 10:57:27 2018
>> @@ -227,10 +227,6 @@ public final class Response {
>> * @param status The status value to set
>> */
>> public void setStatus(int status) {
>> - if (this.status > 399) {
>> - // Don't overwrite first recorded error status
>> - return;
>> - }
>> this.status = status;
>> }
>
> This part of the patch makes TestHttp11InputBuffer fail for me, more
> precisely
>
> Testcase: testNewLinesExcessive took 0.12 sec
> FAILED
> HTTP/1.1 505
> junit.framework.AssertionFailedError: HTTP/1.1 505
> at
> org.apache.coyote.http11.TestHttp11InputBuffer.testNewLinesExcessive(TestHttp11InputBuffer.java:385)
I can't repeat this but looking at the test is does look as if it might
depend on network behaviour.
> The test expects status 400, but gets 505. The 505 is being set in the
> following stack:
>
> at org.apache.coyote.Response.setStatus(Response.java:231)
> at
> org.apache.coyote.http11.Http11Processor.prepareRequest(Http11Processor.java:588)
But I can put a break point here and catch this happening.
> at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:388)
> at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
>
> at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:754)
>
> ...
>
> In prepareRequest(), the String protocolMB contains the text "null".
>
> I don't know whether we just need to fix the test expectation, or tested
> code.
I think something along the lines of the r1830547 changes for
Http11InputBuffer should do the trick. I should be able to commit
something shortly.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org