From 9a1ef4425214dd79734097be66745ab397c49d56 Mon Sep 17 00:00:00 2001 From: Damiano Enerli Date: Tue, 2 Jan 2018 18:17:31 +0100 Subject: [PATCH 1/3] Added check on contact name before Port::open() and Network::connect() This commit add Contact::isValidRegistrationName() and Contact::isValidRegisteredName() functions as an attempt to standardize what YARP accepts as input for opening and connecting ports. --- src/libYARP_OS/include/yarp/os/Contact.h | 23 +++++++ src/libYARP_OS/src/Contact.cpp | 82 ++++++++++++++++++++++++ src/libYARP_OS/src/Network.cpp | 4 +- src/libYARP_OS/src/Port.cpp | 5 +- 4 files changed, 111 insertions(+), 3 deletions(-) diff --git a/src/libYARP_OS/include/yarp/os/Contact.h b/src/libYARP_OS/include/yarp/os/Contact.h index 05c48a87409..1d30388f780 100644 --- a/src/libYARP_OS/include/yarp/os/Contact.h +++ b/src/libYARP_OS/include/yarp/os/Contact.h @@ -128,6 +128,29 @@ class YARP_OS_API yarp::os::Contact { */ static Contact fromString(const ConstString& txt); + /** + * @brief isValidRegisteredName + * + * Checks if a provided name match the syntax of possibly + * registered port names. The syntax is defined into the function code. + * + * @param name char sequence with the name to check + * @return true if the name could correspond to a registered contact + */ + static bool isValidRegisteredName(const char* name); + + /** + * @brief isValidRegistrationName + * + * Checks if a provided name is valid to enter registration phase. + * Valid syntax includes valid registered names plus anonymous ports + * and plain char sequences for topics. + * + * @param name char sequence with the name to check + * @return true if the contact can be registered by YARP network + */ + static bool isValidRegistrationName(const char* name); + /** @} */ /** @{ */ diff --git a/src/libYARP_OS/src/Contact.cpp b/src/libYARP_OS/src/Contact.cpp index cb876276073..6b836bef5f1 100644 --- a/src/libYARP_OS/src/Contact.cpp +++ b/src/libYARP_OS/src/Contact.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #if defined(YARP_HAS_ACE) # include @@ -197,6 +198,87 @@ Contact Contact::fromString(const ConstString& txt) return c; } +bool Contact::isValidRegistrationName(const char* name) +{ + if(strcmp(name,"") == 0) // anonymous + return true; + + const std::string port_name_s(name); + std::regex port_exp("([a-zA-Z0-9]+(?:[_][a-zA-Z0-9]+)*)" // plain expression "abc[_def]" without leading '/', allowed for topics + "|" // OR + "(\\.\\.\\.)"); // "..." is accepted as registration name, a default one is assigned + + std::smatch port_match; + if (std::regex_match(port_name_s, port_match, port_exp)) + return true; + + return isValidRegisteredName(name); // otherwise, we need a good final name +} + +bool Contact::isValidRegisteredName(const char* name) +{ + const std::string port_name_s(name); + std::regex port_exp( + "(?:([a-zA-Z0-9]+(?:[_][a-zA-Z0-9]+)*):/)?" // carrier_name:/ - optional + "(" + "(" // /port[category]@/node~wire syntax + "((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)+)" // port_name - necessary + "(?:" + "([\\+|-][1]?)?" // category - optional, "+" | "-" | "+1" | "-1" + "(@((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)*))" + // @node_name - optional, could also be "" + "(~([_a-zA-Z0-9]+))?" // ~wire_type - optional + ")?" + ")" + "|" // OR + "(" // /node[category]#/port~wire syntax + "((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)+)" // node_name - necessary + "([\\+|-][1]?)?" // category - optional, "+" | "-" | "+1" | "-1" + "\x23" // # - necessary + "((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)+)" // /port_name - necessary + "(~([_a-zA-Z0-9]+))?" // ~wire_type - optional + ")" + "|" // OR + "(" // /node=[category]/port~wire syntax + "((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)+)" // /node_name - necessary + "=" // = - necessary + "([\\+|-][1]?)?" // category - optional, "+" | "-" | "+1" | "-1" + "((?:/[a-zA-Z0-9]+(?:[_:][a-zA-Z0-9]+)*)+)" // /port_name - necessary + "(~([_a-zA-Z0-9]+))?" // ~wire_type - optional + ")" + "|" // OR + "(/" // "/machine:NNN/" syntax + "[a-zA-Z0-9]+(?:\\.[a-zA-Z0-9]+)*" // machine name - "machine.domain.name" or IP addr + ":[\\d]+/?" // port number + ")" + ")"); + + // Matching + std::smatch port_match; + bool match = std::regex_match(port_name_s, port_match, port_exp); + + if (match) + { + return true; + } + else // basic error reporting: + { + std::cout << "\"" << port_name_s << "\" does not match the correct pattern for port definition.\n"; + std::regex_search(port_name_s, port_match, port_exp); + if (port_match[0].str() != "") + { + std::cout << "Check for the error in prefix or suffix of the matched name:\n"; + std::string prefix = (port_match.prefix().str().empty() ? "" : ("[" + port_match.prefix().str() + "] ")); + std::string suffix = (port_match.suffix().str().empty() ? "" : (" [" + port_match.suffix().str() + "]")); + std::cout << prefix << port_match[0].str() << suffix << std::endl; + } + else + { + std::cout << "There is no matching pattern. Check the correct syntax --> where <-- for port names.\n"; + } + return false; + } +} ConstString Contact::getName() const diff --git a/src/libYARP_OS/src/Network.cpp b/src/libYARP_OS/src/Network.cpp index d978df89032..c3d8682008f 100644 --- a/src/libYARP_OS/src/Network.cpp +++ b/src/libYARP_OS/src/Network.cpp @@ -354,12 +354,12 @@ static int metaConnect(const ConstString& src, CARRIER_DEBUG("DYNAMIC_SRC: name=%s, carrier=%s\n", dynamicSrc.getName().c_str(), dynamicSrc.getCarrier().c_str()); CARRIER_DEBUG("DYNAMIC_DST: name=%s, carrier=%s\n", dynamicDest.getName().c_str(), dynamicDest.getCarrier().c_str()); - if(!NetworkBase::isValidPortName(dynamicSrc.getName())) + if(!Contact::isValidRegisteredName(dynamicSrc.getName().c_str())) { fprintf(stderr, "Failure: no way to make connection, invalid source '%s'\n", dynamicSrc.getName().c_str()); return 1; } - if(!NetworkBase::isValidPortName(dynamicDest.getName())) + if(!Contact::isValidRegisteredName(dynamicDest.getName().c_str())) { fprintf(stderr, "Failure: no way to make connection, invalid destination '%s'\n", dynamicDest.getName().c_str()); return 1; diff --git a/src/libYARP_OS/src/Port.cpp b/src/libYARP_OS/src/Port.cpp index ddb9fb18017..01ba4632296 100644 --- a/src/libYARP_OS/src/Port.cpp +++ b/src/libYARP_OS/src/Port.cpp @@ -77,7 +77,10 @@ bool Port::openFake(const ConstString& name) bool Port::open(const ConstString& name) { - return open(Contact(name)); + if(Contact::isValidRegistrationName(name.c_str())) { + return open(Contact::fromString(name)); + } + return false; } bool Port::open(const Contact& contact, bool registerName) From db17ef0777424cd9dd5dd70faa1870e5cd7e8738 Mon Sep 17 00:00:00 2001 From: Damiano Enerli Date: Wed, 3 Jan 2018 17:23:30 +0100 Subject: [PATCH 2/3] added some tests to check valid names for port registering --- tests/libYARP_OS/PortTest.cpp | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/libYARP_OS/PortTest.cpp b/tests/libYARP_OS/PortTest.cpp index f982eba2fc1..b0522b378c4 100644 --- a/tests/libYARP_OS/PortTest.cpp +++ b/tests/libYARP_OS/PortTest.cpp @@ -342,6 +342,60 @@ class PortTest : public UnitTest { virtual ConstString getName() override { return "PortTest"; } + void testName() { + // good names + checkTrue(Contact::isValidRegistrationName(""), "Void name is a good registration name."); + checkTrue(Contact::isValidRegistrationName("..."), "\"...\" name is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/port"), "\"/port\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/port:spec"), "\"/port:spec\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p1/p2"), "\"/p1/p2\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p1/p2/p3/p4"), "\"/p1/p2/p3/p4\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p1/p2/p3:any/p4"), "\"/p1/p2/p3:any/p4\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p1/p_2"), "\"/p1/p_2\" is a good name."); + checkTrue(Contact::isValidRegistrationName("/p1/p_2:sp1:ssp2"), "\"/p1/p_2:sp1:ssp2\" is a good name."); + checkTrue(Contact::isValidRegistrationName("carr://port"), "\"carr://port\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("carr://port/port:spec"), "\"carr://port:spec\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("carr://p1/p2"), "\"carr://p1/p2\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("fast_carr://p1/p2/p3/p4"), "\"fast_carr://p1/p2/p3/p4\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1#/p1/p2"), "\"/n1#/p1/p2\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1+#/p1"), "\"/n1+#/p1\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1+1#/p1"), "\"/n1+1#/p1\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1-1#/p1"), "\"/n1-1#/p1\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1=/p1"), "\"/n1=/p1\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1=-1/p1"), "\"/n1=-1/p1\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/n1=+/p1/p2:sp"), "\"/n1=+/p1/p2:sp\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p@"), "\"/p@\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p:s@/n~wire"), "\"/p:s@/n~wire\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/p-1@/n~wire"), "\"/p-1@/n~wire\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/machine:10005"), "\"machine:10005\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/mac12.at.domain:10005"), "\"mac12.at.domain:10005\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("/111.2.30.4:10005"), "\"111.2.30.4:10005\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("carr://11.12.13.14:10005"), "\"carr://11.12.13.14:10005\" is a good registration name."); + checkTrue(Contact::isValidRegistrationName("carr://11.12.13.14:10005/"),"\"carr://11.12.13.14:10005\" is a good registration name."); + + // bad names: + checkFalse(Contact::isValidRegistrationName("/"), "\"/\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/p space"), "\"/p space\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/_"), "\"/_\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/:"), "\"/:\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/p_"), "\"/p_\" is a not good registration name."); + checkFalse(Contact::isValidRegistrationName("/p/"), "\"/p_\" is a not good registration name."); + checkFalse(Contact::isValidRegistrationName("/port:"), "\"/port:\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/str@ng&%port"), "\"/str@ng&%port\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("str4ng&~c4rr://p1/p2"),"\"str4ng&~c4rr://p1/p2\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("carr:/"), "\"carr:/ \" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("=+/p1/p2:sp"), "\"=+/p1/p2:sp\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("#+/p1/p2:sp"), "\"#+/p1/p2:sp\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/n="), "\"/n=\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("/n#"), "\"/n#\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("="), "\"=\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("#"), "\"#\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("@"), "\"@\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("machine:10005"), "\"machine:10005\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("111.2.30.4:abc"), "\"111.2.30.4:abc\" is not a good registration name."); + checkFalse(Contact::isValidRegistrationName("carr:/11.12.13.14:69"),"\"carr:/11.12.13.14:69\" is not a good registration name."); + } + void testOpen() { report(0,"checking opening and closing ports"); Port out, in; @@ -1546,6 +1600,8 @@ class PortTest : public UnitTest { #ifdef BROKEN_TEST testTcp(); #endif + testName(); + testOpen(); //bbb testReadBuffer(); testPair(); From b5478f8f4bea7ee03946dfdd24a3a658809e18bf Mon Sep 17 00:00:00 2001 From: Damiano Enerli Date: Wed, 3 Jan 2018 17:25:21 +0100 Subject: [PATCH 3/3] drop checks on leading '=' and unify category check '#' syntax allowed only "+" and "-" categories while other syntaxes '@' and '=' allow an optional trailing "1" on the contegory. --- src/libYARP_OS/src/NestedContact.cpp | 21 ++++++++++++++++--- src/libYARP_OS/src/Port.cpp | 6 +++--- .../src/NameServiceOnTriples.cpp | 9 +++----- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/libYARP_OS/src/NestedContact.cpp b/src/libYARP_OS/src/NestedContact.cpp index 535d80f4715..f6771b408f1 100644 --- a/src/libYARP_OS/src/NestedContact.cpp +++ b/src/libYARP_OS/src/NestedContact.cpp @@ -72,9 +72,24 @@ bool NestedContact::fromString(const ConstString& nFullName) { nodeName = fullName.substr(0, idx); nestedName = fullName.substr(idx+1, fullName.length()); char ch = nodeName[nodeName.length()-1]; - if (ch=='-'||ch=='+') { - category += ch; - nodeName = nodeName.substr(0, nodeName.length()-1); + if (ch=='-'||ch=='+'||ch=='1') { + size_t offset = 1; + bool ok = true; + if (ch=='1') { + ok = false; + if (nodeName.length()>=2) { + char ch0 = nodeName[nodeName.length()-2]; + if (ch0=='-'||ch0=='+') { + offset++; + category += ch0; + ok = true; + } + } + } + if (ok) { + category += ch; + nodeName = nodeName.substr(0, nodeName.length()-offset); + } } return true; } diff --git a/src/libYARP_OS/src/Port.cpp b/src/libYARP_OS/src/Port.cpp index 01ba4632296..92eecc6dfec 100644 --- a/src/libYARP_OS/src/Port.cpp +++ b/src/libYARP_OS/src/Port.cpp @@ -130,7 +130,7 @@ bool Port::open(const Contact& contact, bool registerName, PortCoreAdapter *currentCore = &(IMPL()); if (currentCore!=nullptr) { currentCore->active = false; - if (n!="" && (n[0]!='/'||currentCore->includeNode) && n[0]!='=' && n!="..." && n.substr(0, 3)!="...") { + if (n!="" && (n[0]!='/'||currentCore->includeNode) && n!="..." && n.substr(0, 3)!="...") { if (fakeName==nullptr) { Nodes& nodes = NameClient::getNameClient().getNodes(); ConstString node_name = nodes.getActiveName(); @@ -140,7 +140,7 @@ bool Port::open(const Contact& contact, bool registerName, } } } - if (n!="" && n[0]!='/' && n[0]!='=' && n!="..." && n.substr(0, 3)!="...") { + if (n!="" && n[0]!='/' && n!="..." && n.substr(0, 3)!="...") { if (fakeName==nullptr) { YARP_SPRINTF1(Logger::get(), error, "Port name '%s' needs to start with a '/' character", @@ -148,7 +148,7 @@ bool Port::open(const Contact& contact, bool registerName, return false; } } - if (n!="" && n!="..." && n[0]!='=' && n.substr(0, 3)!="...") { + if (n!="" && n!="..." && n.substr(0, 3)!="...") { if (fakeName==nullptr) { ConstString prefix = NetworkBase::getEnvironment("YARP_PORT_PREFIX"); if (prefix!="") { diff --git a/src/libYARP_serversql/src/NameServiceOnTriples.cpp b/src/libYARP_serversql/src/NameServiceOnTriples.cpp index 50754702f49..e7f575ee07e 100644 --- a/src/libYARP_serversql/src/NameServiceOnTriples.cpp +++ b/src/libYARP_serversql/src/NameServiceOnTriples.cpp @@ -273,14 +273,11 @@ bool NameServiceOnTriples::cmdRegister(NameTripleState& act) { at++; } lock(); - if (port=="..." || (port.length()>0 && port[0]=='=')) { + if (port=="...") { Contact c(port, carrier, machine, sock); c = alloc->completePortName(c); - if (port =="...") { - port = c.getName(); - } else { - port = c.getName() + port; - } + port = c.getName(); + } t.setNameValue("port",port.c_str()); act.mem.remove_query(t, nullptr);