From ac132020078ea98b0421edd9d8ba315561904e5b Mon Sep 17 00:00:00 2001 From: James Strachan Date: Fri, 5 May 2006 07:31:35 +0000 Subject: [PATCH 1/3] improved null handling after suggestions from Danielius Jurna git-svn-id: https://svn.apache.org/repos/asf/incubator/activemq/trunk@399999 13f79535-47bb-0310-9956-ffa450edef68 --- .../transport/stomp/CommandParser.java | 19 +++++---- .../transport/stomp/HeaderParser.java | 40 ++++++++++++------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/CommandParser.java b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/CommandParser.java index dba54c0d69..f0f56c9341 100644 --- a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/CommandParser.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/CommandParser.java @@ -34,15 +34,20 @@ class CommandParser { } Command parse(DataInput in) throws IOException, JMSException { - String line; - + String line = null; + // skip white space to next real line - try { - while ((line = in.readLine()).trim().length() == 0) { + while (true) { + line = in.readLine(); + if (line == null) { + throw new IOException("connection was closed"); + } + else { + line = line.trim(); + if (line.length() > 0) { + break; + } } - } - catch (NullPointerException e) { - throw new IOException("connection was closed"); } // figure correct command and return it diff --git a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/HeaderParser.java b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/HeaderParser.java index acc07efc23..480cce2488 100644 --- a/activemq-core/src/main/java/org/apache/activemq/transport/stomp/HeaderParser.java +++ b/activemq-core/src/main/java/org/apache/activemq/transport/stomp/HeaderParser.java @@ -32,28 +32,38 @@ class HeaderParser { */ Properties parse(BufferedReader in) throws IOException { Properties props = new Properties(); - String line; - while (((line = in.readLine()).trim().length() > 0)) { - int seperator_index = line.indexOf(Stomp.Headers.SEPERATOR); - String name = line.substring(0, seperator_index).trim(); - String value = line.substring(seperator_index + 1, line.length()).trim(); - props.setProperty(name, value); + while (true) { + String line = in.readLine(); + if (line != null && line.trim().length() > 0) { + int seperator_index = line.indexOf(Stomp.Headers.SEPERATOR); + String name = line.substring(0, seperator_index).trim(); + String value = line.substring(seperator_index + 1, line.length()).trim(); + props.setProperty(name, value); + } + else { + break; + } } return props; } Properties parse(DataInput in) throws IOException { Properties props = new Properties(); - String line; - while (((line = in.readLine()).trim().length() > 0)) { - try { - int seperator_index = line.indexOf(Stomp.Headers.SEPERATOR); - String name = line.substring(0, seperator_index).trim(); - String value = line.substring(seperator_index + 1, line.length()).trim(); - props.setProperty(name, value); + while (true) { + String line = in.readLine(); + if (line != null && line.trim().length() > 0) { + try { + int seperator_index = line.indexOf(Stomp.Headers.SEPERATOR); + String name = line.substring(0, seperator_index).trim(); + String value = line.substring(seperator_index + 1, line.length()).trim(); + props.setProperty(name, value); + } + catch (Exception e) { + throw new ProtocolException("Unable to parser header line [" + line + "]"); + } } - catch (Exception e) { - throw new ProtocolException("Unable to parser header line [" + line + "]"); + else { + break; } } return props; From 89d6fd0a802d9b58306503dcd1d52c8b15ed169d Mon Sep 17 00:00:00 2001 From: James Strachan Date: Fri, 5 May 2006 12:12:23 +0000 Subject: [PATCH 2/3] avoid NPE git-svn-id: https://svn.apache.org/repos/asf/incubator/activemq/trunk@400057 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/activemq/command/PartialCommand.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/activemq-core/src/main/java/org/apache/activemq/command/PartialCommand.java b/activemq-core/src/main/java/org/apache/activemq/command/PartialCommand.java index 1db19a5c26..dfdc8c981a 100644 --- a/activemq-core/src/main/java/org/apache/activemq/command/PartialCommand.java +++ b/activemq-core/src/main/java/org/apache/activemq/command/PartialCommand.java @@ -130,7 +130,11 @@ public class PartialCommand implements Command { } public String toString() { - return "PartialCommand[id: " + commandId + " data: " + data.length + " byte(s)]"; + int size = 0; + if (data != null) { + size = data.length; + } + return "PartialCommand[id: " + commandId + " data: " + size + " byte(s)]"; } From 9614aa9b4edd5c24ee0bffa623388f976961fa94 Mon Sep 17 00:00:00 2001 From: "Hiram R. Chirino" Date: Mon, 8 May 2006 22:41:08 +0000 Subject: [PATCH 3/3] Connection state tracking would cause a CCE if temp destinations were in use. git-svn-id: https://svn.apache.org/repos/asf/incubator/activemq/trunk@405206 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/activemq/broker/AbstractConnection.java | 9 ++++----- .../org/apache/activemq/state/ConnectionState.java | 13 ++++++++++--- .../activemq/state/ConnectionStateTracker.java | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/activemq-core/src/main/java/org/apache/activemq/broker/AbstractConnection.java b/activemq-core/src/main/java/org/apache/activemq/broker/AbstractConnection.java index 1dea0f54e2..69a0c832ee 100755 --- a/activemq-core/src/main/java/org/apache/activemq/broker/AbstractConnection.java +++ b/activemq-core/src/main/java/org/apache/activemq/broker/AbstractConnection.java @@ -25,7 +25,6 @@ import java.util.List; import org.apache.activemq.Service; import org.apache.activemq.broker.region.ConnectionStatistics; -import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.BrokerInfo; import org.apache.activemq.command.Command; import org.apache.activemq.command.ConnectionControl; @@ -372,7 +371,7 @@ public abstract class AbstractConnection implements Service, Connection, Task, C ConnectionState cs = lookupConnectionState(info.getConnectionId()); broker.addDestinationInfo(cs.getContext(), info); if( info.getDestination().isTemporary() ) { - cs.addTempDestination(info.getDestination()); + cs.addTempDestination(info); } return null; } @@ -532,11 +531,11 @@ public abstract class AbstractConnection implements Service, Connection, Task, C // Cascade the connection stop to temp destinations. for (Iterator iter = cs.getTempDesinations().iterator(); iter.hasNext();) { - ActiveMQDestination dest = (ActiveMQDestination) iter.next(); + DestinationInfo di = (DestinationInfo) iter.next(); try{ - broker.removeDestination(cs.getContext(), dest, 0); + broker.removeDestination(cs.getContext(), di.getDestination(), 0); }catch(Throwable e){ - serviceLog.warn("Failed to remove tmp destination " + dest,e); + serviceLog.warn("Failed to remove tmp destination " + di.getDestination(), e); } iter.remove(); } diff --git a/activemq-core/src/main/java/org/apache/activemq/state/ConnectionState.java b/activemq-core/src/main/java/org/apache/activemq/state/ConnectionState.java index 9e8c26e335..61c20d15cc 100755 --- a/activemq-core/src/main/java/org/apache/activemq/state/ConnectionState.java +++ b/activemq-core/src/main/java/org/apache/activemq/state/ConnectionState.java @@ -20,11 +20,13 @@ package org.apache.activemq.state; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Set; import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.ConnectionInfo; +import org.apache.activemq.command.DestinationInfo; import org.apache.activemq.command.SessionId; import org.apache.activemq.command.SessionInfo; @@ -46,12 +48,17 @@ public class ConnectionState { return info.toString(); } - public void addTempDestination(ActiveMQDestination destination) { - tempDestinations.add(destination); + public void addTempDestination(DestinationInfo info) { + tempDestinations.add(info); } public void removeTempDestination(ActiveMQDestination destination) { - tempDestinations.remove(destination); + for (Iterator iter = tempDestinations.iterator(); iter.hasNext();) { + DestinationInfo di = (DestinationInfo) iter.next(); + if( di.getDestination().equals(destination) ) { + iter.remove(); + } + } } public void addSession(SessionInfo info) { diff --git a/activemq-core/src/main/java/org/apache/activemq/state/ConnectionStateTracker.java b/activemq-core/src/main/java/org/apache/activemq/state/ConnectionStateTracker.java index cb275c4945..edd0b6eab3 100755 --- a/activemq-core/src/main/java/org/apache/activemq/state/ConnectionStateTracker.java +++ b/activemq-core/src/main/java/org/apache/activemq/state/ConnectionStateTracker.java @@ -147,7 +147,7 @@ public class ConnectionStateTracker implements CommandVisitor { public Response processAddDestination(DestinationInfo info) throws Exception { ConnectionState cs = (ConnectionState) connectionStates.get(info.getConnectionId()); if( info.getDestination().isTemporary() ) { - cs.addTempDestination(info.getDestination()); + cs.addTempDestination(info); } return TRACKED_RESPONSE_MARKER; }