Author: kkolinko
Date: Sat Jun 21 16:59:33 2014
New Revision: 1604434

URL: http://svn.apache.org/r1604434
Log:
Assert that MappingData object is empty before performing mapping work.
The old code just skipped all work in that case, but I do not see why that 
would be useful,
and it may return inconsistent results.
The comet and async examples do work successfully with this change.
This allows to simplify the code in Mapper.internalMap() by removing a number 
of checks.

Modified:
    tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java
    tomcat/trunk/webapps/docs/changelog.xml

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=1604434&r1=1604433&r2=1604434&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/mapper/Mapper.java Sat Jun 21 
16:59:33 2014
@@ -622,99 +622,100 @@ public final class Mapper {
     private final void internalMap(CharChunk host, CharChunk uri,
             String version, MappingData mappingData) throws Exception {
 
-        uri.setLimit(-1);
+        if (mappingData.host != null) {
+            // The legacy code (dating down at least to Tomcat 4.1) just
+            // skipped all mapping work in this case. That behaviour has a risk
+            // of returning an inconsistent result.
+            // I do not see a valid use case for it.
+            throw new AssertionError();
+        }
 
-        ContextList contextList = null;
-        MappedContext context = null;
-        ContextVersion contextVersion = null;
+        uri.setLimit(-1);
 
         // Virtual host mapping
-        if (mappingData.host == null) {
-            HostMapping[] hosts = this.hosts;
-            HostMapping hostMapping = exactFindIgnoreCase(hosts, host);
+        HostMapping[] hosts = this.hosts;
+        HostMapping hostMapping = exactFindIgnoreCase(hosts, host);
+        if (hostMapping == null) {
+            if (defaultHostName == null) {
+                return;
+            }
+            hostMapping = exactFind(hosts, defaultHostName);
             if (hostMapping == null) {
-                if (defaultHostName == null) {
-                    return;
-                }
-                hostMapping = exactFind(hosts, defaultHostName);
-                if (hostMapping == null) {
-                    return;
-                }
+                return;
             }
-            mappingData.host = hostMapping.object.host;
-            contextList = hostMapping.object.contextList;
         }
+        mappingData.host = hostMapping.object.host;
 
         // Context mapping
-        if (mappingData.context == null && contextList != null) {
-            MappedContext[] contexts = contextList.contexts;
-            int pos = find(contexts, uri);
-            if (pos == -1) {
-                return;
-            }
+        ContextList contextList = hostMapping.object.contextList;
+        MappedContext[] contexts = contextList.contexts;
+        int pos = find(contexts, uri);
+        if (pos == -1) {
+            return;
+        }
 
-            int lastSlash = -1;
-            int uriEnd = uri.getEnd();
-            int length = -1;
-            boolean found = false;
-            while (pos >= 0) {
-                if (uri.startsWith(contexts[pos].name)) {
-                    length = contexts[pos].name.length();
-                    if (uri.getLength() == length) {
-                        found = true;
-                        break;
-                    } else if (uri.startsWithIgnoreCase("/", length)) {
-                        found = true;
-                        break;
-                    }
-                }
-                if (lastSlash == -1) {
-                    lastSlash = nthSlash(uri, contextList.nesting + 1);
-                } else {
-                    lastSlash = lastSlash(uri);
+        int lastSlash = -1;
+        int uriEnd = uri.getEnd();
+        int length = -1;
+        boolean found = false;
+        while (pos >= 0) {
+            if (uri.startsWith(contexts[pos].name)) {
+                length = contexts[pos].name.length();
+                if (uri.getLength() == length) {
+                    found = true;
+                    break;
+                } else if (uri.startsWithIgnoreCase("/", length)) {
+                    found = true;
+                    break;
                 }
-                uri.setEnd(lastSlash);
-                pos = find(contexts, uri);
             }
-            uri.setEnd(uriEnd);
-
-            if (found) {
-                context = contexts[pos];
-            } else if (contexts[0].name.equals("")) {
-                context = contexts[0];
+            if (lastSlash == -1) {
+                lastSlash = nthSlash(uri, contextList.nesting + 1);
+            } else {
+                lastSlash = lastSlash(uri);
             }
+            uri.setEnd(lastSlash);
+            pos = find(contexts, uri);
+        }
+        uri.setEnd(uriEnd);
 
-            if (context != null) {
-                mappingData.contextPath.setString(context.name);
-            }
+        MappedContext context;
+        if (found) {
+            context = contexts[pos];
+        } else if (contexts[0].name.equals("")) {
+            context = contexts[0];
+        } else {
+            context = null;
         }
 
-        if (context != null) {
-            ContextVersion[] contextVersions = context.versions;
-            int versionCount = contextVersions.length;
-            if (versionCount > 1) {
-                Context[] contextObjects = new Context[contextVersions.length];
-                for (int i = 0; i < contextObjects.length; i++) {
-                    contextObjects[i] = contextVersions[i].object;
-                }
-                mappingData.contexts = contextObjects;
-            }
+        if (context == null) {
+            return;
+        }
+        mappingData.contextPath.setString(context.name);
 
-            if (version != null) {
-                contextVersion = exactFind(contextVersions, version);
-            }
-            if (contextVersion == null) {
-                // Return the latest version
-                contextVersion = contextVersions[versionCount - 1];
+        ContextVersion[] contextVersions = context.versions;
+        int versionCount = contextVersions.length;
+        if (versionCount > 1) {
+            Context[] contextObjects = new Context[contextVersions.length];
+            for (int i = 0; i < contextObjects.length; i++) {
+                contextObjects[i] = contextVersions[i].object;
             }
-            mappingData.context = contextVersion.object;
-            mappingData.contextSlashCount = contextVersion.slashCount;
+            mappingData.contexts = contextObjects;
         }
 
-        // Wrapper mapping
-        if ((contextVersion != null) && (mappingData.wrapper == null)) {
-            internalMapWrapper(contextVersion, uri, mappingData);
+        ContextVersion contextVersion = null;
+        if (version != null) {
+            contextVersion = exactFind(contextVersions, version);
+        }
+        if (contextVersion == null) {
+            // Return the latest version
+            contextVersion = contextVersions[versionCount - 1];
         }
+        mappingData.context = contextVersion.object;
+        mappingData.contextSlashCount = contextVersion.slashCount;
+
+        // Wrapper mapping
+        internalMapWrapper(contextVersion, uri, mappingData);
 
     }
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1604434&r1=1604433&r2=1604434&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Jun 21 16:59:33 2014
@@ -66,6 +66,10 @@
         <bug>56653</bug>: Fix concurrency issue with
         <code>Mapper$ContextList</code> when stopping Contexts. (kkolinko)
       </fix>
+      <fix>
+        Assert that mapping result object is empty before performing mapping
+        work in <code>Mapper</code>. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
 </section>



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

Reply via email to