From 357980202b8a54a061c479e3804149c53f86ca84 Mon Sep 17 00:00:00 2001 From: Jesus Perez Date: Mon, 29 Jan 2024 16:02:07 +0100 Subject: [PATCH 1/4] Refs #20262: Added non-throwing asio calls Signed-off-by: Jesus Perez --- src/cpp/rtps/transport/TCPChannelResource.h | 6 ++++++ .../rtps/transport/TCPChannelResourceBasic.cpp | 13 ++++++++++++- src/cpp/rtps/transport/TCPChannelResourceBasic.h | 7 +++++++ .../rtps/transport/TCPChannelResourceSecure.cpp | 12 ++++++++++++ .../rtps/transport/TCPChannelResourceSecure.h | 7 +++++++ src/cpp/rtps/transport/TCPTransportInterface.cpp | 11 ++++++++++- .../transport/mock/MockTCPChannelResource.cpp | 16 ++++++++++++++++ .../transport/mock/MockTCPChannelResource.h | 6 ++++++ 8 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/transport/TCPChannelResource.h b/src/cpp/rtps/transport/TCPChannelResource.h index 4bceb696bf2..784c256f08c 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.h +++ b/src/cpp/rtps/transport/TCPChannelResource.h @@ -133,6 +133,12 @@ class TCPChannelResource : public ChannelResource virtual asio::ip::tcp::endpoint remote_endpoint() const = 0; virtual asio::ip::tcp::endpoint local_endpoint() const = 0; + + virtual asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const = 0; + + virtual asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const = 0; virtual void set_options( const TCPTransportDescriptor* options) = 0; diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp index bb4aac5702f..05b9d53a2c5 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp @@ -181,7 +181,18 @@ asio::ip::tcp::endpoint TCPChannelResourceBasic::remote_endpoint() const asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint() const { - std::error_code ec; + return socket_->local_endpoint(); +} + +asio::ip::tcp::endpoint TCPChannelResourceBasic::remote_endpoint( + asio::error_code& ec) const +{ + return socket_->remote_endpoint(ec); +} + +asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint( + asio::error_code& ec) const +{ return socket_->local_endpoint(ec); } diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.h b/src/cpp/rtps/transport/TCPChannelResourceBasic.h index 5e7940663e1..fe73c4b657b 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.h +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.h @@ -65,9 +65,16 @@ class TCPChannelResourceBasic : public TCPChannelResource size_t size, asio::error_code& ec) override; + // Non-throwing asio calls asio::ip::tcp::endpoint remote_endpoint() const override; asio::ip::tcp::endpoint local_endpoint() const override; + // Throwing asio calls + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp index 34eac24a617..deddd7ffd31 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp @@ -265,6 +265,18 @@ asio::ip::tcp::endpoint TCPChannelResourceSecure::local_endpoint() const return secure_socket_->lowest_layer().local_endpoint(); } +asio::ip::tcp::endpoint TCPChannelResourceSecure::remote_endpoint( + asio::error_code& ec) const +{ + return secure_socket_->lowest_layer().remote_endpoint(ec); +} + +asio::ip::tcp::endpoint TCPChannelResourceSecure::local_endpoint( + asio::error_code& ec) const +{ + return secure_socket_->lowest_layer().local_endpoint(ec); +} + void TCPChannelResourceSecure::set_options( const TCPTransportDescriptor* options) { diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.h b/src/cpp/rtps/transport/TCPChannelResourceSecure.h index 04ac935c437..21f02c5122e 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.h +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.h @@ -65,9 +65,16 @@ class TCPChannelResourceSecure : public TCPChannelResource size_t size, asio::error_code& ec) override; + // Throwing asio calls asio::ip::tcp::endpoint remote_endpoint() const override; asio::ip::tcp::endpoint local_endpoint() const override; + // Non-throwing asio calls + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 231764e87b6..337d63fe5e8 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -898,7 +898,12 @@ void TCPTransportInterface::perform_listen_operation( if (rtcp_message_manager) { channel = channel_weak.lock(); - endpoint_to_locator(channel->remote_endpoint(), remote_locator); + asio::error_code ec; + endpoint_to_locator(channel->remote_endpoint(ec), remote_locator); + if (ec) + { + remote_locator.kind = LOCATOR_KIND_INVALID; + } if (channel) { @@ -1178,6 +1183,10 @@ bool TCPTransportInterface::Receive( } else { + if (!IsLocatorValid(remote_locator)) + { + endpoint_to_locator(channel->remote_endpoint(), remote_locator); + } IPLocator::setLogicalPort(remote_locator, tcp_header.logical_port); EPROSIMA_LOG_INFO(RTCP_MSG_IN, "[RECEIVE] From: " << remote_locator \ << " - " << receive_buffer_size << " bytes."); diff --git a/test/unittest/transport/mock/MockTCPChannelResource.cpp b/test/unittest/transport/mock/MockTCPChannelResource.cpp index db80e81982c..c22ad835f0e 100644 --- a/test/unittest/transport/mock/MockTCPChannelResource.cpp +++ b/test/unittest/transport/mock/MockTCPChannelResource.cpp @@ -67,6 +67,22 @@ asio::ip::tcp::endpoint MockTCPChannelResource::local_endpoint() const return ep; } +asio::ip::tcp::endpoint MockTCPChannelResource::remote_endpoint( + asio::error_code& ec) const +{ + ec = asio::error_code(); // Indicate no error + asio::ip::tcp::endpoint ep; + return ep; +} + +asio::ip::tcp::endpoint MockTCPChannelResource::local_endpoint( + asio::error_code& ec) const +{ + ec = asio::error_code(); // Indicate no error + asio::ip::tcp::endpoint ep; + return ep; +} + void MockTCPChannelResource::set_options( const TCPTransportDescriptor*) { diff --git a/test/unittest/transport/mock/MockTCPChannelResource.h b/test/unittest/transport/mock/MockTCPChannelResource.h index b48bf087ddb..ecdb186a0fc 100644 --- a/test/unittest/transport/mock/MockTCPChannelResource.h +++ b/test/unittest/transport/mock/MockTCPChannelResource.h @@ -59,6 +59,12 @@ class MockTCPChannelResource : public TCPChannelResource asio::ip::tcp::endpoint local_endpoint() const override; + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override; From bb9774e367fe02f071bce44b4a6bbabbc99d535a Mon Sep 17 00:00:00 2001 From: Jesus Perez Date: Mon, 29 Jan 2024 16:02:59 +0100 Subject: [PATCH 2/4] Refs #20262: Uncrustify Signed-off-by: Jesus Perez --- src/cpp/rtps/transport/TCPChannelResource.h | 2 +- src/cpp/rtps/transport/TCPChannelResourceBasic.cpp | 4 ++-- src/cpp/rtps/transport/TCPChannelResourceBasic.h | 4 ++-- src/cpp/rtps/transport/TCPChannelResourceSecure.h | 4 ++-- src/cpp/rtps/transport/TCPTransportInterface.cpp | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cpp/rtps/transport/TCPChannelResource.h b/src/cpp/rtps/transport/TCPChannelResource.h index 784c256f08c..3443e5d211c 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.h +++ b/src/cpp/rtps/transport/TCPChannelResource.h @@ -133,7 +133,7 @@ class TCPChannelResource : public ChannelResource virtual asio::ip::tcp::endpoint remote_endpoint() const = 0; virtual asio::ip::tcp::endpoint local_endpoint() const = 0; - + virtual asio::ip::tcp::endpoint remote_endpoint( asio::error_code& ec) const = 0; diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp index 05b9d53a2c5..feae2784bf9 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp @@ -185,13 +185,13 @@ asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint() const } asio::ip::tcp::endpoint TCPChannelResourceBasic::remote_endpoint( - asio::error_code& ec) const + asio::error_code& ec) const { return socket_->remote_endpoint(ec); } asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint( - asio::error_code& ec) const + asio::error_code& ec) const { return socket_->local_endpoint(ec); } diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.h b/src/cpp/rtps/transport/TCPChannelResourceBasic.h index fe73c4b657b..e0de0da0c6e 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.h +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.h @@ -71,9 +71,9 @@ class TCPChannelResourceBasic : public TCPChannelResource // Throwing asio calls asio::ip::tcp::endpoint remote_endpoint( - asio::error_code& ec) const override; + asio::error_code& ec) const override; asio::ip::tcp::endpoint local_endpoint( - asio::error_code& ec) const override; + asio::error_code& ec) const override; void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.h b/src/cpp/rtps/transport/TCPChannelResourceSecure.h index 21f02c5122e..31226292b4b 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.h +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.h @@ -71,9 +71,9 @@ class TCPChannelResourceSecure : public TCPChannelResource // Non-throwing asio calls asio::ip::tcp::endpoint remote_endpoint( - asio::error_code& ec) const override; + asio::error_code& ec) const override; asio::ip::tcp::endpoint local_endpoint( - asio::error_code& ec) const override; + asio::error_code& ec) const override; void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 337d63fe5e8..65e8b333f02 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -1185,7 +1185,7 @@ bool TCPTransportInterface::Receive( { if (!IsLocatorValid(remote_locator)) { - endpoint_to_locator(channel->remote_endpoint(), remote_locator); + endpoint_to_locator(channel->remote_endpoint(), remote_locator); } IPLocator::setLogicalPort(remote_locator, tcp_header.logical_port); EPROSIMA_LOG_INFO(RTCP_MSG_IN, "[RECEIVE] From: " << remote_locator \ From 45b281a6ad3336d211fa7a2024e49971f5305aec Mon Sep 17 00:00:00 2001 From: Jesus Perez Date: Tue, 30 Jan 2024 10:39:55 +0100 Subject: [PATCH 3/4] Refs #20262: Apply suggestions Signed-off-by: Jesus Perez --- src/cpp/rtps/transport/TCPChannelResource.h | 20 ++++++++++++++++ .../rtps/transport/TCPChannelResourceBasic.h | 4 ++-- .../rtps/transport/TCPTransportInterface.cpp | 23 +++++++++++++------ .../rtps/transport/TCPTransportInterface.h | 6 +++++ 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/cpp/rtps/transport/TCPChannelResource.h b/src/cpp/rtps/transport/TCPChannelResource.h index 3443e5d211c..1f795c37c6e 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.h +++ b/src/cpp/rtps/transport/TCPChannelResource.h @@ -130,13 +130,33 @@ class TCPChannelResource : public ChannelResource size_t size, asio::error_code& ec) = 0; + /** + * @brief Gets the remote endpoint of the socket connection. + * @throws Exception on failure. + * @return asio::ip::tcp::endpoint of the remote endpoint. + */ virtual asio::ip::tcp::endpoint remote_endpoint() const = 0; + /** + * @brief Gets the local endpoint of the socket connection. + * @throws Exception on failure. + * @return asio::ip::tcp::endpoint of the local endpoint. + */ virtual asio::ip::tcp::endpoint local_endpoint() const = 0; + /** + * @brief Gets the remote endpoint, setting error code if any. + * @param ec Set to indicate what error occurred, if any. + * @return asio::ip::tcp::endpoint of the remote endpoint or returns a default-constructed endpoint object if an error occurred. + */ virtual asio::ip::tcp::endpoint remote_endpoint( asio::error_code& ec) const = 0; + /** + * @brief Gets the local endpoint, setting error code if any. + * @param ec Set to indicate what error occurred, if any. + * @return asio::ip::tcp::endpoint of the remote endpoint or returns a default-constructed endpoint object if an error occurred. + */ virtual asio::ip::tcp::endpoint local_endpoint( asio::error_code& ec) const = 0; diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.h b/src/cpp/rtps/transport/TCPChannelResourceBasic.h index e0de0da0c6e..20928f0f494 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.h +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.h @@ -65,11 +65,11 @@ class TCPChannelResourceBasic : public TCPChannelResource size_t size, asio::error_code& ec) override; - // Non-throwing asio calls + // Throwing asio calls asio::ip::tcp::endpoint remote_endpoint() const override; asio::ip::tcp::endpoint local_endpoint() const override; - // Throwing asio calls + // Non-throwing asio calls asio::ip::tcp::endpoint remote_endpoint( asio::error_code& ec) const override; asio::ip::tcp::endpoint local_endpoint( diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 65e8b333f02..49576f1c11a 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -260,6 +260,19 @@ void TCPTransportInterface::clean() } } +Locator TCPTransportInterface::remote_endpoint_to_locator( + const std::shared_ptr& channel) const +{ + Locator locator; + asio::error_code ec; + endpoint_to_locator(channel->remote_endpoint(ec), locator); + if (ec) + { + LOCATOR_INVALID(locator); + } + return locator; +} + void TCPTransportInterface::bind_socket( std::shared_ptr& channel) { @@ -898,12 +911,8 @@ void TCPTransportInterface::perform_listen_operation( if (rtcp_message_manager) { channel = channel_weak.lock(); - asio::error_code ec; - endpoint_to_locator(channel->remote_endpoint(ec), remote_locator); - if (ec) - { - remote_locator.kind = LOCATOR_KIND_INVALID; - } + + remote_locator = remote_endpoint_to_locator(channel); if (channel) { @@ -1185,7 +1194,7 @@ bool TCPTransportInterface::Receive( { if (!IsLocatorValid(remote_locator)) { - endpoint_to_locator(channel->remote_endpoint(), remote_locator); + remote_locator = remote_endpoint_to_locator(channel); } IPLocator::setLogicalPort(remote_locator, tcp_header.logical_port); EPROSIMA_LOG_INFO(RTCP_MSG_IN, "[RECEIVE] From: " << remote_locator \ diff --git a/src/cpp/rtps/transport/TCPTransportInterface.h b/src/cpp/rtps/transport/TCPTransportInterface.h index f0a2ac082a2..0667fcf81be 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.h +++ b/src/cpp/rtps/transport/TCPTransportInterface.h @@ -188,6 +188,12 @@ class TCPTransportInterface : public TransportInterface const asio::ip::tcp::endpoint& endpoint, Locator& locator) const = 0; + /** + * Converts a remote endpoint to a locator if possible. Otherwise, it sets an invalid locator. + */ + Locator remote_endpoint_to_locator( + const std::shared_ptr& channel) const; + /** * Shutdown method to close the connections of the transports. */ From fe0ae54b15bd14d3acfa8c1473835b4f932a005b Mon Sep 17 00:00:00 2001 From: Jesus Perez Date: Wed, 7 Feb 2024 08:19:39 +0100 Subject: [PATCH 4/4] Refs #20262: Apply suggestions Signed-off-by: Jesus Perez --- src/cpp/rtps/transport/TCPTransportInterface.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 49576f1c11a..9a63e99992e 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -912,10 +912,10 @@ void TCPTransportInterface::perform_listen_operation( { channel = channel_weak.lock(); - remote_locator = remote_endpoint_to_locator(channel); - if (channel) { + remote_locator = remote_endpoint_to_locator(channel); + uint32_t port = channel->local_endpoint().port(); set_name_to_current_thread("dds.tcp.%u", port);