Author: kwall
Date: Fri Sep 23 09:43:18 2016
New Revision: 1762041
URL: http://svn.apache.org/viewvc?rev=1762041&view=rev
Log:
QPID-7366: [Java Broker] Pragmatically guard #publishMessage against
application header key/values that would cause message converter to fail.
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageConverter_Internal_to_v0_10.java
qpid/java/trunk/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/PublishMessageRestTest.java
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java?rev=1762041&r1=1762040&r2=1762041&view=diff
==============================================================================
---
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
(original)
+++
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
Fri Sep 23 09:43:18 2016
@@ -727,7 +727,8 @@ public abstract class AbstractVirtualHos
@Override
public int publishMessage(@Param(name = "message") final ManageableMessage
message)
{
- MessageDestination destination =
getAttainedMessageDestination(message.getAddress());
+ final String address = message.getAddress();
+ MessageDestination destination = address == null ?
getDefaultDestination() : getAttainedMessageDestination(address);
if(destination == null)
{
destination = getDefaultDestination();
@@ -767,6 +768,33 @@ public abstract class AbstractVirtualHos
throw new IllegalArgumentException("The message content (if
present) can only be a string, map or list");
}
}
+ if (message.getHeaders() != null)
+ {
+ for (Map.Entry<String, Object> entry :
message.getHeaders().entrySet())
+ {
+ final String key = entry.getKey();
+ final Object value = entry.getValue();
+ final int keyLength =
key.getBytes(StandardCharsets.UTF_8).length;
+ if (keyLength > 255)
+ {
+ // Disallow keys of more than 255 bytes. Converting for a
0-8..0-10 client would fail to tolerate
+ // key as it cannot be represented as shortstring/str8.
Ultimately is too restrictive for 1.0.
+ throw new IllegalArgumentException(String.format("Header
key '%s' is too long. Must be fewer than 256 bytes",
+ key));
+ }
+ if (keyLength == 0)
+ {
+ // Disallow empty keys. Converting for a 0-8..0-91 client
would fail within Broker. For 0-10, the client fails.
+ throw new IllegalArgumentException(String.format("Header
entries with empty keys unsupported", key));
+ }
+ if (value == null)
+ {
+ // Disallow null values. Converting for a 0-8..0-91 client
would fail within Broker. For 0-10, such
+ // properties are treated as if the property never existed.
+ throw new IllegalArgumentException(String.format("Header
key '%s' value of null is unsupported.", key));
+ }
+ }
+ }
InternalMessage internalMessage =
InternalMessage.createMessage(getMessageStore(), header, body,
message.isPersistent());
AutoCommitTransaction txn = new
AutoCommitTransaction(getMessageStore());
@@ -795,7 +823,7 @@ public abstract class AbstractVirtualHos
}
};
- return destination.send(internalMessage, message.getAddress(),
instanceProperties, txn, null);
+ return destination.send(internalMessage, address, instanceProperties,
txn, null);
}
Modified:
qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageConverter_Internal_to_v0_10.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageConverter_Internal_to_v0_10.java?rev=1762041&r1=1762040&r2=1762041&view=diff
==============================================================================
---
qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageConverter_Internal_to_v0_10.java
(original)
+++
qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageConverter_Internal_to_v0_10.java
Fri Sep 23 09:43:18 2016
@@ -122,11 +122,16 @@ public class MessageConverter_Internal_t
messageProps.setCorrelationId(serverMsg.getMessageHeader().getCorrelationId().getBytes());
}
messageProps.setApplicationHeaders(serverMsg.getMessageHeader().getHeaderMap());
- if (serverMsg.getMessageHeader().getMessageId() != null)
+ String messageIdAsString = serverMsg.getMessageHeader().getMessageId();
+ if (messageIdAsString != null)
{
try
{
-
messageProps.setMessageId(UUID.fromString(serverMsg.getMessageHeader().getMessageId()));
+ if (messageIdAsString.startsWith("ID:"))
+ {
+ messageIdAsString = messageIdAsString.substring(3);
+ }
+ messageProps.setMessageId(UUID.fromString(messageIdAsString));
}
catch (IllegalArgumentException iae)
{
Modified:
qpid/java/trunk/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java?rev=1762041&r1=1762040&r2=1762041&view=diff
==============================================================================
---
qpid/java/trunk/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
(original)
+++
qpid/java/trunk/broker-plugins/amqp-msg-conv-0-8-to-1-0/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
Fri Sep 23 09:43:18 2016
@@ -165,6 +165,9 @@ public class MessageConverter_0_8_to_1_0
applicationProperties = new LinkedHashMap<>(applicationProperties);
applicationProperties.remove("qpid.subject");
}
+ // TODO:
http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-application
+ // Adhere to "the values are restricted to be of simple types only,
that is, excluding map, list, and array types".
+ // 0-8..0-91 for instance suppoorted field tables with maps as values.
sections.add(new ApplicationProperties(applicationProperties));
if(bodySection != null)
{
Modified:
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/PublishMessageRestTest.java
URL:
http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/PublishMessageRestTest.java?rev=1762041&r1=1762040&r2=1762041&view=diff
==============================================================================
---
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/PublishMessageRestTest.java
(original)
+++
qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/rest/PublishMessageRestTest.java
Fri Sep 23 09:43:18 2016
@@ -19,6 +19,9 @@
*/
package org.apache.qpid.systest.rest;
+import static
org.apache.qpid.server.management.plugin.servlet.rest.AbstractServlet.SC_UNPROCESSABLE_ENTITY;
+
+import java.io.IOException;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
@@ -35,6 +38,8 @@ import javax.jms.Session;
import javax.jms.TextMessage;
import javax.servlet.http.HttpServletResponse;
+import com.google.common.base.Strings;
+
import org.apache.qpid.server.model.Port;
import org.apache.qpid.server.model.Queue;
import org.apache.qpid.server.model.port.HttpPort;
@@ -42,7 +47,6 @@ import org.apache.qpid.test.utils.TestBr
public class PublishMessageRestTest extends QpidRestTestCase
{
-
private Connection _connection;
private Session _session;
private String _queueName;
@@ -100,11 +104,9 @@ public class PublishMessageRestTest exte
final long tomorrow = TimeUnit.DAYS.toMillis(1) +
System.currentTimeMillis();
final Map<String, Object> headers = new HashMap<>();
headers.put("stringprop", "mystring");
+ headers.put("longstringprop", Strings.repeat("*", 256));
headers.put("intprop", Integer.MIN_VALUE);
headers.put("longprop", Long.MAX_VALUE);
- //headers.put("", "mykeyisempty"); // 0-8..0-91 Causes Broker to die
(MessageConverter_Internal_to_v0_8), 0-10 fails in JMS client on receipt
- //headers.put("nullpropvalue", null); // 0-8..0-91 Causes Broker
failure (MessageConverter_Internal_to_v0_8), 0-10 JMS client ignores property
-
final Map<String, Object> messageBody = new HashMap<>();
messageBody.put("messageId", messageId);
messageBody.put("address", _queueName);
@@ -132,6 +134,37 @@ public class PublishMessageRestTest exte
assertEquals("Unexpected number of properties", headers.size(), count);
}
+ public void testPublishMessageWithIllegalPropertyKeysAndValues() throws
Exception
+ {
+ {
+ final Map<String, Object> headers = new HashMap<>();
+ final String keytoolong = Strings.repeat("*", 256);
+ headers.put(keytoolong, "helloworld");
+ expectPublishFailure(headers, SC_UNPROCESSABLE_ENTITY);
+ }
+
+ {
+ final Map<String, Object> headers = new HashMap<>();
+ headers.put("", "emptykey");
+ expectPublishFailure(headers, SC_UNPROCESSABLE_ENTITY);
+ }
+
+ {
+ final Map<String, Object> headers = new HashMap<>();
+ headers.put("nullvalue", null);
+ expectPublishFailure(headers, SC_UNPROCESSABLE_ENTITY);
+ }
+ }
+
+ private void expectPublishFailure(final Map<String, Object> headers, final
int responseCode) throws IOException
+ {
+ final Map<String, Object> messageBody = Collections.<String,
Object>singletonMap("headers", headers);
+
+ getRestTestHelper().submitRequest(_publishMessageOpUrl, "POST",
+ Collections.singletonMap("message",
messageBody),
+ responseCode);
+ }
+
public void testPublishStringMessage() throws Exception
{
final String content = "Hello world";
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org
