Author: kkolinko Date: Mon Jun 23 18:01:21 2014 New Revision: 1604895 URL: http://svn.apache.org/r1604895 Log: https://issues.apache.org/bugzilla/show_bug.cgi?id=44312 Log an error if there is a conflict between Host and Alias names. Improve addHost, addHostAlias, removeHost, removeHostAlias methods in Mapper to avoid occasionally removing a wrong host. (Add a check of whether the expected object was found, and whether it is the primary Host or an Alias).
Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/LocalStrings.properties tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/Mapper.java tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/mapper/TestMapper.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/LocalStrings.properties?rev=1604895&r1=1604894&r2=1604895&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/LocalStrings.properties (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/http/mapper/LocalStrings.properties Mon Jun 23 18:01:21 2014 @@ -13,4 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -mapper.removeWrapper=Removing wrapper from Context [{0}] with path [{1}] \ No newline at end of file +mapper.removeWrapper=Removing wrapper from Context [{0}] with path [{1}] +mapper.duplicateHost=Duplicate Host [{0}]. The name is already used by Host [{1}]. This Host will be ignored. +mapper.duplicateHostAlias=Duplicate host Alias [{0}] in Host [{1}]. The name is already used by Host [{2}]. This Alias will be ignored. 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=1604895&r1=1604894&r2=1604895&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 Mon Jun 23 18:01:21 2014 @@ -87,23 +87,18 @@ public final class Mapper { public synchronized void addHost(String name, String[] aliases, Object host) { Host[] newHosts = new Host[hosts.length + 1]; - Host newHost = new Host(); - ContextList contextList = new ContextList(); - newHost.name = name; - newHost.contextList = contextList; - newHost.object = host; + Host newHost = new Host(name, host); if (insertMap(hosts, newHosts, newHost)) { hosts = newHosts; + } else { + Host duplicate = hosts[find(hosts, name)]; + log.error(sm.getString("mapper.duplicateHost", name, + duplicate.realHostName)); + // Do not add aliases, as removeHost(hostName) won't be able to remove them + return; } - for (int i = 0; i < aliases.length; i++) { - newHosts = new Host[hosts.length + 1]; - newHost = new Host(); - newHost.name = aliases[i]; - newHost.contextList = contextList; - newHost.object = host; - if (insertMap(hosts, newHosts, newHost)) { - hosts = newHosts; - } + for (String alias : aliases) { + addHostAliasImpl(alias, newHost); } } @@ -115,21 +110,22 @@ public final class Mapper { */ public synchronized void removeHost(String name) { // Find and remove the old host - int pos = find(hosts, name); - if (pos < 0) { + Host host = exactFind(hosts, name); + if (host == null || host.isAlias()) { return; } - Object host = hosts[pos].object; + Object catalinaHost = host.object; Host[] newHosts = new Host[hosts.length - 1]; if (removeMap(hosts, newHosts, name)) { hosts = newHosts; - } - // Remove all aliases (they will map to the same host object) - for (int i = 0; i < newHosts.length; i++) { - if (newHosts[i].object == host) { - Host[] newHosts2 = new Host[hosts.length - 1]; - if (removeMap(hosts, newHosts2, newHosts[i].name)) { - hosts = newHosts2; + + // Remove all aliases (they will map to the same host object) + for (int i = 0; i < newHosts.length; i++) { + if (newHosts[i].object == catalinaHost) { + Host[] newHosts2 = new Host[hosts.length - 1]; + if (removeMap(hosts, newHosts2, newHosts[i].name)) { + hosts = newHosts2; + } } } } @@ -141,21 +137,30 @@ public final class Mapper { * @param alias The alias to add */ public synchronized void addHostAlias(String name, String alias) { - int pos = find(hosts, name); - if (pos < 0) { + Host realHost = exactFind(hosts, name); + if (realHost == null) { // Should not be adding an alias for a host that doesn't exist but // just in case... return; } - Host realHost = hosts[pos]; + addHostAliasImpl(alias, realHost); + } + private void addHostAliasImpl(String alias, Host realHost) { + Host newHost = new Host(alias, realHost); Host[] newHosts = new Host[hosts.length + 1]; - Host newHost = new Host(); - newHost.name = alias; - newHost.contextList = realHost.contextList; - newHost.object = realHost.object; if (insertMap(hosts, newHosts, newHost)) { hosts = newHosts; + } else { + Host duplicate = hosts[find(hosts, alias)]; + if (duplicate.object == realHost.object) { + // A duplicate Alias for the same Host. + // A harmless redundancy. E.g. + // <Host name="localhost"><Alias>localhost</Alias></Host> + return; + } + log.error(sm.getString("mapper.duplicateHostAlias", alias, + realHost.realHostName, duplicate.realHostName)); } } @@ -165,8 +170,8 @@ public final class Mapper { */ public synchronized void removeHostAlias(String alias) { // Find and remove the alias - int pos = find(hosts, alias); - if (pos < 0) { + Host host = exactFind(hosts, alias); + if (host == null || !host.isAlias()) { return; } Host[] newHosts = new Host[hosts.length - 1]; @@ -1441,8 +1446,30 @@ public final class Mapper { protected static final class Host extends MapElement { - public ContextList contextList = null; + public final String realHostName; + public ContextList contextList; + + /** + * Creates an object for primary Host + */ + public Host(String name, Object host) { + super(name, host); + this.realHostName = name; + this.contextList = new ContextList(); + } + + /** + * Creates an object for an Alias + */ + public Host(String alias, Host realHost) { + super(alias, realHost.object); + this.realHostName = realHost.name; + this.contextList = realHost.contextList; + } + public boolean isAlias() { + return !name.equals(realHostName); + } } Modified: tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/mapper/TestMapper.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/mapper/TestMapper.java?rev=1604895&r1=1604894&r2=1604895&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/mapper/TestMapper.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/tomcat/util/http/mapper/TestMapper.java Mon Jun 23 18:01:21 2014 @@ -16,11 +16,11 @@ */ package org.apache.tomcat.util.http.mapper; +import java.util.concurrent.atomic.AtomicBoolean; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import java.util.concurrent.atomic.AtomicBoolean; - import org.junit.Before; import org.junit.Test; @@ -94,8 +94,13 @@ public class TestMapper extends LoggingB mapper.addHost("iowejoiejfoiew", new String[0], "blah17"); // Alias conflicting with existing Host mapper.addHostAlias("iowejoiejfoiew", "qwigqwiwoih"); + // Alias conflicting with existing Alias + mapper.addHostAlias("sjbjdvwsbvhrb", "iowejoiejfoiew_alias"); // Redundancy. Alias name = Host name. No error here. mapper.addHostAlias("qwigqwiwoih", "qwigqwiwoih"); + // Redundancy. Duplicate Alias for the same Host name. No error here. + mapper.addHostAlias("iowejoiejfoiew", "iowejoiejfoiew_alias"); + mapper.addHostAlias("iowejoiejfoiew", "iowejoiejfoiew_alias"); // Check we have the right number // (added 16 including one host alias. Three duplicates do not increase the count.) @@ -127,6 +132,29 @@ public class TestMapper extends LoggingB } @Test + public void testRemoveHost() { + assertEquals(16, mapper.hosts.length); + mapper.removeHostAlias("iowejoiejfoiew"); + mapper.removeHost("iowejoiejfoiew_alias"); + assertEquals(16, mapper.hosts.length); // No change + mapper.removeHostAlias("iowejoiejfoiew_alias"); + assertEquals(15, mapper.hosts.length); // Removed + + mapper.addHostAlias("iowejoiejfoiew", "iowejoiejfoiew_alias"); + assertEquals(16, mapper.hosts.length); + + final int iowPos = 3; + Mapper.Host hostMapping = mapper.hosts[iowPos]; + Mapper.Host aliasMapping = mapper.hosts[iowPos + 1]; + assertEquals("iowejoiejfoiew_alias", aliasMapping.name); + assertTrue(aliasMapping.isAlias()); + assertEquals(hostMapping.object, aliasMapping.object); + + mapper.removeHost("iowejoiejfoiew"); + assertEquals(14, mapper.hosts.length); // Both host and alias removed + } + + @Test public void testMap() throws Exception { MappingData mappingData = new MappingData(); MessageBytes host = MessageBytes.newInstance(); 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=1604895&r1=1604894&r2=1604895&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Jun 23 18:01:21 2014 @@ -59,6 +59,11 @@ <subsection name="Catalina"> <changelog> <fix> + <bug>44312</bug>: Log an error if there is a conflict between Host and + Alias names. Improve host management methods in <code>Mapper</code> + to avoid occasionally removing a wrong host. (kkolinko) + </fix> + <fix> <bug>55282</bug>: Ensure that one and the same application listener is added only once when starting the web application. (violetagg) </fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org