Skip to content

Commit

Permalink
Fixes skupperproject#1681: disallow link detach/reattach attempts
Browse files Browse the repository at this point in the history
  • Loading branch information
kgiusti committed Nov 25, 2024
1 parent b478fb5 commit 8d39541
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/adaptors/amqp/container.c
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ void qd_container_handle_event(qd_container_t *container, pn_event_t *event,
}
break;

case PN_LINK_REMOTE_DETACH :
case PN_LINK_REMOTE_CLOSE :
if (!(pn_connection_state(conn) & PN_LOCAL_CLOSED)) {
pn_link = pn_event_link(event);
Expand Down Expand Up @@ -642,14 +641,31 @@ void qd_container_handle_event(qd_container_t *container, pn_event_t *event,
}
break;

case PN_LINK_LOCAL_DETACH:
case PN_LINK_LOCAL_CLOSE:
pn_link = pn_event_link(event);
if (pn_link_state(pn_link) == (PN_LOCAL_CLOSED | PN_REMOTE_CLOSED)) {
add_link_to_free_list(&qd_conn->free_link_list, pn_link);
}
break;

case PN_LINK_REMOTE_DETACH:
case PN_LINK_LOCAL_DETACH:
// The router does not support detaching and reattaching links as described in the AMQP 1.0 Specification
// Section 2.6.4 Detaching And Reattaching A Link. If the remote attempts this it is an error - force the
// connection closed.
if (!(pn_connection_state(conn) & PN_LOCAL_CLOSED)) {
pn_condition_t *cond = pn_connection_condition(conn);
pn_condition_set_name(cond, QD_AMQP_COND_NOT_IMPLEMENTED);
pn_condition_set_description(cond, "Link detach/reattach not supported");
pn_connection_close(conn);

pn_link = pn_event_link(event);
qd_link = (qd_link_t*) pn_link_get_context(pn_link);
qd_log(LOG_CONTAINER, QD_LOG_ERROR,
"[C%"PRIu64"][L%"PRIu64"] Error: Link detach/reattach not supported, connection closed",
qd_conn->connection_id, qd_link ? qd_link->link_id : 0);
}
break;

case PN_LINK_FLOW :
pn_link = pn_event_link(event);
Expand Down
58 changes: 58 additions & 0 deletions tests/system_tests_one_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,16 @@ def test_50_extension_capabilities(self):
self.assertIn(symbol('ANONYMOUS-RELAY'), rc)
self.assertIn(symbol('qd.streaming-links'), rc)

def test_51_unsupported_link_reattach(self):
"""
The router does not support reattaching detached links. Ensure that
clients attempting to detach/reattach are detected and handled as an
connection error
"""
test = LinkReattachTest(self.address)
test.run()
self.assertIsNone(test.error)


class Entity:
def __init__(self, status_code, status_description, attrs):
Expand Down Expand Up @@ -3304,6 +3314,54 @@ def run(self):
sleep(0.1)


class LinkReattachTest(MessagingHandler):
"""
The router does not support link detach/re-attach. Attempt to detach a link
without closing it as if attempting to do a re-attach. This should cause
the router to force close the connection with error.
"""

def __init__(self, addr):
super(LinkReattachTest, self).__init__()
self.address = addr
self.error = None
self.conn = None
self.rx_link = None

def done(self):
if self.timer:
self.timer.cancel()
if self.conn:
self.conn.close()

def timeout(self):
self.error = "Timeout Expired"
self.done()

def on_start(self, event):
self.timer = event.reactor.schedule(TIMEOUT, TestTimeout(self))
self.conn = event.container.connect(self.address)
self.rx_link = event.container.create_receiver(self.conn,
source="test/reattach",
name="ReattachTest")

def on_link_opened(self, event):
if event.receiver == self.rx_link:
# Issue a non-close detach. This is the first step in the link
# reattach process
self.rx_link.detach()

def on_connection_error(self, event):
# Success if the router has force-closed due to the reattach attempt
desc = event.connection.remote_condition.description
if "reattach not supported" not in desc:
self.error = f"Unexpected error: {desc}"
self.done()

def run(self):
Container(self).run()


class DataConnectionCountTest(TestCase):
"""
Start the router with different numbers of worker threads and make sure
Expand Down

0 comments on commit 8d39541

Please sign in to comment.