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