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

Reply via email to