qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rob Godfrey (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (QPID-7981) [Java Broker] [AMQP1.0] Protocol layer assumes source and target are not null.
Date Mon, 23 Oct 2017 20:14:00 GMT

    [ https://issues.apache.org/jira/browse/QPID-7981?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16215760#comment-16215760
] 

Rob Godfrey commented on QPID-7981:
-----------------------------------

{quote}
I am aware the protocol is symmetric, however the spec states: "If there is no pre-existing
terminus, and the peer does not wish to create a new one, this is indicated by setting the
local terminus (source or target as appropriate) to null." That wording only makes sense in
the case that the link was initiated by the peer, i.e. where the local process is 'responding'
to a 'request' from the peer.
{quote}
Correct, the specific case where it makes sense for a peer to send null as the *local* terminus
is where it is acting in response to an attach it has received.
{quote}
As the spec says: "a peer MUST take responsibility for verifying that the remote terminus
meets its requirements" and that makes perfect sense to me. The distinction between empty
and null does not. If I ask for a remote target with address Q, whether the peer responds
with either null or empty (or even with a target with address P as you suggest) doesn't seem
important, neither matches my 'requirements'.
{quote}
It is correct that a peer MUST always verify - though I'll be honest and wonder how many actually
do :-).  The fundamental difference that can be inferred from receiving null rather than empty
or P from your partner is that null indicates that they got your attach and could not service
it.  Receiving a non null terminus implies that either the request was sent spontaneously
by your partner (unlikely if the relationship is client<->broker), or that the remote
container considers this the resumption of an existing link where there is already a terminus
associated.
{quote}
As the spec is defined, it never makes sense for the initiator of a link to set the local
terminus to null (rather than empty), so the case of simultaneous initiation of a link by
both peers needs to be detected by actually comparing fields of the remote terminus as the
peer sees it with local expectations, not by relying on it being null. The only way it will
be null is if it is indeed a response to a request. 
{quote}
Correct.  To guard against simultaneous link attachment (or unexpected link resumption) you
*always* need to verify the remote terminus.  
{quote}
That is the only 'very specific meaning' of null, as opposed to empty.
{quote}
It is the only meaning *when speaking of the local terminus*.  
When used for the remote source/target it is the difference between "i don't remember/I don't
care what the remote terminus looks like" (null) and "I specifically want the empty terminus"
(empty).
{quote}
However, having re-read the spec on these points I can see that my 'interpretation' was based
on what I think is sensible rather than on what the spec actually states (though as a minor
gripe on formatting, the diagrams use the character '-' to indicate both an 'empty' target
(figure 2.36) and a 'null' target (figure 2.33) which doesn't help to convey the apparently
important semantic distinction between them).
{quote}
I agree on the gripe with the formatting.  I'll raise an AMQP JIRA for this, and the need
to include more examples of correct (and incorrect) patterns around link attachment.

> [Java Broker] [AMQP1.0] Protocol layer assumes source and target are not null.
> ------------------------------------------------------------------------------
>
>                 Key: QPID-7981
>                 URL: https://issues.apache.org/jira/browse/QPID-7981
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>            Reporter: Keith Wall
>             Fix For: qpid-java-broker-7.0.0
>
>
> Testing the Broker against Rhea's {{example/helloworld.js}} shows that the AMQP 1.0 protocol
layer fails in at least two places.
> The first is if the {{Target}} (not mandatory) is omitted from the incoming attach.
> {noformat}
> 2017-10-21 18:13:20,609 DEBUG [IO-/10.211.55.13:52033] (o.a.q.s.p.frame) - RECV[/10.211.55.13:52033|0]
: Attach{name=7e65e0a0-961f-e941-b42b-262380143f76,handle=0,role=receiver,source=Source{address=examples}}
> 2017-10-21 18:13:20,622 DEBUG [IO-/10.211.55.13:52033] (o.a.q.s.p.v.LinkImpl) - Error
attaching link
> java.lang.NullPointerException: null
> 	at org.apache.qpid.server.protocol.v1_0.SendingLinkEndpoint.createConsumerTarget(SendingLinkEndpoint.java:209)
> 	at org.apache.qpid.server.protocol.v1_0.SendingLinkEndpoint.attachReceived(SendingLinkEndpoint.java:636)
> 	at org.apache.qpid.server.protocol.v1_0.SendingLinkEndpoint.establishLink(SendingLinkEndpoint.java:352)
> 	at org.apache.qpid.server.protocol.v1_0.AbstractLinkEndpoint.receiveAttach(AbstractLinkEndpoint.java:131)
> 	at org.apache.qpid.server.protocol.v1_0.LinkImpl.attach(LinkImpl.java:106)
> 	at org.apache.qpid.server.protocol.v1_0.Session_1_0.receiveAttach(Session_1_0.java:210)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.receiveAttach(AMQPConnection_1_0Impl.java:435)
> 	at org.apache.qpid.server.protocol.v1_0.type.transport.Attach.invoke(Attach.java:366)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.received(AMQPConnection_1_0Impl.java:517)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.lambda$receive$0(AMQPConnection_1_0Impl.java:469)
> 	at java.security.AccessController.doPrivileged(Native Method)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.receive(AMQPConnection_1_0Impl.java:463)
> 	at org.apache.qpid.server.protocol.v1_0.framing.FrameHandler.parse(FrameHandler.java:211)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.lambda$received$11(AMQPConnection_1_0Impl.java:1316)
> 	at java.security.AccessController.doPrivileged(Native Method)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.received(AMQPConnection_1_0Impl.java:1291)
> 	at org.apache.qpid.server.transport.MultiVersionProtocolEngine.received(MultiVersionProtocolEngine.java:134)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.processAmqpData(NonBlockingConnection.java:606)
> 	at org.apache.qpid.server.transport.NonBlockingConnectionTLSDelegate.processData(NonBlockingConnectionTLSDelegate.java:136)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.doRead(NonBlockingConnection.java:496)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.doWork(NonBlockingConnection.java:270)
> 	at org.apache.qpid.server.transport.NetworkConnectionScheduler.processConnection(NetworkConnectionScheduler.java:134)
> 	at org.apache.qpid.server.transport.SelectorThread$ConnectionProcessor.processConnection(SelectorThread.java:563)
> 	at org.apache.qpid.server.transport.SelectorThread$SelectionTask.performSelect(SelectorThread.java:354)
> 	at org.apache.qpid.server.transport.SelectorThread$SelectionTask.run(SelectorThread.java:97)
> 	at org.apache.qpid.server.transport.SelectorThread.run(SelectorThread.java:521)
> 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> 	at org.apache.qpid.server.bytebuffer.QpidByteBuffer.lambda$null$0(QpidByteBuffer.java:1396)
> 	at java.lang.Thread.run(Thread.java:748)
> {noformat}
> The second is if the {{Source}}  (again not mandatory) is omitted:
> {noformat}
> 2017-10-21 18:19:52,556 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.v.f.FrameHandler) -
RECV 89 bytes
> 2017-10-21 18:19:52,557 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.frame) - RECV[/10.211.55.13:52035|0]
: Attach{name=c2c65769-7d3b-5440-b8fb-f00badbddd0d,handle=1,role=sender,target=Target{address=examples},initialDeliveryCount=0}
> 2017-10-21 18:19:52,562 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.frame) - SEND[/10.211.55.13:52035|0]
: Attach{name=c2c65769-7d3b-5440-b8fb-f00badbddd0d,handle=1,role=receiver,target=Target{address=examples,durable=none,capabilities=[REJECT_UNROUTABLE,
DELAYED_DELIVERY]},unsettled={},maxMessageSize=1000000000,offeredCapabilities=[REJECT_UNROUTABLE,
DELAYED_DELIVERY],properties={}}
> 2017-10-21 18:19:52,565 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.frame) - SEND[/10.211.55.13:52035|0]
: Flow{nextIncomingId=0,incomingWindow=8192,nextOutgoingId=0,outgoingWindow=2048,handle=1,deliveryCount=0,linkCredit=20000,echo=false}
> 2017-10-21 18:19:52,566 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.v.f.FrameHandler) -
RECV 0 bytes
> 2017-10-21 18:19:52,566 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.t.NonBlockingConnection)
- Written 430 bytes
> 2017-10-21 18:19:52,568 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.t.NonBlockingConnection)
- Read 0 byte(s)
> 2017-10-21 18:19:52,570 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.t.NonBlockingConnection)
- Read 82 byte(s)
> 2017-10-21 18:19:52,571 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.v.f.FrameHandler) -
RECV 53 bytes
> 2017-10-21 18:19:52,571 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.frame) - RECV[/10.211.55.13:52035|0]
: Transfer{handle=1,deliveryId=0,deliveryTag=\x00,messageFormat=0,settled=false}
> 2017-10-21 18:19:52,580 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.p.QpidServiceLoader)
- Found 3 implementations of interface org.apache.qpid.server.plugin.MessageFormat
> 2017-10-21 18:19:52,604 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.m.AbstractConfiguredObject)
- authorise returned DEFER
> 2017-10-21 18:19:52,604 DEBUG [IO-/10.211.55.13:52035] (o.a.q.s.m.AbstractConfiguredObject)
- authorise returned DEFER, returing default: ALLOWED
> 2017-10-21 18:19:52,608 WARN  [IO-/10.211.55.13:52035] (o.a.q.s.p.v.f.FrameHandler) -
Unexpected exception handling frame
> java.lang.NullPointerException: null
> 	at org.apache.qpid.server.protocol.v1_0.StandardReceivingLinkEndpoint.receiveDelivery(StandardReceivingLinkEndpoint.java:262)
> 	at org.apache.qpid.server.protocol.v1_0.AbstractReceivingLinkEndpoint.receiveTransfer(AbstractReceivingLinkEndpoint.java:163)
> 	at org.apache.qpid.server.protocol.v1_0.Session_1_0.receiveTransfer(Session_1_0.java:572)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.receiveTransfer(AMQPConnection_1_0Impl.java:794)
> 	at org.apache.qpid.server.protocol.v1_0.type.transport.Transfer.invoke(Transfer.java:295)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.received(AMQPConnection_1_0Impl.java:517)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.lambda$receive$0(AMQPConnection_1_0Impl.java:469)
> 	at java.security.AccessController.doPrivileged(Native Method)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.receive(AMQPConnection_1_0Impl.java:463)
> 	at org.apache.qpid.server.protocol.v1_0.framing.FrameHandler.parse(FrameHandler.java:211)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.lambda$received$11(AMQPConnection_1_0Impl.java:1316)
> 	at java.security.AccessController.doPrivileged(Native Method)
> 	at org.apache.qpid.server.protocol.v1_0.AMQPConnection_1_0Impl.received(AMQPConnection_1_0Impl.java:1291)
> 	at org.apache.qpid.server.transport.MultiVersionProtocolEngine.received(MultiVersionProtocolEngine.java:134)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.processAmqpData(NonBlockingConnection.java:606)
> 	at org.apache.qpid.server.transport.NonBlockingConnectionTLSDelegate.processData(NonBlockingConnectionTLSDelegate.java:136)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.doRead(NonBlockingConnection.java:496)
> 	at org.apache.qpid.server.transport.NonBlockingConnection.doWork(NonBlockingConnection.java:270)
> 	at org.apache.qpid.server.transport.NetworkConnectionScheduler.processConnection(NetworkConnectionScheduler.java:134)
> 	at org.apache.qpid.server.transport.SelectorThread$ConnectionProcessor.processConnection(SelectorThread.java:563)
> 	at org.apache.qpid.server.transport.SelectorThread$SelectionTask.performSelect(SelectorThread.java:354)
> 	at org.apache.qpid.server.transport.SelectorThread$SelectionTask.run(SelectorThread.java:97)
> 	at org.apache.qpid.server.transport.SelectorThread.run(SelectorThread.java:521)
> 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> 	at org.apache.qpid.server.bytebuffer.QpidByteBuffer.lambda$null$0(QpidByteBuffer.java:1396)
> 	at java.lang.Thread.run(Thread.java:748)
> {noformat}
> Default source and target to empty objects appears to resolve the problem.
> The protocol tests should be strengthen to cover these cases/



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message