You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2014/08/25 15:47:57 UTC

svn commit: r1620326 - in /tomcat/trunk: java/org/apache/catalina/mapper/ test/org/apache/catalina/mapper/ webapps/docs/ webapps/examples/jsp/forward/ webapps/examples/jsp/include/

Author: kkolinko
Date: Mon Aug 25 13:47:56 2014
New Revision: 1620326

URL: http://svn.apache.org/r1620326
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56882
Fix regression in processing of includes and forwards when Context have been reloaded.
Tomcat was responding with HTTP Status 503.

This includes test cases.
TestMapper:
Amend context mapper tests
- Check wrapper names
- Test mapper map() method used by include and forward mapping
- Add test for Context reload (BZ 56882)

TestMapperWebapps:
- Add test for Context reload (BZ 56882)

Modified:
    tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java
    tomcat/trunk/test/org/apache/catalina/mapper/TestMapper.java
    tomcat/trunk/test/org/apache/catalina/mapper/TestMapperWebapps.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/examples/jsp/forward/one.jsp
    tomcat/trunk/webapps/examples/jsp/forward/two.html
    tomcat/trunk/webapps/examples/jsp/include/foo.jsp
    tomcat/trunk/webapps/examples/jsp/include/include.jsp

Modified: tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java Mon Aug 25 13:47:56 2014
@@ -305,6 +305,7 @@ public final class Mapper {
                     int pos = find(contextVersions, version);
                     if (pos >= 0 && contextVersions[pos].name.equals(version)) {
                         contextVersions[pos] = newContextVersion;
+                        contextObjectToContextVersionMap.put(context, newContextVersion);
                     }
                 }
             }

Modified: tomcat/trunk/test/org/apache/catalina/mapper/TestMapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/mapper/TestMapper.java?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/mapper/TestMapper.java (original)
+++ tomcat/trunk/test/org/apache/catalina/mapper/TestMapper.java Mon Aug 25 13:47:56 2014
@@ -23,6 +23,7 @@ import java.util.concurrent.atomic.Atomi
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
@@ -103,6 +104,10 @@ public class TestMapper extends LoggingB
         mapper.addContextVersion("iowejoiejfoiew", host, "/foo/bar",
                 "0", createContext("context2"), welcomes, null, null);
 
+        mapper.addWrappers("iowejoiejfoiew", "/foo", "0", Arrays
+                .asList(new WrapperMappingInfo[] {
+                        new WrapperMappingInfo("/",
+                                createWrapper("context1-defaultWrapper"), false, false) }));
         mapper.addWrappers("iowejoiejfoiew", "/foo/bar", "0", Arrays
                 .asList(new WrapperMappingInfo[] {
                         new WrapperMappingInfo("/fo/*",
@@ -301,11 +306,19 @@ public class TestMapper extends LoggingB
         assertEquals("0", mappedContext.versions[0].name);
         Host oldHost = mappedHost.object;
         Context oldContext = mappedContext.versions[0].object;
+        assertEquals("context2", oldContext.getName());
+
+        Context oldContext1 = mappedHost.contextList.contexts[contextPos - 1].versions[0].object;
+        assertEquals("context1", oldContext1.getName());
 
         mappingData.recycle();
         mapper.map(hostMB, uriMB, null, mappingData);
         assertEquals("blah7", mappingData.host.getName());
         assertEquals("context2", mappingData.context.getName());
+        assertEquals("wrapper5", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(oldContext, uriMB, mappingData);
+        assertEquals("wrapper5", mappingData.wrapper.getName());
 
         Context newContext = createContext("newContext");
         mapper.addContextVersion(
@@ -317,7 +330,7 @@ public class TestMapper extends LoggingB
                 null,
                 null,
                 Arrays.asList(new WrapperMappingInfo[] { new WrapperMappingInfo(
-                        "/", createWrapper("default"), false, false) }));
+                        "/", createWrapper("newContext-default"), false, false) }));
 
         assertEquals(2, mappedContext.versions.length);
         assertEquals("0", mappedContext.versions[0].name);
@@ -325,6 +338,10 @@ public class TestMapper extends LoggingB
         mappingData.recycle();
         mapper.map(hostMB, uriMB, null, mappingData);
         assertEquals("newContext", mappingData.context.getName());
+        assertEquals("newContext-default", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(newContext, uriMB, mappingData);
+        assertEquals("newContext-default", mappingData.wrapper.getName());
 
         mapper.removeContextVersion(oldContext, hostName, contextPath, "0");
 
@@ -333,6 +350,10 @@ public class TestMapper extends LoggingB
         mappingData.recycle();
         mapper.map(hostMB, uriMB, null, mappingData);
         assertEquals("newContext", mappingData.context.getName());
+        assertEquals("newContext-default", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(newContext, uriMB, mappingData);
+        assertEquals("newContext-default", mappingData.wrapper.getName());
 
         mapper.removeContextVersion(oldContext, hostName, contextPath, "1");
 
@@ -341,6 +362,10 @@ public class TestMapper extends LoggingB
         mappingData.recycle();
         mapper.map(hostMB, uriMB, null, mappingData);
         assertEquals("context1", mappingData.context.getName());
+        assertEquals("context1-defaultWrapper", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(oldContext1, uriMB, mappingData);
+        assertEquals("context1-defaultWrapper", mappingData.wrapper.getName());
 
         mapper.addContextVersion(
                 hostName,
@@ -351,7 +376,7 @@ public class TestMapper extends LoggingB
                 null,
                 null,
                 Arrays.asList(new WrapperMappingInfo[] { new WrapperMappingInfo(
-                        "/", createWrapper("default"), false, false) }));
+                        "/", createWrapper("newContext-defaultWrapper2"), false, false) }));
         mappedContext = mappedHost.contextList.contexts[contextPos];
 
         assertEquals(contextPath, mappedContext.name);
@@ -360,6 +385,89 @@ public class TestMapper extends LoggingB
         mappingData.recycle();
         mapper.map(hostMB, uriMB, null, mappingData);
         assertEquals("newContext", mappingData.context.getName());
+        assertEquals("newContext-defaultWrapper2", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(newContext, uriMB, mappingData);
+        assertEquals("newContext-defaultWrapper2", mappingData.wrapper.getName());
+    }
+
+    @Test
+    public void testReloadContextVersion() throws Exception {
+        final String hostName = "iowejoiejfoiew";
+        final int iowPos = 3;
+        final String contextPath = "/foo/bar";
+        final int contextPos = 2;
+
+        MappingData mappingData = new MappingData();
+        MessageBytes hostMB = MessageBytes.newInstance();
+        MessageBytes uriMB = MessageBytes.newInstance();
+        hostMB.setString(hostName);
+        uriMB.setString("/foo/bar/blah/bobou/foo");
+
+        // Verifying configuration created by setUp()
+        Mapper.MappedHost mappedHost = mapper.hosts[iowPos];
+        assertEquals(hostName, mappedHost.name);
+        Mapper.MappedContext mappedContext = mappedHost.contextList.contexts[contextPos];
+        assertEquals(contextPath, mappedContext.name);
+        assertEquals(1, mappedContext.versions.length);
+        assertEquals("0", mappedContext.versions[0].name);
+        Host oldHost = mappedHost.object;
+        Context oldContext = mappedContext.versions[0].object;
+        assertEquals("context2", oldContext.getName());
+
+        Context oldContext1 = mappedHost.contextList.contexts[contextPos - 1].versions[0].object;
+        assertEquals("context1", oldContext1.getName());
+
+        mappingData.recycle();
+        mapper.map(hostMB, uriMB, null, mappingData);
+        assertEquals("blah7", mappingData.host.getName());
+        assertEquals("context2", mappingData.context.getName());
+        assertEquals("wrapper5", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(oldContext, uriMB, mappingData);
+        assertEquals("wrapper5", mappingData.wrapper.getName());
+
+        // Mark context as paused
+        // This is what happens when context reload starts
+        mapper.pauseContextVersion(oldContext, hostName, contextPath, "0");
+
+        mappingData.recycle();
+        mapper.map(hostMB, uriMB, null, mappingData);
+        assertEquals("blah7", mappingData.host.getName());
+        assertEquals("context2", mappingData.context.getName());
+        // Wrapper is not mapped for incoming requests if context is paused
+        assertNull(mappingData.wrapper);
+        mappingData.recycle();
+        mapper.map(oldContext, uriMB, mappingData);
+        // Wrapper is mapped for mapping method used by forward or include dispatch
+        assertEquals("wrapper5", mappingData.wrapper.getName());
+
+        // Re-add the same context, but different list of wrappers
+        // This is what happens when context reload completes
+        mapper.addContextVersion(
+                hostName,
+                oldHost,
+                contextPath,
+                "0",
+                oldContext,
+                null,
+                null,
+                Arrays.asList(new WrapperMappingInfo[] { new WrapperMappingInfo(
+                        "/", createWrapper("newDefaultWrapper"), false, false) }));
+
+        mappedContext = mappedHost.contextList.contexts[contextPos];
+        assertEquals(contextPath, mappedContext.name);
+        assertEquals(1, mappedContext.versions.length);
+        assertEquals("0", mappedContext.versions[0].name);
+
+        mappingData.recycle();
+        mapper.map(hostMB, uriMB, null, mappingData);
+        assertEquals("blah7", mappingData.host.getName());
+        assertEquals("context2", mappingData.context.getName());
+        assertEquals("newDefaultWrapper", mappingData.wrapper.getName());
+        mappingData.recycle();
+        mapper.map(oldContext, uriMB, mappingData);
+        assertEquals("newDefaultWrapper", mappingData.wrapper.getName());
     }
 
     @Test

Modified: tomcat/trunk/test/org/apache/catalina/mapper/TestMapperWebapps.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/mapper/TestMapperWebapps.java?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/mapper/TestMapperWebapps.java (original)
+++ tomcat/trunk/test/org/apache/catalina/mapper/TestMapperWebapps.java Mon Aug 25 13:47:56 2014
@@ -86,7 +86,7 @@ public class TestMapperWebapps extends T
     }
 
     @Test
-    public void testContextReload_Bug56658() throws Exception {
+    public void testContextReload_Bug56658_Bug56882() throws Exception {
         Tomcat tomcat = getTomcatInstance();
 
         File appDir = new File(getBuildDirectory(), "webapps/examples");
@@ -115,6 +115,24 @@ public class TestMapperWebapps extends T
         text = res.toString();
         Assert.assertTrue(text, text.contains("<title>Apache Tomcat Examples</title>"));
 
+        long timeA = System.currentTimeMillis();
+        res = getUrl("http://localhost:" + getPort()
+                + "/examples/jsp/include/include.jsp");
+        String timestamp = findCommonPrefix(timeA, System.currentTimeMillis());
+        text = res.toString();
+        Assert.assertTrue(text, text.contains(
+                "In place evaluation of another JSP which gives you the current time: " + timestamp));
+        Assert.assertTrue(text, text.contains(
+                "To get the current time in ms"));
+        Assert.assertTrue(text, text.contains(
+                "by including the output of another JSP: " + timestamp));
+        Assert.assertTrue(text, text.contains(":-)"));
+
+        res = getUrl("http://localhost:" + getPort()
+                + "/examples/jsp/forward/forward.jsp");
+        text = res.toString();
+        Assert.assertTrue(text, text.contains("VM Memory usage"));
+
         ctxt.reload();
 
         res = getUrl("http://localhost:" + getPort()
@@ -130,6 +148,24 @@ public class TestMapperWebapps extends T
         res = getUrl("http://localhost:" + getPort() + "/examples/index.html");
         text = res.toString();
         Assert.assertTrue(text, text.contains("<title>Apache Tomcat Examples</title>"));
+
+        timeA = System.currentTimeMillis();
+        res = getUrl("http://localhost:" + getPort()
+                + "/examples/jsp/include/include.jsp");
+        timestamp = findCommonPrefix(timeA, System.currentTimeMillis());
+        text = res.toString();
+        Assert.assertTrue(text, text.contains(
+                "In place evaluation of another JSP which gives you the current time: " + timestamp));
+        Assert.assertTrue(text, text.contains(
+                "To get the current time in ms"));
+        Assert.assertTrue(text, text.contains(
+                "by including the output of another JSP: " + timestamp));
+        Assert.assertTrue(text, text.contains(":-)"));
+
+        res = getUrl("http://localhost:" + getPort()
+                + "/examples/jsp/forward/forward.jsp");
+        text = res.toString();
+        Assert.assertTrue(text, text.contains("VM Memory usage"));
     }
 
     @Test
@@ -189,4 +225,17 @@ public class TestMapperWebapps extends T
                 new HashMap<String,List<String>>());
         Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc);
     }
+
+    /**
+     * Prepare a string to search in messages that contain a timestamp, when it
+     * is known that the timestamp was printed between {@code timeA} and
+     * {@code timeB}.
+     */
+    private static String findCommonPrefix(long timeA, long timeB) {
+        while ((timeA != timeB) && timeA > 0) {
+            timeA /= 10;
+            timeB /= 10;
+        }
+        return String.valueOf(timeA);
+    }
 }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Aug 25 13:47:56 2014
@@ -56,6 +56,11 @@
         Add support for the EECDH alias when using the OpenSSL cipher syntax to
         define JSSE ciphers. (markt)
       </add>
+      <fix>
+        <bug>56882</bug>: Fix regression in processing of includes and forwards
+        when Context have been reloaded. Tomcat was responding with HTTP Status
+        503 (Servlet xxx is currently unavailable). (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

Modified: tomcat/trunk/webapps/examples/jsp/forward/one.jsp
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/examples/jsp/forward/one.jsp?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/webapps/examples/jsp/forward/one.jsp (original)
+++ tomcat/trunk/webapps/examples/jsp/forward/one.jsp Mon Aug 25 13:47:56 2014
@@ -19,5 +19,5 @@
 <body bgcolor="white">
 <font color="red">
 
-VM Memory usage < 50%.
+VM Memory usage &lt; 50%.
 </html>
\ No newline at end of file

Modified: tomcat/trunk/webapps/examples/jsp/forward/two.html
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/examples/jsp/forward/two.html?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/webapps/examples/jsp/forward/two.html (original)
+++ tomcat/trunk/webapps/examples/jsp/forward/two.html Mon Aug 25 13:47:56 2014
@@ -19,5 +19,5 @@
 <body bgcolor="white">
 <font color="red">
 
-VM Memory usage > 50%.
+VM Memory usage &gt; 50%.
 </html>
\ No newline at end of file

Modified: tomcat/trunk/webapps/examples/jsp/include/foo.jsp
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/examples/jsp/include/foo.jsp?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/webapps/examples/jsp/include/foo.jsp (original)
+++ tomcat/trunk/webapps/examples/jsp/include/foo.jsp Mon Aug 25 13:47:56 2014
@@ -13,9 +13,5 @@
   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.
---%>
 
-<body bgcolor="white">
-<font color="red">
-
-<%= System.currentTimeMillis() %>
+--%><%= System.currentTimeMillis() %>

Modified: tomcat/trunk/webapps/examples/jsp/include/include.jsp
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/examples/jsp/include/include.jsp?rev=1620326&r1=1620325&r2=1620326&view=diff
==============================================================================
--- tomcat/trunk/webapps/examples/jsp/include/include.jsp (original)
+++ tomcat/trunk/webapps/examples/jsp/include/include.jsp Mon Aug 25 13:47:56 2014
@@ -22,14 +22,9 @@
 
 <%@ page buffer="5kb" autoFlush="false" %>
 
-<p>In place evaluation of another JSP which gives you the current time:
-
-<%@ include file="foo.jsp" %>
-
-<p> <jsp:include page="foo.html" flush="true"/> by including the output of another JSP:
-
-<jsp:include page="foo.jsp" flush="true"/>
+<p>In place evaluation of another JSP which gives you the current time: <%@ include file="foo.jsp" %>
 
+<p> <jsp:include page="foo.html" flush="true"/> by including the output of another JSP: <jsp:include page="foo.jsp" flush="true"/>
 :-)
 
 </html>



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