From 58fea273c59e1487e104d2881a100670175534f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ferreira=20Gonz=C3=A1lez?= Date: Mon, 4 Dec 2023 15:37:15 +0100 Subject: [PATCH 1/2] Add Autofill port to TCP Transport (#4061) * Refs #20019: Blackbox Test Signed-off-by: cferreiragonz * Refs #20019: Unittest autofill port Signed-off-by: cferreiragonz * Refs #20019: TCP Autofill port Signed-off-by: cferreiragonz Rearrange update of port Signed-off-by: cferreiragonz * Refs #20019: Uncrustify Signed-off-by: cferreiragonz * Refs #20019: Add static_cast conversion Signed-off-by: cferreiragonz * Refs #20019: Apply revision changes Signed-off-by: cferreiragonz --------- Signed-off-by: cferreiragonz (cherry picked from commit 36f31dd53a6290ecd6e080a97300849a2b5598d7) # Conflicts: # src/cpp/rtps/transport/TCPAcceptor.cpp # test/blackbox/CMakeLists.txt # test/blackbox/common/BlackboxTestsTransportTCP.cpp --- src/cpp/rtps/transport/TCPAcceptor.cpp | 6 ++ .../rtps/transport/TCPTransportInterface.cpp | 17 +++-- .../rtps/transport/TCPTransportInterface.h | 2 +- src/cpp/rtps/transport/TCPv4Transport.cpp | 4 +- src/cpp/rtps/transport/TCPv6Transport.cpp | 4 +- test/blackbox/CMakeLists.txt | 7 ++ .../common/BlackboxTestsTransportTCP.cpp | 72 ++++++++++++++++++- test/unittest/transport/TCPv4Tests.cpp | 38 ++++++++++ test/unittest/transport/TCPv6Tests.cpp | 38 ++++++++++ 9 files changed, 175 insertions(+), 13 deletions(-) diff --git a/src/cpp/rtps/transport/TCPAcceptor.cpp b/src/cpp/rtps/transport/TCPAcceptor.cpp index 1531af4b6e5..a6d77061b03 100644 --- a/src/cpp/rtps/transport/TCPAcceptor.cpp +++ b/src/cpp/rtps/transport/TCPAcceptor.cpp @@ -31,6 +31,7 @@ TCPAcceptor::TCPAcceptor( , locator_(locator) , io_service_(&io_service) { + locator_.port = acceptor_.local_endpoint().port(); endpoint_ = asio::ip::tcp::endpoint(parent->generate_protocol(), IPLocator::getPhysicalPort(locator_)); } @@ -43,7 +44,12 @@ TCPAcceptor::TCPAcceptor( , locator_(locator) , io_service_(&io_service) { +<<<<<<< HEAD endpoint_ = asio::ip::tcp::endpoint(asio::ip::address_v4::from_string(interface), +======= + locator_.port = acceptor_.local_endpoint().port(); + endpoint_ = asio::ip::tcp::endpoint(asio::ip::address::from_string(interface), +>>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) IPLocator::getPhysicalPort(locator_)); } diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 26a03e3ecc8..651ba841c50 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -252,9 +252,10 @@ void TCPTransportInterface::calculate_crc( header.crc = crc; } -bool TCPTransportInterface::create_acceptor_socket( +uint16_t TCPTransportInterface::create_acceptor_socket( const Locator& locator) { + uint16_t final_port = 0; try { if (is_interface_whitelist_empty()) @@ -264,16 +265,18 @@ bool TCPTransportInterface::create_acceptor_socket( { std::shared_ptr acceptor = std::make_shared(io_service_, this, locator); - acceptors_[locator] = acceptor; + acceptors_[acceptor->locator()] = acceptor; acceptor->accept(this, ssl_context_); + final_port = static_cast(acceptor->locator().port); } else #endif // if TLS_FOUND { std::shared_ptr acceptor = std::make_shared(io_service_, this, locator); - acceptors_[locator] = acceptor; + acceptors_[acceptor->locator()] = acceptor; acceptor->accept(this); + final_port = static_cast(acceptor->locator().port); } logInfo(RTCP, " OpenAndBindInput (physical: " << IPLocator::getPhysicalPort(locator) << "; logical: " @@ -290,16 +293,18 @@ bool TCPTransportInterface::create_acceptor_socket( { std::shared_ptr acceptor = std::make_shared(io_service_, sInterface, locator); - acceptors_[locator] = acceptor; + acceptors_[acceptor->locator()] = acceptor; acceptor->accept(this, ssl_context_); + final_port = static_cast(acceptor->locator().port); } else #endif // if TLS_FOUND { std::shared_ptr acceptor = std::make_shared(io_service_, sInterface, locator); - acceptors_[locator] = acceptor; + acceptors_[acceptor->locator()] = acceptor; acceptor->accept(this); + final_port = static_cast(acceptor->locator().port); } logInfo(RTCP, " OpenAndBindInput (physical: " << IPLocator::getPhysicalPort(locator) << "; logical: " @@ -322,7 +327,7 @@ bool TCPTransportInterface::create_acceptor_socket( return false; } - return true; + return final_port; } void TCPTransportInterface::fill_rtcp_header( diff --git a/src/cpp/rtps/transport/TCPTransportInterface.h b/src/cpp/rtps/transport/TCPTransportInterface.h index 780047782bb..1388f079c64 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.h +++ b/src/cpp/rtps/transport/TCPTransportInterface.h @@ -136,7 +136,7 @@ class TCPTransportInterface : public TransportInterface std::shared_ptr& channel); //! Creates a TCP acceptor to wait for incoming connections by the given locator. - bool create_acceptor_socket( + uint16_t create_acceptor_socket( const Locator& locator); virtual void get_ips( diff --git a/src/cpp/rtps/transport/TCPv4Transport.cpp b/src/cpp/rtps/transport/TCPv4Transport.cpp index e07e9c282d8..07779d638f2 100644 --- a/src/cpp/rtps/transport/TCPv4Transport.cpp +++ b/src/cpp/rtps/transport/TCPv4Transport.cpp @@ -81,10 +81,10 @@ TCPv4Transport::TCPv4Transport( interface_whitelist_.emplace_back(ip::address_v4::from_string(interface)); } - for (uint16_t port : configuration_.listening_ports) + for (uint16_t& port : configuration_.listening_ports) { Locator locator(LOCATOR_KIND_TCPv4, port); - create_acceptor_socket(locator); + port = create_acceptor_socket(locator); } #if !TLS_FOUND diff --git a/src/cpp/rtps/transport/TCPv6Transport.cpp b/src/cpp/rtps/transport/TCPv6Transport.cpp index abdbec6a5e9..73edb226611 100644 --- a/src/cpp/rtps/transport/TCPv6Transport.cpp +++ b/src/cpp/rtps/transport/TCPv6Transport.cpp @@ -85,10 +85,10 @@ TCPv6Transport::TCPv6Transport( interface_whitelist_.emplace_back(ip::address_v6::from_string(interface)); } - for (uint16_t port : configuration_.listening_ports) + for (uint16_t& port : configuration_.listening_ports) { Locator locator(LOCATOR_KIND_TCPv6, port); - create_acceptor_socket(locator); + port = create_acceptor_socket(locator); } #if !TLS_FOUND diff --git a/test/blackbox/CMakeLists.txt b/test/blackbox/CMakeLists.txt index 39646737f8f..e711652f496 100644 --- a/test/blackbox/CMakeLists.txt +++ b/test/blackbox/CMakeLists.txt @@ -234,6 +234,13 @@ if(NOT LibP11_FOUND) endif() # GTEST_INDIVIDUAL endif() # LibP11_FOUND +<<<<<<< HEAD +======= +if(EPROSIMA_TEST_DNS_NOT_SET_UP) + set(dns_filter "-Discovery.ServerClientEnvironmentSetUpDNS") +endif() + +>>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) file(GLOB RTPS_BLACKBOXTESTS_TEST_SOURCE "common/RTPSBlackboxTests*.cpp") set(RTPS_BLACKBOXTESTS_SOURCE ${RTPS_BLACKBOXTESTS_TEST_SOURCE} types/HelloWorld.cxx diff --git a/test/blackbox/common/BlackboxTestsTransportTCP.cpp b/test/blackbox/common/BlackboxTestsTransportTCP.cpp index f104dbd22e3..e6db1e7a351 100644 --- a/test/blackbox/common/BlackboxTestsTransportTCP.cpp +++ b/test/blackbox/common/BlackboxTestsTransportTCP.cpp @@ -21,6 +21,14 @@ #include #include +<<<<<<< HEAD +======= +#include "TCPReqRepHelloWorldRequester.hpp" +#include "TCPReqRepHelloWorldReplier.hpp" +#include "PubSubReader.hpp" +#include "PubSubWriter.hpp" + +>>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) using namespace eprosima::fastrtps; using namespace eprosima::fastrtps::rtps; @@ -526,7 +534,7 @@ TEST_P(TransportTCP, TCPv6_equal_operator) // Test copy constructor and copy assignment for TCPv6 TEST_P(TransportTCP, TCPv6_copy) { - // Change some varibles in order to check the non default cretion + // Change some varibles in order to check the non default creation TCPv6TransportDescriptor tcpv6_transport; tcpv6_transport.enable_tcp_nodelay = !tcpv6_transport.enable_tcp_nodelay; // change default value tcpv6_transport.max_logical_port = tcpv6_transport.max_logical_port + 10; // change default value @@ -602,6 +610,66 @@ TEST(TransportTCP, Client_reconnection) delete requester; } +// Test copy constructor and copy assignment for TCPv4 +TEST_P(TransportTCP, TCPv4_autofill_port) +{ + PubSubReader p1(TEST_TOPIC_NAME); + PubSubReader p2(TEST_TOPIC_NAME); + + // Add TCP Transport with listening port 0 + auto p1_transport = std::make_shared(); + p1_transport->add_listener_port(0); + p1.disable_builtin_transport().add_user_transport_to_pparams(p1_transport); + p1.init(); + ASSERT_TRUE(p1.isInitialized()); + + // Add TCP Transport with listening port different from 0 + uint16_t port = 12345; + auto p2_transport = std::make_shared(); + p2_transport->add_listener_port(port); + p2.disable_builtin_transport().add_user_transport_to_pparams(p2_transport); + p2.init(); + ASSERT_TRUE(p2.isInitialized()); + + LocatorList_t p1_locators; + p1.get_native_reader().get_listening_locators(p1_locators); + EXPECT_TRUE(IPLocator::getPhysicalPort(p1_locators.begin()[0]) != 0); + + LocatorList_t p2_locators; + p2.get_native_reader().get_listening_locators(p2_locators); + EXPECT_TRUE(IPLocator::getPhysicalPort(p2_locators.begin()[0]) == port); +} + +// Test copy constructor and copy assignment for TCPv6 +TEST_P(TransportTCP, TCPv6_autofill_port) +{ + PubSubReader p1(TEST_TOPIC_NAME); + PubSubReader p2(TEST_TOPIC_NAME); + + // Add TCP Transport with listening port 0 + auto p1_transport = std::make_shared(); + p1_transport->add_listener_port(0); + p1.disable_builtin_transport().add_user_transport_to_pparams(p1_transport); + p1.init(); + ASSERT_TRUE(p1.isInitialized()); + + // Add TCP Transport with listening port different from 0 + uint16_t port = 12345; + auto p2_transport = std::make_shared(); + p2_transport->add_listener_port(port); + p2.disable_builtin_transport().add_user_transport_to_pparams(p2_transport); + p2.init(); + ASSERT_TRUE(p2.isInitialized()); + + LocatorList_t p1_locators; + p1.get_native_reader().get_listening_locators(p1_locators); + EXPECT_TRUE(IPLocator::getPhysicalPort(p1_locators.begin()[0]) != 0); + + LocatorList_t p2_locators; + p2.get_native_reader().get_listening_locators(p2_locators); + EXPECT_TRUE(IPLocator::getPhysicalPort(p2_locators.begin()[0]) == port); +} + #ifdef INSTANTIATE_TEST_SUITE_P #define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w) #else @@ -622,4 +690,4 @@ GTEST_INSTANTIATE_TEST_MACRO(TransportTCP, return "Transport" + suffix; } - }); + }); \ No newline at end of file diff --git a/test/unittest/transport/TCPv4Tests.cpp b/test/unittest/transport/TCPv4Tests.cpp index 1f976605b77..a6155ef2033 100644 --- a/test/unittest/transport/TCPv4Tests.cpp +++ b/test/unittest/transport/TCPv4Tests.cpp @@ -1508,6 +1508,44 @@ TEST_F(TCPv4Tests, header_read_interrumption) thread.join(); } +// This test verifies that the autofill port feature correctly sets an automatic port when +// the descriptors's port is set to 0. +TEST_F(TCPv4Tests, autofill_port) +{ + // Check normal port assignation + TCPv4TransportDescriptor test_descriptor; + test_descriptor.add_listener_port(g_default_port); + TCPv4Transport transportUnderTest(test_descriptor); + transportUnderTest.init(); + + EXPECT_TRUE(transportUnderTest.configuration()->listening_ports[0] == g_default_port); + + // Check default port assignation + TCPv4TransportDescriptor test_descriptor_autofill; + test_descriptor_autofill.add_listener_port(0); + TCPv4Transport transportUnderTest_autofill(test_descriptor_autofill); + transportUnderTest_autofill.init(); + + EXPECT_TRUE(transportUnderTest_autofill.configuration()->listening_ports[0] != 0); + EXPECT_TRUE(transportUnderTest_autofill.configuration()->listening_ports.size() == 1); + + uint16_t port = 12345; + TCPv4TransportDescriptor test_descriptor_multiple_autofill; + test_descriptor_multiple_autofill.add_listener_port(0); + test_descriptor_multiple_autofill.add_listener_port(port); + test_descriptor_multiple_autofill.add_listener_port(0); + TCPv4Transport transportUnderTest_multiple_autofill(test_descriptor_multiple_autofill); + transportUnderTest_multiple_autofill.init(); + + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[0] != 0); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[1] == port); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[2] != 0); + EXPECT_TRUE( + transportUnderTest_multiple_autofill.configuration()->listening_ports[0] != + transportUnderTest_multiple_autofill.configuration()->listening_ports[2]); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports.size() == 3); +} + void TCPv4Tests::HELPER_SetDescriptorDefaults() { descriptor.add_listener_port(g_default_port); diff --git a/test/unittest/transport/TCPv6Tests.cpp b/test/unittest/transport/TCPv6Tests.cpp index 1c3d786c3ca..318481548c3 100644 --- a/test/unittest/transport/TCPv6Tests.cpp +++ b/test/unittest/transport/TCPv6Tests.cpp @@ -165,6 +165,44 @@ TEST_F(TCPv6Tests, opening_and_closing_input_channel) ASSERT_FALSE (transportUnderTest.IsInputChannelOpen(multicastFilterLocator)); ASSERT_FALSE (transportUnderTest.CloseInputChannel(multicastFilterLocator)); } + +// This test verifies that the autofill port feature correctly sets an automatic port when +// the descriptors's port is set to 0. +TEST_F(TCPv6Tests, autofill_port) +{ + // Check normal port assignation + TCPv6TransportDescriptor test_descriptor; + test_descriptor.add_listener_port(g_default_port); + TCPv6Transport transportUnderTest(test_descriptor); + transportUnderTest.init(); + + EXPECT_TRUE(transportUnderTest.configuration()->listening_ports[0] == g_default_port); + + // Check default port assignation + TCPv6TransportDescriptor test_descriptor_autofill; + test_descriptor_autofill.add_listener_port(0); + TCPv6Transport transportUnderTest_autofill(test_descriptor_autofill); + transportUnderTest_autofill.init(); + + EXPECT_TRUE(transportUnderTest_autofill.configuration()->listening_ports[0] != 0); + EXPECT_TRUE(transportUnderTest_autofill.configuration()->listening_ports.size() == 1); + + uint16_t port = 12345; + TCPv6TransportDescriptor test_descriptor_multiple_autofill; + test_descriptor_multiple_autofill.add_listener_port(0); + test_descriptor_multiple_autofill.add_listener_port(port); + test_descriptor_multiple_autofill.add_listener_port(0); + TCPv6Transport transportUnderTest_multiple_autofill(test_descriptor_multiple_autofill); + transportUnderTest_multiple_autofill.init(); + + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[0] != 0); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[1] == port); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports[2] != 0); + EXPECT_TRUE( + transportUnderTest_multiple_autofill.configuration()->listening_ports[0] != + transportUnderTest_multiple_autofill.configuration()->listening_ports[2]); + EXPECT_TRUE(transportUnderTest_multiple_autofill.configuration()->listening_ports.size() == 3); +} /* TEST_F(TCPv6Tests, send_and_receive_between_both_secure_ports) { From c165147580fc942db516bef930cf737717dd6559 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Tue, 5 Dec 2023 16:02:19 +0100 Subject: [PATCH 2/2] Fix Conflicts Signed-off-by: cferreiragonz --- src/cpp/rtps/transport/TCPAcceptor.cpp | 6 +----- test/blackbox/CMakeLists.txt | 9 +-------- test/blackbox/common/BlackboxTestsTransportTCP.cpp | 10 ++-------- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/src/cpp/rtps/transport/TCPAcceptor.cpp b/src/cpp/rtps/transport/TCPAcceptor.cpp index a6d77061b03..5c6d31596cc 100644 --- a/src/cpp/rtps/transport/TCPAcceptor.cpp +++ b/src/cpp/rtps/transport/TCPAcceptor.cpp @@ -44,12 +44,8 @@ TCPAcceptor::TCPAcceptor( , locator_(locator) , io_service_(&io_service) { -<<<<<<< HEAD - endpoint_ = asio::ip::tcp::endpoint(asio::ip::address_v4::from_string(interface), -======= locator_.port = acceptor_.local_endpoint().port(); - endpoint_ = asio::ip::tcp::endpoint(asio::ip::address::from_string(interface), ->>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) + endpoint_ = asio::ip::tcp::endpoint(asio::ip::address_v4::from_string(interface), IPLocator::getPhysicalPort(locator_)); } diff --git a/test/blackbox/CMakeLists.txt b/test/blackbox/CMakeLists.txt index e711652f496..0475b6fabf2 100644 --- a/test/blackbox/CMakeLists.txt +++ b/test/blackbox/CMakeLists.txt @@ -234,13 +234,6 @@ if(NOT LibP11_FOUND) endif() # GTEST_INDIVIDUAL endif() # LibP11_FOUND -<<<<<<< HEAD -======= -if(EPROSIMA_TEST_DNS_NOT_SET_UP) - set(dns_filter "-Discovery.ServerClientEnvironmentSetUpDNS") -endif() - ->>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) file(GLOB RTPS_BLACKBOXTESTS_TEST_SOURCE "common/RTPSBlackboxTests*.cpp") set(RTPS_BLACKBOXTESTS_SOURCE ${RTPS_BLACKBOXTESTS_TEST_SOURCE} types/HelloWorld.cxx @@ -369,7 +362,7 @@ if(FASTRTPS_API_TESTS) GTest::gtest $<$:eProsima_p11> # $ ) - + add_blackbox_gtest(BlackboxTests_FastRTPS SOURCES ${BLACKBOXTESTS_TEST_SOURCE} ENVIRONMENTS "CERTS_PATH=${PROJECT_SOURCE_DIR}/test/certs" "TOPIC_RANDOM_NUMBER=${TOPIC_RANDOM_NUMBER}" diff --git a/test/blackbox/common/BlackboxTestsTransportTCP.cpp b/test/blackbox/common/BlackboxTestsTransportTCP.cpp index e6db1e7a351..efb19cae3fd 100644 --- a/test/blackbox/common/BlackboxTestsTransportTCP.cpp +++ b/test/blackbox/common/BlackboxTestsTransportTCP.cpp @@ -15,20 +15,14 @@ #include "BlackboxTests.hpp" #include "TCPReqRepHelloWorldRequester.hpp" #include "TCPReqRepHelloWorldReplier.hpp" +#include "PubSubReader.hpp" +#include "PubSubWriter.hpp" #include #include #include -<<<<<<< HEAD -======= -#include "TCPReqRepHelloWorldRequester.hpp" -#include "TCPReqRepHelloWorldReplier.hpp" -#include "PubSubReader.hpp" -#include "PubSubWriter.hpp" - ->>>>>>> 36f31dd53 (Add Autofill port to TCP Transport (#4061)) using namespace eprosima::fastrtps; using namespace eprosima::fastrtps::rtps;