Author: kkolinko Date: Sun Jun 22 23:05:10 2014 New Revision: 1604663 URL: http://svn.apache.org/r1604663 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, async, websocket examples do work successfully with this change. It is backport of r1604434 from tomcat/trunk.
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java?rev=1604663&r1=1604662&r2=1604663&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java Sun Jun 22 23:05:10 2014 @@ -726,79 +726,83 @@ public final class Mapper { private final void internalMap(CharChunk host, CharChunk uri, String version, MappingData mappingData) throws Exception { + 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(); + } + uri.setLimit(-1); - Context[] contexts = null; + Context[] contexts; Context context = null; ContextVersion contextVersion = null; int nesting = 0; // Virtual host mapping - if (mappingData.host == null) { - Host[] hosts = this.hosts; - int pos = findIgnoreCase(hosts, host); - if ((pos != -1) && (host.equalsIgnoreCase(hosts[pos].name))) { + Host[] hosts = this.hosts; + int pos = findIgnoreCase(hosts, host); + if ((pos != -1) && (host.equalsIgnoreCase(hosts[pos].name))) { + mappingData.host = hosts[pos].object; + contexts = hosts[pos].contextList.contexts; + nesting = hosts[pos].contextList.nesting; + } else { + if (defaultHostName == null) { + return; + } + pos = find(hosts, defaultHostName); + if ((pos != -1) && (defaultHostName.equals(hosts[pos].name))) { mappingData.host = hosts[pos].object; contexts = hosts[pos].contextList.contexts; nesting = hosts[pos].contextList.nesting; } else { - if (defaultHostName == null) { - return; - } - pos = find(hosts, defaultHostName); - if ((pos != -1) && (defaultHostName.equals(hosts[pos].name))) { - mappingData.host = hosts[pos].object; - contexts = hosts[pos].contextList.contexts; - nesting = hosts[pos].contextList.nesting; - } else { - return; - } + return; } } // Context mapping - if (mappingData.context == null) { - int pos = find(contexts, uri); - if (pos == -1) { - return; - } + 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, 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) { - if (contexts[0].name.equals("")) { - context = contexts[0]; - } + if (lastSlash == -1) { + lastSlash = nthSlash(uri, nesting + 1); } else { - context = contexts[pos]; + lastSlash = lastSlash(uri); } - if (context != null) { - mappingData.contextPath.setString(context.name); + uri.setEnd(lastSlash); + pos = find(contexts, uri); + } + uri.setEnd(uriEnd); + + if (!found) { + if (contexts[0].name.equals("")) { + context = contexts[0]; } + } else { + context = contexts[pos]; + } + if (context != null) { + mappingData.contextPath.setString(context.name); } if (context != null) { @@ -816,7 +820,7 @@ public final class Mapper { // Return the latest version contextVersion = contextVersions[versionCount - 1]; } else { - int pos = find(contextVersions, version); + pos = find(contextVersions, version); if (pos < 0 || !contextVersions[pos].name.equals(version)) { // Return the latest version contextVersion = contextVersions[versionCount - 1]; @@ -829,7 +833,7 @@ public final class Mapper { } // Wrapper mapping - if ((contextVersion != null) && (mappingData.wrapper == null)) { + if (contextVersion != null) { internalMapWrapper(contextVersion, uri, mappingData); } Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1604663&r1=1604662&r2=1604663&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Sun Jun 22 23:05:10 2014 @@ -112,6 +112,10 @@ version. Ensure that remapping selects the version that we expect. (kkolinko) </fix> + <fix> + Assert that mapping result object is empty before performing mapping + work in <code>Mapper</code>. (kkolinko) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org