You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/08/26 15:32:45 UTC
svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/
test/webapp/jsp/ webapps/docs/
Author: markt
Date: Tue Aug 26 13:32:45 2014
New Revision: 1620596
URL: http://svn.apache.org/r1620596
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
Enable any HTTP method to be used to request a JSP page that has the isErrorPage page directive set to true.
Added:
tomcat/trunk/test/webapp/jsp/error.jsp (with props)
Modified:
tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1620596&r1=1620595&r2=1620596&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Aug 26 13:32:45 2014
@@ -255,6 +255,11 @@ public abstract class Compiler {
// to be GC'd and save memory.
ctxt.setWriter(null);
+ // Need to know if the JSP is an error page at runtime to determine
+ // which HTTP methods are permitted. Error pages permit any. Normal
+ // pages only permit GET, POST or HEAD.
+ jsw.setErrorPage(pageInfo.isErrorPage());
+
if (log.isDebugEnabled()) {
t4 = System.currentTimeMillis();
log.debug("Generated " + javaFileName + " total=" + (t4 - t1)
Modified: tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java?rev=1620596&r1=1620595&r2=1620596&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java (original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java Tue Aug 26 13:32:45 2014
@@ -25,7 +25,6 @@ import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
-import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
@@ -282,19 +281,6 @@ public class JspServlet extends HttpServ
HttpServletResponse response)
throws ServletException, IOException {
- String method = request.getMethod();
-
- if (!"GET".equals(method) && !"POST".equals(method) && !"HEAD".equals(method) &&
- !DispatcherType.ERROR.equals(request.getDispatcherType())) {
- // Specification states behaviour is undefined
- // Jasper opts to reject any other verbs, partly as they are
- // unlikely to make sense in a JSP context and partly to protect
- // against verb tampering
- response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED,
- Localizer.getMessage("jsp.error.servlet.invalid.method"));
- return;
- }
-
//jspFile may be configured as an init-param for this servlet instance
String jspUri = jspFile;
Modified: tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java?rev=1620596&r1=1620595&r2=1620596&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java (original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java Tue Aug 26 13:32:45 2014
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
+import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
@@ -103,6 +104,7 @@ public class JspServletWrapper {
private final boolean unloadAllowed;
private final boolean unloadByCount;
private final boolean unloadByIdle;
+ private boolean errorPage;
/*
* JspServletWrapper for JSP pages.
@@ -399,7 +401,6 @@ public class JspServletWrapper {
}
try {
-
/*
* (3) Handle limitation of number of loaded Jsps
*/
@@ -419,6 +420,21 @@ public class JspServletWrapper {
}
}
}
+
+ String method = request.getMethod();
+
+ if (!"GET".equals(method) && !"POST".equals(method) && !"HEAD".equals(method) &&
+ !DispatcherType.ERROR.equals(request.getDispatcherType()) &&
+ !isErrorPage()) {
+ // Specification states behaviour is undefined
+ // Jasper opts to reject any other verbs, partly as they are
+ // unlikely to make sense in a JSP context and partly to protect
+ // against verb tampering
+ response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED,
+ Localizer.getMessage("jsp.error.servlet.invalid.method"));
+ return;
+ }
+
/*
* (4) Service request
*/
@@ -586,4 +602,13 @@ public class JspServletWrapper {
}
}
+
+ public void setErrorPage(boolean errorPage) {
+ this.errorPage = errorPage;
+ }
+
+
+ public boolean isErrorPage() {
+ return errorPage;
+ }
}
Modified: tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java?rev=1620596&r1=1620595&r2=1620596&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java (original)
+++ tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java Tue Aug 26 13:32:45 2014
@@ -36,7 +36,7 @@ import org.apache.tomcat.util.descriptor
public class TestJspServlet extends TomcatBaseTest {
@Test
- public void testBug56568() throws Exception {
+ public void testBug56568a() throws Exception {
Tomcat tomcat = getTomcatInstance();
// Use the test web application so JSP support is available and the
@@ -45,7 +45,7 @@ public class TestJspServlet extends Tom
Context context = tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
// Create a servlet that always throws an exception for a PUT request
- Tomcat.addServlet(context, "Bug56568Servlet", new Bug56568Servlet());
+ Tomcat.addServlet(context, "Bug56568Servlet", new Bug56568aServlet());
context.addServletMapping("/bug56568", "Bug56568Servlet");
// Configure a JSP page to handle the 500 error response
@@ -67,7 +67,27 @@ public class TestJspServlet extends Tom
Assert.assertEquals(500, rc);
}
- private static class Bug56568Servlet extends HttpServlet {
+ @Test
+ public void testBug56568b() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ // Use the test web application so JSP support is available and the
+ // default JSP error page can be used.
+ File appDir = new File("test/webapp");
+ tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
+
+ tomcat.start();
+
+ int rc = methodUrl("http://localhost:" + getPort() + "/test/jsp/error.jsp",
+ new ByteChunk(), 500000, null, null, "PUT");
+
+ // Make sure we get a 200 response and not a 405 response
+ // which would indicate that error.jsp is complaining about being called
+ // with the PUT method.
+ Assert.assertEquals(200, rc);
+ }
+
+ private static class Bug56568aServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
Added: tomcat/trunk/test/webapp/jsp/error.jsp
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/jsp/error.jsp?rev=1620596&view=auto
==============================================================================
--- tomcat/trunk/test/webapp/jsp/error.jsp (added)
+++ tomcat/trunk/test/webapp/jsp/error.jsp Tue Aug 26 13:32:45 2014
@@ -0,0 +1,22 @@
+<%--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+--%>
+<%@page session="false" isErrorPage="true" %>
+<html>
+ <body>
+ <p>ERROR</p>
+ </body>
+</html>
\ No newline at end of file
Propchange: tomcat/trunk/test/webapp/jsp/error.jsp
------------------------------------------------------------------------------
svn:eol-style = native
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1620596&r1=1620595&r2=1620596&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug 26 13:32:45 2014
@@ -79,6 +79,11 @@
functional impact but the code was less efficient as a result of the
error. Based on a patch by martinschaef. (markt)
</fix>
+ <fix>
+ <bug>56568</bug>: Enable any HTTP method to be used to request a JSP
+ page that has the <code>isErrorPage</code> page directive set to
+ <code>true</code>. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/
test/webapp/jsp/ webapps/docs/
Posted by Rémy Maucherat <re...@apache.org>.
2014-08-27 11:06 GMT+02:00 Mark Thomas <ma...@apache.org>:
> I've started to ignore the CI failures due to the false positive rate.
> That is probably a sign some time needs to be spent looking at the CI
> failures and fixing them.
>
> I'm not a fan of gump and am mostly looking at CI instead for testsuite
runs. However, there are no false positives at all there right now since
the poor thing seems dead. Problem solved !
http://ci.apache.org/builders/tomcat-trunk
Rémy
Re: svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/ test/webapp/jsp/
webapps/docs/
Posted by Mark Thomas <ma...@apache.org>.
On 27/08/2014 13:34, Konstantin Kolinko wrote:
> 2014-08-27 13:06 GMT+04:00 Mark Thomas <ma...@apache.org>:
>> On 26/08/2014 23:16, Konstantin Kolinko wrote:
>>> 2014-08-26 17:32 GMT+04:00 <ma...@apache.org>:
>>>> Author: markt
>>>> Date: Tue Aug 26 13:32:45 2014
>>>> New Revision: 1620596
>>>>
>>>> URL: http://svn.apache.org/r1620596
>>>> Log:
>>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
>>>> Enable any HTTP method to be used to request a JSP page that has the isErrorPage page directive set to true.
>>>>
>>>> Added:
>>>> tomcat/trunk/test/webapp/jsp/error.jsp (with props)
>>>> Modified:
>>>> tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>>>> tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
>>>> tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
>>>> tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
>>>> tomcat/trunk/webapps/docs/changelog.xml
>>>> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>>>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1620596&r1=1620595&r2=1620596&view=diff
>>>> ==============================================================================
>>>> --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
>>>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Aug 26 13:32:45 2014
>>>> @@ -255,6 +255,11 @@ public abstract class Compiler {
>>>> // to be GC'd and save memory.
>>>> ctxt.setWriter(null);
>>>>
>>>> + // Need to know if the JSP is an error page at runtime to determine
>>>> + // which HTTP methods are permitted. Error pages permit any. Normal
>>>> + // pages only permit GET, POST or HEAD.
>>>> + jsw.setErrorPage(pageInfo.isErrorPage());
>>>> +
>>>
>>> Apparently this causes org.apache.jasper.TestJspC to fail with many NPEs,
>>> as noted by Gump in tomcat-trunk-test-nio:
>>>
>>> [junit] java.lang.NullPointerException
>>> [junit] at org.apache.jasper.compiler.Compiler.generateJava(Compiler.java:261)
>>> [junit] at org.apache.jasper.compiler.Compiler.compile(Compiler.java:361)
>>> [junit] at org.apache.jasper.JspC.processFile(JspC.java:1217)
>>>
>>> All other test cases completed successfully.
>>
>> Thanks. Fixed.
>
> I think that method is a wrong place to configure a JspServletWrapper.
I think you are right. I was trying fail early but I think the check is
going to need to be lower - probably in the generated code.
> The generateJava() method generates java text for a JSP page. It will
> be skipped if there is no need to recompile the page.
> E.g. if web application was reloaded and loads a previously compiled class.
That is one failure mode.
> I also wonder how this feature works with precompiled JSPs that are
> decalred as <servlet>s in web.xml.
I suspect that is another.
I'll take another look.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/
test/webapp/jsp/ webapps/docs/
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-08-27 13:06 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 26/08/2014 23:16, Konstantin Kolinko wrote:
>> 2014-08-26 17:32 GMT+04:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Tue Aug 26 13:32:45 2014
>>> New Revision: 1620596
>>>
>>> URL: http://svn.apache.org/r1620596
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
>>> Enable any HTTP method to be used to request a JSP page that has the isErrorPage page directive set to true.
>>>
>>> Added:
>>> tomcat/trunk/test/webapp/jsp/error.jsp (with props)
>>> Modified:
>>> tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>>> tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
>>> tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
>>> tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
>>> tomcat/trunk/webapps/docs/changelog.xml
>>> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1620596&r1=1620595&r2=1620596&view=diff
>>> ==============================================================================
>>> --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
>>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Aug 26 13:32:45 2014
>>> @@ -255,6 +255,11 @@ public abstract class Compiler {
>>> // to be GC'd and save memory.
>>> ctxt.setWriter(null);
>>>
>>> + // Need to know if the JSP is an error page at runtime to determine
>>> + // which HTTP methods are permitted. Error pages permit any. Normal
>>> + // pages only permit GET, POST or HEAD.
>>> + jsw.setErrorPage(pageInfo.isErrorPage());
>>> +
>>
>> Apparently this causes org.apache.jasper.TestJspC to fail with many NPEs,
>> as noted by Gump in tomcat-trunk-test-nio:
>>
>> [junit] java.lang.NullPointerException
>> [junit] at org.apache.jasper.compiler.Compiler.generateJava(Compiler.java:261)
>> [junit] at org.apache.jasper.compiler.Compiler.compile(Compiler.java:361)
>> [junit] at org.apache.jasper.JspC.processFile(JspC.java:1217)
>>
>> All other test cases completed successfully.
>
> Thanks. Fixed.
I think that method is a wrong place to configure a JspServletWrapper.
The generateJava() method generates java text for a JSP page. It will
be skipped if there is no need to recompile the page.
E.g. if web application was reloaded and loads a previously compiled class.
I also wonder how this feature works with precompiled JSPs that are
decalred as <servlet>s in web.xml.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/ test/webapp/jsp/
webapps/docs/
Posted by Mark Thomas <ma...@apache.org>.
On 26/08/2014 23:16, Konstantin Kolinko wrote:
> 2014-08-26 17:32 GMT+04:00 <ma...@apache.org>:
>> Author: markt
>> Date: Tue Aug 26 13:32:45 2014
>> New Revision: 1620596
>>
>> URL: http://svn.apache.org/r1620596
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
>> Enable any HTTP method to be used to request a JSP page that has the isErrorPage page directive set to true.
>>
>> Added:
>> tomcat/trunk/test/webapp/jsp/error.jsp (with props)
>> Modified:
>> tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>> tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
>> tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
>> tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
>> tomcat/trunk/webapps/docs/changelog.xml
>> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1620596&r1=1620595&r2=1620596&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Aug 26 13:32:45 2014
>> @@ -255,6 +255,11 @@ public abstract class Compiler {
>> // to be GC'd and save memory.
>> ctxt.setWriter(null);
>>
>> + // Need to know if the JSP is an error page at runtime to determine
>> + // which HTTP methods are permitted. Error pages permit any. Normal
>> + // pages only permit GET, POST or HEAD.
>> + jsw.setErrorPage(pageInfo.isErrorPage());
>> +
>
> Apparently this causes org.apache.jasper.TestJspC to fail with many NPEs,
> as noted by Gump in tomcat-trunk-test-nio:
>
> [junit] java.lang.NullPointerException
> [junit] at org.apache.jasper.compiler.Compiler.generateJava(Compiler.java:261)
> [junit] at org.apache.jasper.compiler.Compiler.compile(Compiler.java:361)
> [junit] at org.apache.jasper.JspC.processFile(JspC.java:1217)
>
> All other test cases completed successfully.
Thanks. Fixed.
I've started to ignore the CI failures due to the false positive rate.
That is probably a sign some time needs to be spent looking at the CI
failures and fixing them.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1620596 - in /tomcat/trunk: java/org/apache/jasper/compiler/
java/org/apache/jasper/servlet/ test/org/apache/jasper/servlet/
test/webapp/jsp/ webapps/docs/
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-08-26 17:32 GMT+04:00 <ma...@apache.org>:
> Author: markt
> Date: Tue Aug 26 13:32:45 2014
> New Revision: 1620596
>
> URL: http://svn.apache.org/r1620596
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56568
> Enable any HTTP method to be used to request a JSP page that has the isErrorPage page directive set to true.
>
> Added:
> tomcat/trunk/test/webapp/jsp/error.jsp (with props)
> Modified:
> tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
> tomcat/trunk/java/org/apache/jasper/servlet/JspServlet.java
> tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java
> tomcat/trunk/test/org/apache/jasper/servlet/TestJspServlet.java
> tomcat/trunk/webapps/docs/changelog.xml
> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1620596&r1=1620595&r2=1620596&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
> +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Aug 26 13:32:45 2014
> @@ -255,6 +255,11 @@ public abstract class Compiler {
> // to be GC'd and save memory.
> ctxt.setWriter(null);
>
> + // Need to know if the JSP is an error page at runtime to determine
> + // which HTTP methods are permitted. Error pages permit any. Normal
> + // pages only permit GET, POST or HEAD.
> + jsw.setErrorPage(pageInfo.isErrorPage());
> +
Apparently this causes org.apache.jasper.TestJspC to fail with many NPEs,
as noted by Gump in tomcat-trunk-test-nio:
[junit] java.lang.NullPointerException
[junit] at org.apache.jasper.compiler.Compiler.generateJava(Compiler.java:261)
[junit] at org.apache.jasper.compiler.Compiler.compile(Compiler.java:361)
[junit] at org.apache.jasper.JspC.processFile(JspC.java:1217)
All other test cases completed successfully.
> if (log.isDebugEnabled()) {
> t4 = System.currentTimeMillis();
> log.debug("Generated " + javaFileName + " total=" + (t4 - t1)
>
(....)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org