Author: kkolinko
Date: Sun Nov 9 14:04:37 2014
New Revision: 1637679
URL: http://svn.apache.org/r1637679
Log:
Assert that MappingData object is empty before performing mapping work.
It is backport of r1604663
Motivation: Remove dead branches. Protect Mapper from operating on
non-recycled MappingData. Using non-recycled MappingData might result in
mapping request onto a different target, like an issue that prevented us
from releasing 8.0.4 and fixed by r1580080/r1580083. I do not know such
bugs in Tomcat 6, but I want the code to be more safe.
Just a single (mappingData.host != null) check is enough to discern
recycled vs. non-recycled data. Checks for other MappingData fields are
removed by this patch, simplifying the code.
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1637679&r1=1637678&r2=1637679&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sun Nov 9 14:04:37 2014
@@ -28,26 +28,6 @@ None
PATCHES PROPOSED TO BACKPORT:
[ New proposals should be added at the end of the list ]
-* Assert that MappingData object is empty before performing mapping work.
- It is backport of r1604663
-
- Motivation: Remove dead branches. Protect Mapper from operating on
- non-recycled MappingData. Using non-recycled MappingData might result in
- mapping request onto a different target, like an issue that prevented us
- from releasing 8.0.4 and fixed by r1580080/r1580083. I do not know such
- bugs in Tomcat 6, but I want the code to be more safe.
- Just a single (mappingData.host != null) check is enough to discern
- recycled vs. non-recycled data. Checks for other MappingData fields are
- removed by this patch, simplifying the code.
-
- A patch generated with "svn diff -x --ignore-space-change" for easier
- overview of the change is
-
https://people.apache.org/~kkolinko/patches/2014-06-23_tc6_Mapper_diff-x-b.patch
-
- https://people.apache.org/~kkolinko/patches/2014-06-23_tc6_Mapper.patch
- +1: kkolinko, markt, remm
- -1:
-
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56780
Enable Tomcat to start when using a IBM JRE in strict SP800-131a mode
This back-ports the fix as well as some additional changes to more closely
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java?rev=1637679&r1=1637678&r2=1637679&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java
Sun Nov 9 14:04:37 2014
@@ -588,82 +588,86 @@ public final class Mapper {
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;
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.context = context.object;
- 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.context = context.object;
+ mappingData.contextPath.setString(context.name);
}
// Wrapper mapping
- if ((context != null) && (mappingData.wrapper == null)) {
+ if (context != null) {
internalMapWrapper(context, uri, mappingData);
}
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1637679&r1=1637678&r2=1637679&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sun Nov 9 14:04:37 2014
@@ -44,6 +44,14 @@
General, Catalina, Coyote, Jasper, Cluster, Web applications, Other
-->
<section name="Tomcat 6.0.43 (markt)">
+ <subsection name="Catalina">
+ <changelog>
+ <fix>
+ Assert that mapping result object is empty before performing mapping
+ work in <code>Mapper</code>. (kkolinko)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Coyote">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
