You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2016/12/22 20:09:24 UTC
svn commit: r1775722 - in /qpid/java/trunk:
broker-core/src/main/java/org/apache/qpid/server/exchange/
broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/
test-profiles/python_tests/
Author: kwall
Date: Thu Dec 22 20:09:23 2016
New Revision: 1775722
URL: http://svn.apache.org/viewvc?rev=1775722&view=rev
Log:
QPID-6028: [Java Broker] Prevent NPE when unbinding from the fanout exchange.
Ensure the AbstractExchange interpretation of bindingKey is uniform across the whole API (that is we always treat a null bindingKey as the empty string).
Prevent 0-8..0-91 passing bindingKey "null" when unbinding with null binding key.
Fixes QPID-3987 too.
Modified:
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/FanoutExchangeImpl.java
qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
qpid/java/trunk/test-profiles/python_tests/JavaPre010PythonExcludes
Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java?rev=1775722&r1=1775721&r2=1775722&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/AbstractExchange.java Thu Dec 22 20:09:23 2016
@@ -281,8 +281,13 @@ public abstract class AbstractExchange<T
return _virtualHost;
}
+ @Override
public boolean isBound(String bindingKey, Map<String,Object> arguments, Queue<?> queue)
{
+ if (bindingKey == null)
+ {
+ bindingKey = "";
+ }
for(Binding b : _bindings)
{
if(bindingKey.equals(b.getBindingKey()) && queue.getName().equals(b.getDestination()))
@@ -295,8 +300,14 @@ public abstract class AbstractExchange<T
return false;
}
+ @Override
public boolean isBound(String bindingKey, Queue<?> queue)
{
+ if (bindingKey == null)
+ {
+ bindingKey = "";
+ }
+
for(Binding b : _bindings)
{
if(bindingKey.equals(b.getBindingKey()) && queue.getName().equals(b.getDestination()))
@@ -307,8 +318,14 @@ public abstract class AbstractExchange<T
return false;
}
+ @Override
public boolean isBound(String bindingKey)
{
+ if (bindingKey == null)
+ {
+ bindingKey = "";
+ }
+
for(Binding b : _bindings)
{
if(bindingKey.equals(b.getBindingKey()))
@@ -319,6 +336,7 @@ public abstract class AbstractExchange<T
return false;
}
+ @Override
public boolean isBound(Queue<?> queue)
{
for(Binding b : _bindings)
@@ -347,7 +365,7 @@ public abstract class AbstractExchange<T
return false;
}
-
+ @Override
public boolean isBound(Map<String, Object> arguments)
{
for(Binding b : _bindings)
@@ -366,6 +384,11 @@ public abstract class AbstractExchange<T
@Override
public boolean isBound(String bindingKey, Map<String, Object> arguments)
{
+ if (bindingKey == null)
+ {
+ bindingKey = "";
+ }
+
for(Binding b : _bindings)
{
if(b.getBindingKey().equals(bindingKey) &&
@@ -379,11 +402,13 @@ public abstract class AbstractExchange<T
return false;
}
+ @Override
public boolean hasBindings()
{
return !_bindings.isEmpty();
}
+ @Override
public Exchange<?> getAlternateExchange()
{
return _alternateExchange;
@@ -406,11 +431,13 @@ public abstract class AbstractExchange<T
}
}
+ @Override
public void removeReference(ExchangeReferrer exchange)
{
_referrers.remove(exchange);
}
+ @Override
public void addReference(ExchangeReferrer exchange)
{
_referrers.put(exchange, Boolean.TRUE);
@@ -434,11 +461,7 @@ public abstract class AbstractExchange<T
protected abstract void onUnbind(final BindingIdentifier binding);
- public Map<String, Object> getArguments()
- {
- return Collections.emptyMap();
- }
-
+ @Override
public long getBindingCount()
{
return getBindings().size();
@@ -485,6 +508,7 @@ public abstract class AbstractExchange<T
return queues;
}
+ @Override
public final <M extends ServerMessage<? extends StorableMessageMetaData>> int send(final M message,
final String routingAddress,
final InstanceProperties instanceProperties,
@@ -773,19 +797,6 @@ public abstract class AbstractExchange<T
bind(queue.getName(), bindingKey, arguments, true);
}
-
- private ListenableFuture<Void> autoDeleteIfNecessaryAsync()
- {
- if (isAutoDeletePending())
- {
- _logger.debug("Auto-deleting exchange: {}", this);
-
- return deleteAsync();
- }
-
- return Futures.immediateFuture(null);
- }
-
private boolean autoDeleteIfNecessary()
{
if (isAutoDeletePending())
@@ -899,8 +910,12 @@ public abstract class AbstractExchange<T
}
@Override
- public boolean hasBinding(final String bindingKey, final Queue<?> queue)
+ public boolean hasBinding(String bindingKey, final Queue<?> queue)
{
+ if (bindingKey == null)
+ {
+ bindingKey = "";
+ }
for (Binding b : _bindings)
{
if (b.getBindingKey().equals(bindingKey) && b.getDestination().equals(queue.getName()))
Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/FanoutExchangeImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/FanoutExchangeImpl.java?rev=1775722&r1=1775721&r2=1775722&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/FanoutExchangeImpl.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/exchange/FanoutExchangeImpl.java Thu Dec 22 20:09:23 2016
@@ -160,7 +160,7 @@ class FanoutExchangeImpl extends Abstrac
public BindingSet removeBinding(final BindingIdentifier binding)
{
Queue<?> queue = (Queue<?>) binding.getDestination();
- if(_filteredBindings.get(queue).containsKey(binding))
+ if(_filteredBindings.containsKey(queue) && _filteredBindings.get(queue).containsKey(binding))
{
final Map<Queue<?>, Map<BindingIdentifier, FilterManager>> filteredBindings = new HashMap<>(_filteredBindings);
final Map<BindingIdentifier, FilterManager> bindingsForQueue = new HashMap<>(filteredBindings.remove(queue));
Modified: qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java?rev=1775722&r1=1775721&r2=1775722&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java (original)
+++ qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java Thu Dec 22 20:09:23 2016
@@ -3082,7 +3082,7 @@ public class AMQChannel
@Override
public void receiveQueueBind(final AMQShortString queueName,
final AMQShortString exchange,
- AMQShortString routingKey,
+ AMQShortString bindingKey,
final boolean nowait,
final FieldTable argumentsTable)
{
@@ -3090,7 +3090,7 @@ public class AMQChannel
{
_logger.debug("RECV[" + _channelId + "] QueueBind[" +" queue: " + queueName +
" exchange: " + exchange +
- " bindingKey: " + routingKey +
+ " bindingKey: " + bindingKey +
" nowait: " + nowait + " arguments: " + argumentsTable + " ]");
}
@@ -3103,16 +3103,15 @@ public class AMQChannel
if (queue != null)
{
- if (routingKey == null)
+ if (bindingKey == null)
{
- routingKey = AMQShortString.valueOf(queue.getName());
+ bindingKey = AMQShortString.valueOf(queue.getName());
}
}
}
else
{
queue = getQueue(queueName.toString());
- routingKey = routingKey == null ? AMQShortString.EMPTY_STRING : routingKey;
}
if (queue == null)
@@ -3146,16 +3145,16 @@ public class AMQChannel
{
Map<String, Object> arguments = FieldTable.convertToMap(argumentsTable);
- String bindingKey = String.valueOf(routingKey);
+ String bindingKeyStr = AMQShortString.toString(bindingKey);
- if (!exch.isBound(bindingKey, arguments, queue))
+ if (!exch.isBound(bindingKeyStr, arguments, queue))
{
- if (!exch.addBinding(bindingKey, queue, arguments)
+ if (!exch.addBinding(bindingKeyStr, queue, arguments)
&& ExchangeDefaults.TOPIC_EXCHANGE_CLASS.equals(
exch.getType()))
{
- exch.replaceBinding(bindingKey, queue, arguments);
+ exch.replaceBinding(bindingKeyStr, queue, arguments);
}
}
@@ -3166,7 +3165,7 @@ public class AMQChannel
+ " to exchange "
+ exch
+ " with routing key "
- + routingKey);
+ + bindingKeyStr);
}
if (!nowait)
{
@@ -3547,14 +3546,13 @@ public class AMQChannel
}
else
{
-
final Exchange<?> exch = getExchange(exchange.toString());
if (exch == null)
{
closeChannel(ErrorCodes.NOT_FOUND, "Exchange '" + exchange + "' does not exist.");
}
- else if (!exch.hasBinding(String.valueOf(bindingKey), queue))
+ else if (!exch.hasBinding(AMQShortString.toString(bindingKey), queue))
{
closeChannel(ErrorCodes.NOT_FOUND, "No such binding");
}
@@ -3562,7 +3560,7 @@ public class AMQChannel
{
try
{
- exch.deleteBinding(String.valueOf(bindingKey), queue);
+ exch.deleteBinding(AMQShortString.toString(bindingKey), queue);
final AMQMethodBody responseBody = _connection.getMethodRegistry().createQueueUnbindOkBody();
sync();
Modified: qpid/java/trunk/test-profiles/python_tests/JavaPre010PythonExcludes
URL: http://svn.apache.org/viewvc/qpid/java/trunk/test-profiles/python_tests/JavaPre010PythonExcludes?rev=1775722&r1=1775721&r2=1775722&view=diff
==============================================================================
--- qpid/java/trunk/test-profiles/python_tests/JavaPre010PythonExcludes (original)
+++ qpid/java/trunk/test-profiles/python_tests/JavaPre010PythonExcludes Thu Dec 22 20:09:23 2016
@@ -29,10 +29,6 @@ qpid_tests.broker_0_9.query.QueryTests.t
qpid_tests.broker_0_9.query.QueryTests.test_binding_query_topic
qpid_tests.broker_0_9.query.QueryTests.test_exchange_query
-# QPID-3987 Tests broker_0_9.queue.QueueTests.test_unbind_fanout and broker_0_9.queue.QueueTests.test_unbind_headers fail with 404, 'No such binding'
-qpid_tests.broker_0_9.queue.QueueTests.test_unbind_fanout
-qpid_tests.broker_0_9.queue.QueueTests.test_unbind_headers
-
#QPID-4832 Apache Qpid Broker for Java does not support an explicit bind to a default exchange
qpid_tests.broker_0_8.exchange.DefaultExchangeRuleTests.testDefaultExchangeExplicitBind
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org