Skip to content

Commit 7641f3d

Browse files
mergify[bot]juanlofer-eprosima
mergify[bot]
and
juanlofer-eprosima
authored
Allow domain names (DNS) when parsing TCP locators XML elements (#5429) (#5448)
* Refs #21506. Add regression tests * Refs #21506. Attempt DNS resolution when setting locator's IP * Refs #21506. Fix failing tests and apply suggestions * Refs #21506. Apply more suggestions --------- (cherry picked from commit 50c5848) Signed-off-by: Juan Lopez Fernandez <[email protected]> Co-authored-by: juanlofer-eprosima <[email protected]>
1 parent dc0c4b9 commit 7641f3d

File tree

5 files changed

+408
-60
lines changed

5 files changed

+408
-60
lines changed

‎src/cpp/rtps/xmlparser/XMLElementParser.cpp‎

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3176,24 +3176,11 @@ XMLP_ret XMLParser::getXMLLocatorUDPv4(
31763176
{
31773177
return XMLP_ret::XML_ERROR;
31783178
}
3179-
// Check whether the address is IPv4
3180-
if (!IPLocator::isIPv4(s))
3179+
if (!IPLocator::setIPv4(locator, s))
31813180
{
3182-
auto response = rtps::IPLocator::resolveNameDNS(s);
3183-
3184-
// Add the first valid IPv4 address that we can find
3185-
if (response.first.size() > 0)
3186-
{
3187-
s = response.first.begin()->data();
3188-
}
3189-
else
3190-
{
3191-
EPROSIMA_LOG_ERROR(XMLPARSER,
3192-
"DNS server did not return any IPv4 address for: '" << s << "'. Name: " << name);
3193-
return XMLP_ret::XML_ERROR;
3194-
}
3181+
EPROSIMA_LOG_ERROR(XMLPARSER, "Failed to parse UDPv4 locator's " << ADDRESS << " tag");
3182+
return XMLP_ret::XML_ERROR;
31953183
}
3196-
IPLocator::setIPv4(locator, s);
31973184
}
31983185
else
31993186
{
@@ -3249,24 +3236,11 @@ XMLP_ret XMLParser::getXMLLocatorUDPv6(
32493236
{
32503237
return XMLP_ret::XML_ERROR;
32513238
}
3252-
// Check whether the address is IPv6
3253-
if (!IPLocator::isIPv6(s))
3239+
if (!IPLocator::setIPv6(locator, s))
32543240
{
3255-
auto response = rtps::IPLocator::resolveNameDNS(s);
3256-
3257-
// Add the first valid IPv6 address that we can find
3258-
if (response.second.size() > 0)
3259-
{
3260-
s = response.second.begin()->data();
3261-
}
3262-
else
3263-
{
3264-
EPROSIMA_LOG_ERROR(XMLPARSER,
3265-
"DNS server did not return any IPv6 address for: '" << s << "'. Name: " << name);
3266-
return XMLP_ret::XML_ERROR;
3267-
}
3241+
EPROSIMA_LOG_ERROR(XMLPARSER, "Failed to parse UDPv6 locator's " << ADDRESS << " tag");
3242+
return XMLP_ret::XML_ERROR;
32683243
}
3269-
IPLocator::setIPv6(locator, s);
32703244
}
32713245
else
32723246
{
@@ -3337,7 +3311,11 @@ XMLP_ret XMLParser::getXMLLocatorTCPv4(
33373311
{
33383312
return XMLP_ret::XML_ERROR;
33393313
}
3340-
IPLocator::setIPv4(locator, s);
3314+
if (!IPLocator::setIPv4(locator, s))
3315+
{
3316+
EPROSIMA_LOG_ERROR(XMLPARSER, "Failed to parse TCPv4 locator's " << ADDRESS << " tag");
3317+
return XMLP_ret::XML_ERROR;
3318+
}
33413319
}
33423320
else if (strcmp(name, WAN_ADDRESS) == 0)
33433321
{
@@ -3347,7 +3325,11 @@ XMLP_ret XMLParser::getXMLLocatorTCPv4(
33473325
{
33483326
return XMLP_ret::XML_ERROR;
33493327
}
3350-
IPLocator::setWan(locator, s);
3328+
if (!IPLocator::setWan(locator, s))
3329+
{
3330+
EPROSIMA_LOG_ERROR(XMLPARSER, "Failed to parse TCPv4 locator's " << WAN_ADDRESS << " tag");
3331+
return XMLP_ret::XML_ERROR;
3332+
}
33513333
}
33523334
else if (strcmp(name, UNIQUE_LAN_ID) == 0)
33533335
{
@@ -3426,7 +3408,11 @@ XMLP_ret XMLParser::getXMLLocatorTCPv6(
34263408
{
34273409
return XMLP_ret::XML_ERROR;
34283410
}
3429-
IPLocator::setIPv6(locator, s);
3411+
if (!IPLocator::setIPv6(locator, s))
3412+
{
3413+
EPROSIMA_LOG_ERROR(XMLPARSER, "Failed to parse TCPv6 locator's " << ADDRESS << " tag");
3414+
return XMLP_ret::XML_ERROR;
3415+
}
34303416
}
34313417
else
34323418
{

‎src/cpp/utils/IPLocator.cpp‎

Lines changed: 95 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ namespace eprosima {
2929
namespace fastrtps {
3030
namespace rtps {
3131

32-
static const std::regex IPv4_REGEX("^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}"
33-
"(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$");
32+
static const std::regex IPv4_REGEX("^(?:(?:0*25[0-5]|0*2[0-4][0-9]|0*[01]?[0-9][0-9]?)\\.){3}"
33+
"(?:0*25[0-5]|0*2[0-4][0-9]|0*[01]?[0-9][0-9]?)$");
3434
static const std::regex IPv6_QUARTET_REGEX("^(?:[A-Fa-f0-9]){0,4}$");
3535

3636
// Factory
@@ -106,7 +106,31 @@ bool IPLocator::setIPv4(
106106
// This function do not set address to 0 in case it fails
107107
// Be careful, do not set all IP to 0 because WAN and LAN could be set beforehand
108108

109-
std::stringstream ss(ipv4);
109+
std::string s(ipv4);
110+
if (!IPLocator::isIPv4(s))
111+
{
112+
// Attempt DNS resolution
113+
auto response = IPLocator::resolveNameDNS(s);
114+
115+
// Use the first valid IPv4 address that we can find
116+
if (response.first.size() > 0)
117+
{
118+
s = response.first.begin()->data();
119+
// Redundant check for extra security (here a custom regex is used instead of asio's verification)
120+
if (!IPLocator::isIPv4(s))
121+
{
122+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "DNS name [" << ipv4 << "] resolved into wrong IPv4 format: " << s);
123+
return false;
124+
}
125+
}
126+
else
127+
{
128+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv4 " << s << " error format. Expected X.X.X.X or valid DNS name");
129+
return false;
130+
}
131+
}
132+
133+
std::stringstream ss(s);
110134
uint32_t a;
111135
uint32_t b;
112136
uint32_t c;
@@ -127,7 +151,7 @@ bool IPLocator::setIPv4(
127151
// If there are more info to read, it fails
128152
return ss.rdbuf()->in_avail() == 0;
129153
}
130-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv4 " << ipv4 << " error format. Expected X.X.X.X");
154+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv4 " << s << " error format. Expected X.X.X.X or valid DNS name");
131155
return false;
132156
}
133157

@@ -261,14 +285,33 @@ bool IPLocator::setIPv6(
261285
return false;
262286
}
263287

264-
if (!IPv6isCorrect(ipv6))
288+
std::string s(ipv6);
289+
if (!IPLocator::isIPv6(s))
265290
{
266-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " is not well defined");
267-
return false;
291+
// Attempt DNS resolution
292+
auto response = IPLocator::resolveNameDNS(s);
293+
294+
// Use the first valid IPv6 address that we can find
295+
if (response.second.size() > 0)
296+
{
297+
s = response.second.begin()->data();
298+
// Redundant check for extra security (here a custom regex is used instead of asio's verification)
299+
if (!IPLocator::isIPv6(s))
300+
{
301+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "DNS name [" << ipv6 << "] resolved into wrong IPv6 format: " << s);
302+
return false;
303+
}
304+
}
305+
else
306+
{
307+
EPROSIMA_LOG_WARNING(IP_LOCATOR,
308+
"IPv6 " << s << " error format. Expected well defined address or valid DNS name");
309+
return false;
310+
}
268311
}
269312

270313
LOCATOR_ADDRESS_INVALID(locator.address);
271-
uint16_t count = (uint16_t) std::count_if( ipv6.begin(), ipv6.end(), []( char c )
314+
uint16_t count = (uint16_t) std::count_if( s.begin(), s.end(), []( char c )
272315
{
273316
return c == ':';
274317
}); // C type cast to avoid Windows warnings
@@ -281,10 +324,10 @@ bool IPLocator::setIPv6(
281324
size_t aux_prev; // This must be size_t as string::npos could change value depending on size_t size
282325

283326
// Check whether is a zero block and where
284-
if (ipv6.front() == ':')
327+
if (s.front() == ':')
285328
{
286329
// First element equal : -> starts with zeros
287-
if (ipv6.back() == ':')
330+
if (s.back() == ':')
288331
{
289332
// Empty string (correct ipv6 format)
290333
initial_zeros = 16;
@@ -298,7 +341,7 @@ bool IPLocator::setIPv6(
298341
initial_zeros = (7 - (count - 2)) * 2;
299342
}
300343
}
301-
else if (ipv6.back() == ':')
344+
else if (s.back() == ':')
302345
{
303346
// Last element equal : -> ends with zeros
304347
// It does not start with :: (previous if)
@@ -307,8 +350,8 @@ bool IPLocator::setIPv6(
307350
else
308351
{
309352
// It does not starts or ends with zeros, but it could have :: in the middle or not have it
310-
aux_prev = ipv6.size(); // Aux could be 1 so this number must be unreacheable
311-
aux = ipv6.find(':'); // Index of first ':'
353+
aux_prev = s.size(); // Aux could be 1 so this number must be unreacheable
354+
aux = s.find(':'); // Index of first ':'
312355

313356
// Look for "::" will loop string twice
314357
// Therefore, we use this loop that will go over less or equal once
@@ -324,13 +367,13 @@ bool IPLocator::setIPv6(
324367
// Not "::" found, keep searching in next ':'
325368
position_zeros += 2; // It stores the point where the 0 block is
326369
aux_prev = aux;
327-
aux = ipv6.find(':', aux + 1);
370+
aux = s.find(':', aux + 1);
328371
}
329372
}
330373

331374
char punct;
332375
std::stringstream ss;
333-
ss << std::hex << ipv6;
376+
ss << std::hex << s;
334377
uint16_t i;
335378
uint32_t input_aux; // It cannot be uint16_t or we could not find whether the input number is bigger than allowed
336379

@@ -350,7 +393,7 @@ bool IPLocator::setIPv6(
350393
ss >> punct >> input_aux;
351394
if (input_aux >= 65536)
352395
{
353-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " has values higher than expected (65536)");
396+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << s << " has values higher than expected (65536)");
354397
return false;
355398
}
356399
locator.address[i++] = octet(input_aux >> 8);
@@ -371,7 +414,7 @@ bool IPLocator::setIPv6(
371414
ss >> input_aux >> punct;
372415
if (input_aux >= 65536)
373416
{
374-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " has values higher than expected (65536)");
417+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << s << " has values higher than expected (65536)");
375418
return false;
376419
}
377420
locator.address[i++] = octet(input_aux >> 8);
@@ -393,7 +436,7 @@ bool IPLocator::setIPv6(
393436
ss >> input_aux >> punct;
394437
if (input_aux >= 65536)
395438
{
396-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " has values higher than expected (65536)");
439+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << s << " has values higher than expected (65536)");
397440
return false;
398441
}
399442
locator.address[i++] = octet(input_aux >> 8);
@@ -410,7 +453,7 @@ bool IPLocator::setIPv6(
410453
ss >> punct >> input_aux;
411454
if (input_aux >= 65536)
412455
{
413-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " has values higher than expected (65536)");
456+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << s << " has values higher than expected (65536)");
414457
return false;
415458
}
416459
locator.address[i++] = octet(input_aux >> 8);
@@ -428,7 +471,7 @@ bool IPLocator::setIPv6(
428471
ss >> punct >> input_aux;
429472
if (input_aux >= 65536)
430473
{
431-
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << ipv6 << " has values higher than expected (65536)");
474+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv6 " << s << " has values higher than expected (65536)");
432475
return false;
433476
}
434477
locator.address[i++] = octet(input_aux >> 8);
@@ -678,7 +721,37 @@ bool IPLocator::setWan(
678721
Locator_t& locator,
679722
const std::string& wan)
680723
{
681-
std::stringstream ss(wan);
724+
if (locator.kind != LOCATOR_KIND_TCPv4)
725+
{
726+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "Trying to set WAN address in a non TCP-IPv4 Locator");
727+
return false;
728+
}
729+
730+
std::string s(wan);
731+
if (!IPLocator::isIPv4(s))
732+
{
733+
// Attempt DNS resolution
734+
auto response = IPLocator::resolveNameDNS(s);
735+
736+
// Use the first valid IPv4 address that we can find
737+
if (response.first.size() > 0)
738+
{
739+
s = response.first.begin()->data();
740+
// Redundant check for extra security (here a custom regex is used instead of asio's verification)
741+
if (!IPLocator::isIPv4(s))
742+
{
743+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "DNS name [" << wan << "] resolved into wrong IPv4 format: " << s);
744+
return false;
745+
}
746+
}
747+
else
748+
{
749+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv4 " << s << " error format. Expected X.X.X.X or valid DNS name");
750+
return false;
751+
}
752+
}
753+
754+
std::stringstream ss(s);
682755
int a, b, c, d; //to store the 4 ints
683756
char ch; //to temporarily store the '.'
684757

@@ -690,6 +763,7 @@ bool IPLocator::setWan(
690763
locator.address[11] = (octet)d;
691764
return true;
692765
}
766+
EPROSIMA_LOG_WARNING(IP_LOCATOR, "IPv4 " << s << " error format. Expected X.X.X.X or valid DNS name");
693767
return false;
694768
}
695769

‎test/unittest/utils/LocatorTests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class IPLocatorTests : public ::testing::Test
7575

7676
const std::string ipv4_any = "0.0.0.0";
7777
const std::string ipv4_invalid = "0.0.0.0";
78+
const std::string ipv4_invalid_format = "192.168.1.256.1";
7879
const std::string ipv6_any = "::";
7980
const std::string ipv6_invalid = "0:0:0:0:0:0:0:0";
8081

@@ -160,7 +161,6 @@ TEST_F(IPLocatorTests, setIPv4_from_string)
160161
{
161162
// Error cases
162163
ASSERT_FALSE(IPLocator::setIPv4(locator, "1.1.1.256")); // Too high number
163-
ASSERT_FALSE(IPLocator::setIPv4(locator, "1.1.1")); // Too few args
164164
ASSERT_FALSE(IPLocator::setIPv4(locator, "1.1.1.1.1")); // Too much args
165165

166166
// Change to IPv6
@@ -1348,8 +1348,8 @@ TEST_F(IPLocatorTests, setIPv4address)
13481348
}
13491349

13501350
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7", "9.10.11.12", "13.14.15.16"));
1351-
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7.8", "9.10.11", "13.14.15.16"));
1352-
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7.8", "9.10.11.12", "13.14.15"));
1351+
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7.8", ipv4_invalid_format, "13.14.15.16"));
1352+
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7.8", "9.10.11.12", ipv4_invalid_format));
13531353

13541354
locator.kind = LOCATOR_KIND_TCPv6;
13551355
ASSERT_FALSE(IPLocator::setIPv4address(locator, "1.2.3.4.5.6.7.8", "9.10.11.12", "13.14.15.16"));

‎test/unittest/xmlparser/CMakeLists.txt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,11 @@ target_link_libraries(XMLParserTests GTest::gtest
291291
if(QNX)
292292
target_link_libraries(XMLParserTests socket)
293293
endif()
294-
gtest_discover_tests(XMLParserTests)
294+
if(EPROSIMA_TEST_DNS_NOT_SET_UP)
295+
message(STATUS "Ignoring 'getXMLLocatorDNS*'")
296+
set(IGNORE_COMMAND "-getXMLLocatorDNS*")
297+
endif()
298+
gtest_discover_tests(XMLParserTests TEST_FILTER ${IGNORE_COMMAND})
295299
###################################### XMLParserTests ########################################################
296300

297301
####################################### XMLTreeTests #########################################################

0 commit comments

Comments
 (0)