QPID-2158: add length validation into AMQShortString, remove dead code from AMQDestinations, truncate exception messages with length over 255 before sending them over the wire in AMQChannelException and AMQConnectionException.

Applied patch by Oleksandr Rudyy <orudyy@gmail.com>

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1133037 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 67 insertions, 38 deletions

diff --git a/java/common/src/main/java/org/apache/qpid/AMQChannelException.java b/java/common/src/main/java/org/apache/qpid/AMQChannelException.java
index ef9420ba87..2f6290b55a 100644
--- a/java/common/src/main/java/org/apache/qpid/AMQChannelException.java
+++ b/java/common/src/main/java/org/apache/qpid/AMQChannelException.java
@@ -54,6 +54,7 @@ public class AMQChannelException extends AMQException
     public AMQFrame getCloseFrame(int channel)
     {
         MethodRegistry reg = MethodRegistry.getMethodRegistry(new ProtocolVersion(major,minor));
-        return new AMQFrame(channel, reg.createChannelCloseBody(getErrorCode() == null ? AMQConstant.INTERNAL_ERROR.getCode() : getErrorCode().getCode(), new AMQShortString(getMessage()),_classId,_methodId));
+        return new AMQFrame(channel, reg.createChannelCloseBody(getErrorCode() == null ? AMQConstant.INTERNAL_ERROR.getCode() : getErrorCode().getCode(), getMessageAsShortString(),_classId,_methodId));
     }
+
 }
diff --git a/java/common/src/main/java/org/apache/qpid/AMQConnectionException.java b/java/common/src/main/java/org/apache/qpid/AMQConnectionException.java
index 8ef6facef1..ca9c9f9dc4 100644
--- a/java/common/src/main/java/org/apache/qpid/AMQConnectionException.java
+++ b/java/common/src/main/java/org/apache/qpid/AMQConnectionException.java
@@ -62,9 +62,10 @@ public class AMQConnectionException extends AMQException
         MethodRegistry reg = MethodRegistry.getMethodRegistry(new ProtocolVersion(major,minor));
         return new AMQFrame(0,
                             reg.createConnectionCloseBody(getErrorCode().getCode(),
-                                                          new AMQShortString(getMessage()),
+                                                          getMessageAsShortString(),
                                                           _classId,
                                                           _methodId));
 
     }
+
 }
diff --git a/java/common/src/main/java/org/apache/qpid/AMQException.java b/java/common/src/main/java/org/apache/qpid/AMQException.java
index b0c6fccc9e..86d439d269 100644
--- a/java/common/src/main/java/org/apache/qpid/AMQException.java
+++ b/java/common/src/main/java/org/apache/qpid/AMQException.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid;
 
+import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.protocol.AMQConstant;
 
 /**
@@ -121,4 +122,19 @@ public class AMQException extends Exception
 
         return newAMQE;
     }
+
+    /**
+     * Truncates the exception message to 255 characters if its length exceeds 255.
+     *
+     * @return exception message
+     */
+    public AMQShortString getMessageAsShortString()
+    {
+        String message = getMessage();
+        if (message != null && message.length() > AMQShortString.MAX_LENGTH)
+        {
+            message = message.substring(0, AMQShortString.MAX_LENGTH - 3) + "...";
+        }
+        return new AMQShortString(message);
+    }
 }
diff --git a/java/common/src/main/java/org/apache/qpid/framing/AMQShortString.java b/java/common/src/main/java/org/apache/qpid/framing/AMQShortString.java
index 39a9beb9e8..2b9e2ffaba 100644
--- a/java/common/src/main/java/org/apache/qpid/framing/AMQShortString.java
+++ b/java/common/src/main/java/org/apache/qpid/framing/AMQShortString.java
@@ -37,6 +37,10 @@ import java.lang.ref.WeakReference;
  */
 public final class AMQShortString implements CharSequence, Comparable<AMQShortString>
 {
+    /**
+     * The maximum number of octets in AMQ short string as defined in AMQP specification
+     */
+    public static final int MAX_LENGTH = 255;
     private static final byte MINUS = (byte)'-';
     private static final byte ZERO = (byte) '0';
 
@@ -118,22 +122,19 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
 
     public AMQShortString(byte[] data)
     {
-
+        if (data == null)
+        {
+            throw new NullPointerException("Cannot create AMQShortString with null data[]");
+        }
+        if (data.length > MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         _data = data.clone();
         _length = data.length;
         _offset = 0;
     }
 
-    public AMQShortString(byte[] data, int pos)
-    {
-        final int size = data[pos++];
-        final byte[] dataCopy = new byte[size];
-        System.arraycopy(data,pos,dataCopy,0,size);
-        _length = size;
-        _data = dataCopy;
-        _offset = 0;
-    }
-
     public AMQShortString(String data)
     {
         this((data == null) ? EMPTY_CHAR_ARRAY : data.toCharArray());
@@ -146,7 +147,12 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
         {
             throw new NullPointerException("Cannot create AMQShortString with null char[]");
         }
-
+        // the current implementation of 0.8/0.9.x short string encoding
+        // supports only ASCII characters
+        if (data.length> MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         final int length = data.length;
         final byte[] stringBytes = new byte[length];
         int hash = 0;
@@ -165,6 +171,17 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
 
     public AMQShortString(CharSequence charSequence)
     {
+        if (charSequence == null)
+        {
+            // it should be possible to create short string for null data
+            charSequence = "";
+        }
+        // the current implementation of 0.8/0.9.x short string encoding
+        // supports only ASCII characters
+        if (charSequence.length() > MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         final int length = charSequence.length();
         final byte[] stringBytes = new byte[length];
         int hash = 0;
@@ -184,6 +201,10 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
 
     private AMQShortString(ByteBuffer data, final int length)
     {
+        if (length > MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         if(data.isDirect() || data.isReadOnly())
         {
             byte[] dataBytes = new byte[length];
@@ -205,8 +226,17 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
 
     private AMQShortString(final byte[] data, final int from, final int to)
     {
+        if (data == null)
+        {
+            throw new NullPointerException("Cannot create AMQShortString with null data[]");
+        }
+        int length = to - from;
+        if (length > MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         _offset = from;
-        _length = to - from;
+        _length = length;
         _data = data;
     }
 
@@ -245,29 +275,6 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
         return new CharSubSequence(start, end);
     }
 
-    public int writeToByteArray(byte[] encoding, int pos)
-    {
-        final int size = length();
-        encoding[pos++] = (byte) size;
-        System.arraycopy(_data,_offset,encoding,pos,size);
-        return pos+size;
-    }
-
-    public static AMQShortString readFromByteArray(byte[] byteEncodedDestination, int pos)
-    {
-
-
-        final AMQShortString shortString = new AMQShortString(byteEncodedDestination, pos);
-        if(shortString.length() == 0)
-        {
-            return null;
-        }
-        else
-        {
-            return shortString;
-        }
-    }
-
     public static AMQShortString readFromBuffer(ByteBuffer buffer)
     {
         final short length = buffer.getUnsigned();
@@ -690,6 +697,10 @@ public final class AMQShortString implements CharSequence, Comparable<AMQShortSt
             size += term.length();
         }
 
+        if (size > MAX_LENGTH)
+        {
+            throw new IllegalArgumentException("Cannot create AMQShortString with number of octets over 255!");
+        }
         byte[] data = new byte[size];
         int pos = 0;
         final byte[] delimData = delim._data;