Author: trustin Date: 2008-11-13 01:55:17 -0500 (Thu, 13 Nov 2008) New Revision: 4673 Added: remoting2/branches/2.2/.settings/ remoting2/branches/2.2/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java remoting2/branches/2.x/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java Modified: remoting2/branches/2.2/src/main/org/jboss/remoting/ConnectionValidator.java remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java Log: Fixed issue: JBREM-1055 (ConnectionValidator.run() should have a sanity test to prevent calls from application code) * Added an if block that throws an IllegalStateException when run() is called directly * Added its related test case Modified: remoting2/branches/2.2/src/main/org/jboss/remoting/ConnectionValidator.java =================================================================== --- remoting2/branches/2.2/src/main/org/jboss/remoting/ConnectionValidator.java 2008-11-13 06:42:30 UTC (rev 4672) +++ remoting2/branches/2.2/src/main/org/jboss/remoting/ConnectionValidator.java 2008-11-13 06:55:17 UTC (rev 4673) @@ -22,11 +22,6 @@ package org.jboss.remoting; -import org.jboss.logging.Logger; -import org.jboss.remoting.transport.ClientInvoker; -import org.jboss.remoting.util.StoppableTimerTask; -import org.jboss.remoting.util.TimerUtil; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -35,9 +30,15 @@ import java.util.Timer; import java.util.TimerTask; +import org.jboss.logging.Logger; +import org.jboss.remoting.transport.ClientInvoker; +import org.jboss.remoting.util.StoppableTimerTask; +import org.jboss.remoting.util.TimerUtil; + /** * @author <a href="mailto:tom.elrod@jboss.com">Tom Elrod</a> * @author <a href="mailto:ovidiu@jboss.org">Ovidiu Feodorov</a> + * @author <a href="mailto:tlee@redhat.com">Trustin Lee</a> */ public class ConnectionValidator extends TimerTask implements StoppableTimerTask { @@ -172,11 +173,15 @@ o = config.get("NumberOfCallRetries"); if (o != null) + { localConfig.put("NumberOfCallRetries", o); + } o = config.get("NumberOfRetries"); if (o != null) + { localConfig.put("NumberOfRetries", o); + } } if (metadata != null) @@ -200,10 +205,14 @@ } if (localConfig.get(ServerInvoker.TIMEOUT) == null) + { localConfig.put(ServerInvoker.TIMEOUT, DEFAULT_PING_TIMEOUT); + } if (localConfig.get("NumberOfCallRetries") == null) + { localConfig.put("NumberOfCallRetries", DEFAULT_NUMBER_OF_PING_RETRIES); + } return localConfig; } @@ -238,8 +247,8 @@ { this.client = client; this.pingPeriod = pingPeriod; - this.listeners = new ArrayList(); - this.stopped = false; + listeners = new ArrayList(); + stopped = false; getParameters(client, new HashMap()); log.debug(this + " created"); } @@ -247,9 +256,9 @@ public ConnectionValidator(Client client, Map metadata) { this.client = client; - this.pingPeriod = DEFAULT_PING_PERIOD; - this.listeners = new ArrayList(); - this.stopped = false; + pingPeriod = DEFAULT_PING_PERIOD; + listeners = new ArrayList(); + stopped = false; this.metadata = new HashMap(metadata); getParameters(client, metadata); log.debug(this + " created"); @@ -274,6 +283,16 @@ */ public void run() { + synchronized (lock) { + if (timer == null) + { + throw new IllegalStateException( + ConnectionValidator.class.getName() + ".run() should not be " + + "called directly; use " + ConnectionValidator.class.getName() + + ".addConnectionListener() instead."); + } + } + TimerTask tt = new WaitOnConnectionCheckTimerTask(); try @@ -297,7 +316,10 @@ if (tieToLease && client.getLeasePeriod() > 0) { - if (trace) log.trace(this + " sending PING tied to lease"); + if (trace) + { + log.trace(this + " sending PING tied to lease"); + } isValid = doCheckConnectionWithLease(); } else @@ -513,7 +535,7 @@ try { Map metadata = new HashMap(); - metadata.put(ServerInvoker.INVOKER_SESSION_ID, this.invokerSessionId); + metadata.put(ServerInvoker.INVOKER_SESSION_ID, invokerSessionId); InvocationRequest ir = new InvocationRequest(null, Subsystem.SELF, "$PING$", metadata, null, null); @@ -571,13 +593,16 @@ synchronized(lock) { if (stopped) + { return false; + } if (!listeners.isEmpty()) { listeners.clear(); } stopped = true; + timer = null; } if (clientInvoker != null) @@ -627,7 +652,10 @@ { int elapsed = (int) (System.currentTimeMillis() - start); int wait = pingTimeout - elapsed; - if (wait <= 0) break; + if (wait <= 0) + { + break; + } try { Added: remoting2/branches/2.2/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java =================================================================== --- remoting2/branches/2.2/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java (rev 0) +++ remoting2/branches/2.2/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java 2008-11-13 06:55:17 UTC (rev 4673) @@ -0,0 +1,78 @@ +package org.jboss.test.remoting; + +import java.io.IOException; +import java.net.MalformedURLException; +import java.util.Map; + +import junit.framework.TestCase; + +import org.jboss.remoting.Client; +import org.jboss.remoting.ConnectionFailedException; +import org.jboss.remoting.ConnectionValidator; +import org.jboss.remoting.InvokerLocator; +import org.jboss.remoting.MicroRemoteClientInvoker; +import org.jboss.remoting.marshal.Marshaller; +import org.jboss.remoting.marshal.UnMarshaller; +import org.jboss.remoting.transport.ClientInvoker; + +public class ConnectionValidatorTest extends TestCase +{ + + public void testShouldDisallowDirectRun() + { + ConnectionValidator cv = new ConnectionValidator(new Client() { + public Map getConfiguration() + { + return null; + } + + public ClientInvoker getInvoker() + { + try + { + return new MicroRemoteClientInvoker( + new InvokerLocator("http://dummy:65535/dummy/")) { + + public String getSessionId() + { + return "dummyId"; + } + + protected String getDefaultDataType() + { + throw new UnsupportedOperationException(); + } + + protected void handleConnect() throws ConnectionFailedException + { + throw new UnsupportedOperationException(); + } + + protected void handleDisconnect() + { + throw new UnsupportedOperationException(); + } + + protected Object transport(String sessionId, Object invocation, Map metadata, Marshaller marshaller, + UnMarshaller unmarshaller) throws IOException, ConnectionFailedException, ClassNotFoundException + { + throw new UnsupportedOperationException(); + } + }; + } + catch (MalformedURLException e) + { + throw new RuntimeException(e); + } + } + }); + + try + { + cv.run(); + fail("Should throw IllegalStateException"); + } catch (IllegalStateException e) { + // Expected + } + } +} Property changes on: remoting2/branches/2.2/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java ___________________________________________________________________ Name: svn:keywords + Rev Date Name: svn:eol-style + native Modified: remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java =================================================================== --- remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java 2008-11-13 06:42:30 UTC (rev 4672) +++ remoting2/branches/2.x/src/main/org/jboss/remoting/ConnectionValidator.java 2008-11-13 06:55:17 UTC (rev 4673) @@ -22,11 +22,6 @@ package org.jboss.remoting; -import org.jboss.logging.Logger; -import org.jboss.remoting.transport.ClientInvoker; -import org.jboss.remoting.util.StoppableTimerTask; -import org.jboss.remoting.util.TimerUtil; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -35,9 +30,15 @@ import java.util.Timer; import java.util.TimerTask; +import org.jboss.logging.Logger; +import org.jboss.remoting.transport.ClientInvoker; +import org.jboss.remoting.util.StoppableTimerTask; +import org.jboss.remoting.util.TimerUtil; + /** * @author <a href="mailto:tom.elrod@jboss.com">Tom Elrod</a> * @author <a href="mailto:ovidiu@jboss.org">Ovidiu Feodorov</a> + * @author <a href="mailto:tlee@redhat.com">Trustin Lee</a> */ public class ConnectionValidator extends TimerTask implements StoppableTimerTask { @@ -172,11 +173,15 @@ o = config.get("NumberOfCallRetries"); if (o != null) + { localConfig.put("NumberOfCallRetries", o); + } o = config.get("NumberOfRetries"); if (o != null) + { localConfig.put("NumberOfRetries", o); + } } if (metadata != null) @@ -200,10 +205,14 @@ } if (localConfig.get(ServerInvoker.TIMEOUT) == null) + { localConfig.put(ServerInvoker.TIMEOUT, DEFAULT_PING_TIMEOUT); + } if (localConfig.get("NumberOfCallRetries") == null) + { localConfig.put("NumberOfCallRetries", DEFAULT_NUMBER_OF_PING_RETRIES); + } return localConfig; } @@ -238,8 +247,8 @@ { this.client = client; this.pingPeriod = pingPeriod; - this.listeners = new ArrayList(); - this.stopped = false; + listeners = new ArrayList(); + stopped = false; getParameters(client, new HashMap()); log.debug(this + " created"); } @@ -247,9 +256,9 @@ public ConnectionValidator(Client client, Map metadata) { this.client = client; - this.pingPeriod = DEFAULT_PING_PERIOD; - this.listeners = new ArrayList(); - this.stopped = false; + pingPeriod = DEFAULT_PING_PERIOD; + listeners = new ArrayList(); + stopped = false; this.metadata = new HashMap(metadata); getParameters(client, metadata); log.debug(this + " created"); @@ -274,6 +283,16 @@ */ public void run() { + synchronized (lock) { + if (timer == null) + { + throw new IllegalStateException( + ConnectionValidator.class.getName() + ".run() should not be " + + "called directly; use " + ConnectionValidator.class.getName() + + ".addConnectionListener() instead."); + } + } + TimerTask tt = new WaitOnConnectionCheckTimerTask(); try @@ -297,7 +316,10 @@ if (tieToLease && client.getLeasePeriod() > 0) { - if (trace) log.trace(this + " sending PING tied to lease"); + if (trace) + { + log.trace(this + " sending PING tied to lease"); + } isValid = doCheckConnectionWithLease(); } else @@ -513,7 +535,7 @@ try { Map metadata = new HashMap(); - metadata.put(ServerInvoker.INVOKER_SESSION_ID, this.invokerSessionId); + metadata.put(ServerInvoker.INVOKER_SESSION_ID, invokerSessionId); InvocationRequest ir = new InvocationRequest(null, Subsystem.SELF, "$PING$", metadata, null, null); @@ -571,13 +593,16 @@ synchronized(lock) { if (stopped) + { return false; + } if (!listeners.isEmpty()) { listeners.clear(); } stopped = true; + timer = null; } if (clientInvoker != null) @@ -627,7 +652,10 @@ { int elapsed = (int) (System.currentTimeMillis() - start); int wait = pingTimeout - elapsed; - if (wait <= 0) break; + if (wait <= 0) + { + break; + } try { Added: remoting2/branches/2.x/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java =================================================================== --- remoting2/branches/2.x/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java (rev 0) +++ remoting2/branches/2.x/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java 2008-11-13 06:55:17 UTC (rev 4673) @@ -0,0 +1,78 @@ +package org.jboss.test.remoting; + +import java.io.IOException; +import java.net.MalformedURLException; +import java.util.Map; + +import junit.framework.TestCase; + +import org.jboss.remoting.Client; +import org.jboss.remoting.ConnectionFailedException; +import org.jboss.remoting.ConnectionValidator; +import org.jboss.remoting.InvokerLocator; +import org.jboss.remoting.MicroRemoteClientInvoker; +import org.jboss.remoting.marshal.Marshaller; +import org.jboss.remoting.marshal.UnMarshaller; +import org.jboss.remoting.transport.ClientInvoker; + +public class ConnectionValidatorTest extends TestCase +{ + + public void testShouldDisallowDirectRun() + { + ConnectionValidator cv = new ConnectionValidator(new Client() { + public Map getConfiguration() + { + return null; + } + + public ClientInvoker getInvoker() + { + try + { + return new MicroRemoteClientInvoker( + new InvokerLocator("http://dummy:65535/dummy/")) { + + public String getSessionId() + { + return "dummyId"; + } + + protected String getDefaultDataType() + { + throw new UnsupportedOperationException(); + } + + protected void handleConnect() throws ConnectionFailedException + { + throw new UnsupportedOperationException(); + } + + protected void handleDisconnect() + { + throw new UnsupportedOperationException(); + } + + protected Object transport(String sessionId, Object invocation, Map metadata, Marshaller marshaller, + UnMarshaller unmarshaller) throws IOException, ConnectionFailedException, ClassNotFoundException + { + throw new UnsupportedOperationException(); + } + }; + } + catch (MalformedURLException e) + { + throw new RuntimeException(e); + } + } + }); + + try + { + cv.run(); + fail("Should throw IllegalStateException"); + } catch (IllegalStateException e) { + // Expected + } + } +} Property changes on: remoting2/branches/2.x/src/tests/org/jboss/test/remoting/ConnectionValidatorTest.java ___________________________________________________________________ Name: svn:keywords + Rev Date Name: svn:eol-style + native