]> source.dussan.org Git - tigervnc.git/commitdiff
Make strSplit() simpler and safer
authorPierre Ossman <ossman@cendio.se>
Wed, 11 Jan 2023 16:21:56 +0000 (17:21 +0100)
committerPierre Ossman <ossman@cendio.se>
Sat, 4 Feb 2023 13:03:13 +0000 (14:03 +0100)
Get rid of all the magical re-allocation and shuffling and instead just
return a new set of strings that is fully splitted. Will consume a bit
more memory, but is a lot safer to use as there is less confusion about
ownership of memory.

common/network/TcpSocket.cxx
common/rfb/LogWriter.cxx
common/rfb/SSecurityPlain.cxx
common/rfb/Security.cxx
common/rfb/util.cxx
common/rfb/util.h
win/rfb_win32/LaunchProcess.cxx
win/rfb_win32/Win32Util.cxx
win/rfb_win32/Win32Util.h
win/vncconfig/Connections.h
win/vncconfig/Legacy.cxx

index ec36b33dd5e28e454a0878f2701aa6f124b14c06..4feab0ae5cbc147f7b69093a1b12220480c53cb9 100644 (file)
@@ -504,13 +504,13 @@ void network::createTcpListeners(std::list<SocketListener*> *listeners,
 
 
 TcpFilter::TcpFilter(const char* spec) {
-  rfb::CharArray tmp;
-  tmp.buf = rfb::strDup(spec);
-  while (tmp.buf) {
-    rfb::CharArray first;
-    rfb::strSplit(tmp.buf, ',', &first.buf, &tmp.buf);
-    if (strlen(first.buf))
-      filter.push_back(parsePattern(first.buf));
+  std::vector<std::string> patterns;
+
+  patterns = rfb::strSplit(spec, ',');
+
+  for (size_t i = 0; i < patterns.size(); i++) {
+    if (!patterns[i].empty())
+      filter.push_back(parsePattern(patterns[i].c_str()));
   }
 }
 
@@ -603,14 +603,16 @@ TcpFilter::verifyConnection(Socket* s) {
 TcpFilter::Pattern TcpFilter::parsePattern(const char* p) {
   TcpFilter::Pattern pattern;
 
-  rfb::CharArray addr, pref;
-  bool prefix_specified;
+  std::vector<std::string> parts;
   int family;
 
   initSockets();
 
-  prefix_specified = rfb::strSplit(&p[1], '/', &addr.buf, &pref.buf);
-  if (addr.buf[0] == '\0') {
+  parts = rfb::strSplit(&p[1], '/');
+  if (parts.size() > 2)
+    throw Exception("invalid filter specified");
+
+  if (parts[0].empty()) {
     // Match any address
     memset (&pattern.address, 0, sizeof (pattern.address));
     pattern.address.u.sa.sa_family = AF_UNSPEC;
@@ -618,22 +620,19 @@ TcpFilter::Pattern TcpFilter::parsePattern(const char* p) {
   } else {
     struct addrinfo hints;
     struct addrinfo *ai;
-    char *p = addr.buf;
     int result;
     memset (&hints, 0, sizeof (hints));
     hints.ai_family = AF_UNSPEC;
     hints.ai_flags = AI_NUMERICHOST;
 
     // Take out brackets, if present
-    if (*p == '[') {
-      size_t len;
-      p++;
-      len = strlen (p);
-      if (len > 0 && p[len - 1] == ']')
-        p[len - 1] = '\0';
+    if (parts[0][0] == '[') {
+      parts[0].erase(0, 1);
+      if (!parts[0].empty() && parts[0][parts.size()-1] == ']')
+        parts[0].erase(parts.size()-1, 1);
     }
 
-    if ((result = getaddrinfo (p, NULL, &hints, &ai)) != 0) {
+    if ((result = getaddrinfo (parts[0].c_str(), NULL, &hints, &ai)) != 0) {
       throw GAIException("unable to resolve host by name", result);
     }
 
@@ -642,14 +641,14 @@ TcpFilter::Pattern TcpFilter::parsePattern(const char* p) {
 
     family = pattern.address.u.sa.sa_family;
 
-    if (prefix_specified) {
+    if (parts.size() > 1) {
       if (family == AF_INET &&
-          rfb::strContains(pref.buf, '.')) {
+          (parts[1].find('.') != std::string::npos)) {
         throw Exception("mask no longer supported for filter, "
                         "use prefix instead");
       }
 
-      pattern.prefixlen = (unsigned int) atoi(pref.buf);
+      pattern.prefixlen = (unsigned int) atoi(parts[1].c_str());
     } else {
       switch (family) {
       case AF_INET:
index a86e9652fcbf16ae29b7894d7f92260bd39747b4..398d387277a761aedd1fda722487283c27109382 100644 (file)
@@ -76,19 +76,20 @@ LogWriter::getLogWriter(const char* name) {
 }
 
 bool LogWriter::setLogParams(const char* params) {
-  CharArray logwriterName, loggerName, logLevel;
-  if (!strSplit(params, ':', &logwriterName.buf, &loggerName.buf) ||
-    !strSplit(loggerName.buf, ':', &loggerName.buf, &logLevel.buf)) {
+  std::vector<std::string> parts;
+  parts = strSplit(params, ':');
+  if (parts.size() != 3) {
     fprintf(stderr,"failed to parse log params:%s\n",params);
     return false;
   }
-  int level = atoi(logLevel.buf);
+  int level = atoi(parts[2].c_str());
   Logger* logger = 0;
-  if (strcmp("", loggerName.buf) != 0) {
-    logger = Logger::getLogger(loggerName.buf);
-    if (!logger) fprintf(stderr,"no logger found! %s\n",loggerName.buf);
+  if (!parts[1].empty()) {
+    logger = Logger::getLogger(parts[1].c_str());
+    if (!logger)
+      fprintf(stderr, "no logger found! %s\n", parts[1].c_str());
   }
-  if (strcmp("*", logwriterName.buf) == 0) {
+  if (parts[0] == "*") {
     LogWriter* current = log_writers;
     while (current) {
       current->setLog(logger);
@@ -97,9 +98,9 @@ bool LogWriter::setLogParams(const char* params) {
     }
     return true;
   } else {
-    LogWriter* logwriter = getLogWriter(logwriterName.buf);
+    LogWriter* logwriter = getLogWriter(parts[0].c_str());
     if (!logwriter) {
-      fprintf(stderr,"no logwriter found! %s\n",logwriterName.buf);
+      fprintf(stderr, "no logwriter found! %s\n", parts[0].c_str());
     } else {
       logwriter->setLog(logger);
       logwriter->setLevel(level);
@@ -122,11 +123,12 @@ bool LogParameter::setParam(const char* v) {
   if (immutable) return true;
   LogWriter::setLogParams("*::0");
   StringParameter::setParam(v);
-  CharArray logParam;
-  CharArray params(strDup(getValueStr().c_str()));
-  while (params.buf) {
-    strSplit(params.buf, ',', &logParam.buf, &params.buf);
-    if (strlen(logParam.buf) && !LogWriter::setLogParams(logParam.buf))
+  std::vector<std::string> parts;
+  parts = strSplit(v, ',');
+  for (size_t i = 0; i < parts.size(); i++) {
+    if (parts[i].empty())
+        continue;
+    if (!LogWriter::setLogParams(parts[i].c_str()))
       return false;
   }
   return true;
index 018ca517eaa260532e6478662708db6eda284382..8bef8d8ccdbf053703ae03f5b9040b9f3ed0d9cd 100644 (file)
@@ -45,22 +45,15 @@ StringParameter PasswordValidator::plainUsers
 
 bool PasswordValidator::validUser(const char* username)
 {
-  CharArray users(strDup(plainUsers.getValueStr().c_str()));
-  CharArray user;
+  std::vector<std::string> users;
 
-  while (users.buf) {
-    strSplit(users.buf, ',', &user.buf, &users.buf);
-#ifdef WIN32
-    if (0 == stricmp(user.buf, "*"))
-         return true;
-    if (0 == stricmp(user.buf, username))
-         return true;
-#else
-    if (!strcmp(user.buf, "*"))
-         return true;
-    if (!strcmp(user.buf, username))
-         return true;
-#endif
+  users = strSplit(plainUsers, ',');
+
+  for (size_t i = 0; i < users.size(); i++) {
+    if (users[i] == "*")
+      return true;
+    if (users[i] == username)
+      return true;
   }
   return false;
 }
index 56d953f0e81bce70ffb43490115223f854d3d3e6..be07bae373ca53c1ac1377e01c3b7a79af8758fa 100644 (file)
@@ -206,10 +206,10 @@ const char* rfb::secTypeName(uint32_t num)
 std::list<uint32_t> rfb::parseSecTypes(const char* types_)
 {
   std::list<uint32_t> result;
-  CharArray types(strDup(types_)), type;
-  while (types.buf) {
-    strSplit(types.buf, ',', &type.buf, &types.buf);
-    uint32_t typeNum = secTypeNum(type.buf);
+  std::vector<std::string> types;
+  types = strSplit(types_, ',');
+  for (size_t i = 0; i < types.size(); i++) {
+    uint32_t typeNum = secTypeNum(types[i].c_str());
     if (typeNum != secTypeInvalid)
       result.push_back(typeNum);
   }
index 30cf03008651ab38e400026372e66fdc2b4250dd..6f12ad7d7b7c8946663ee7e04ebd567b166bbff6 100644 (file)
@@ -66,35 +66,24 @@ namespace rfb {
     delete [] s;
   }
 
-
-  bool strSplit(const char* src, const char limiter, char** out1, char** out2, bool fromEnd) {
-    CharArray out1old, out2old;
-    if (out1) out1old.buf = *out1;
-    if (out2) out2old.buf = *out2;
-    int len = strlen(src);
-    int i=0, increment=1, limit=len;
-    if (fromEnd) {
-      i=len-1; increment = -1; limit = -1;
-    }
-    while (i!=limit) {
-      if (src[i] == limiter) {
-        if (out1) {
-          *out1 = new char[i+1];
-          if (i) memcpy(*out1, src, i);
-          (*out1)[i] = 0;
-        }
-        if (out2) {
-          *out2 = new char[len-i];
-          if (len-i-1) memcpy(*out2, &src[i+1], len-i-1);
-          (*out2)[len-i-1] = 0;
-        }
-        return true;
+  std::vector<std::string> strSplit(const char* src,
+                                    const char delimiter)
+  {
+    std::vector<std::string> out;
+    const char *start, *stop;
+
+    start = src;
+    do {
+      stop = strchr(start, delimiter);
+      if (stop == NULL) {
+        out.push_back(start);
+      } else {
+        out.push_back(std::string(start, stop-start));
+        start = stop + 1;
       }
-      i+=increment;
-    }
-    if (out1) *out1 = strDup(src);
-    if (out2) *out2 = 0;
-    return false;
+    } while (stop != NULL);
+
+    return out;
   }
 
   bool strContains(const char* src, char c) {
index d8d7c65393849354183f764f2e731d242e3042b7..2540ab3b318a40c0f73afcc90de1b3caf1f3c8d7 100644 (file)
@@ -61,14 +61,9 @@ namespace rfb {
   char* strDup(const char* s);
   void strFree(char* s);
 
-  // Returns true if split successful.  Returns false otherwise.
-  // ALWAYS *copies* first part of string to out1 buffer.
-  // If limiter not found, leaves out2 alone (null) and just copies to out1.
-  // If out1 or out2 non-zero, calls strFree and zeroes them.
-  // If fromEnd is true, splits at end of string rather than beginning.
-  // Either out1 or out2 may be null, in which case the split will not return
-  // that part of the string.  Obviously, setting both to 0 is not useful...
-  bool strSplit(const char* src, const char limiter, char** out1, char** out2, bool fromEnd=false);
+  // Splits a string with the specified delimiter
+  std::vector<std::string> strSplit(const char* src,
+                                    const char delimiter);
 
   // Returns true if src contains c
   bool strContains(const char* src, char c);
index 13d7e9506e66ec32c97c12fc8d888fbb25e52a9d..7d17caf7ae799278914e1f800e58377a19114a9e 100644 (file)
@@ -67,7 +67,9 @@ void LaunchProcess::start(HANDLE userToken, bool createConsole) {
   CharArray exePath;
   if (!strContains(exeName.buf, '\\')) {
     ModuleFileName filename;
-    CharArray path; splitPath(filename.buf, &path.buf, 0);
+    CharArray path(strDup(filename.buf));
+    if (strContains(path.buf, '\\'))
+      *strrchr(path.buf, '\\') = '\0';
     exePath.buf = new char[strlen(path.buf) + strlen(exeName.buf) + 2];
     sprintf(exePath.buf, "%s\\%s", path.buf, exeName.buf);
   } else {
index 5f0bdbc7e1ae11dedaf18153c91258ad1ea03929..ad3015745a00fc95b56184d930698e338dc9179f 100644 (file)
@@ -82,11 +82,6 @@ const char* FileVersionInfo::getVerString(const char* name, DWORD langId) {
 }
 
 
-bool splitPath(const char* path, char** dir, char** file) {
-  return strSplit(path, '\\', dir, file, true);
-}
-
-
 void centerWindow(HWND handle, HWND parent) {
   RECT r;
   MonitorInfo mi(parent ? parent : handle);
index beb22ec788b47ff72f9989c423f83731828c5de9..642e377f9de02412c13207016109ea77d9dad4e4 100644 (file)
@@ -36,8 +36,6 @@ namespace rfb {
       const char* getVerString(const char* name, DWORD langId = 0x080904b0);
     };
 
-    bool splitPath(const char* path, char** dir, char** file);
-
     // Center the window to a rectangle, or to a parent window.
     // Optionally, resize the window to lay within the rect or parent window
     // If the parent window is NULL then the working area if the window's
index f9b6654786ed6f9c1b1013647e58bba501ef1d4a..492525f38a74bb2f8d14f2ee2670f9f7f6050c3d 100644 (file)
@@ -100,13 +100,11 @@ namespace rfb {
         while (SendMessage(listBox, LB_GETCOUNT, 0, 0))
           SendMessage(listBox, LB_DELETESTRING, 0, 0);
 
-        CharArray tmp;
-        tmp.buf = strDup(hosts.getValueStr().c_str());
-        while (tmp.buf) {
-          CharArray first;
-          strSplit(tmp.buf, ',', &first.buf, &tmp.buf);
-          if (strlen(first.buf))
-            SendMessage(listBox, LB_ADDSTRING, 0, (LPARAM)first.buf);
+        std::vector<std::string> hostv;
+        hostv = strSplit(hosts, ',');
+        for (size_t i = 0; i < hostv.size(); i++) {
+          if (!hostv[i].empty())
+            SendMessage(listBox, LB_ADDSTRING, 0, (LPARAM)hostv[i].c_str());
         }
 
         onCommand(IDC_RFB_ENABLE, EN_CHANGE);
index c9c5e697ec026acd61374eed0b5a10352af1a7b3..bbd796d092f706bb09c550a0b8db3e64568b79b5 100644 (file)
@@ -69,30 +69,26 @@ void LegacyPage::LoadPrefs()
 
             // Reformat AuthHosts to Hosts.  Wish I'd left the format the same. :( :( :(
             try {
-              CharArray tmp(strDup(authHosts.c_str()));
-              while (tmp.buf) {
-
-                // Split the AuthHosts string into patterns to match
-                CharArray first;
-                rfb::strSplit(tmp.buf, ':', &first.buf, &tmp.buf);
-                if (strlen(first.buf)) {
+              // Split the AuthHosts string into patterns to match
+              std::vector<std::string> patterns;
+              patterns = rfb::strSplit(authHosts.c_str(), ':');
+              for (size_t i = 0; i < patterns.size(); i++) {
+                if (!patterns[i].empty()) {
                   int bits = 0;
                   char pattern[1+4*4+4];
-                  pattern[0] = first.buf[0];
+                  pattern[0] = patterns[i][0];
                   pattern[1] = 0;
 
                   // Split the pattern into IP address parts and process
-                  rfb::CharArray address;
-                  address.buf = rfb::strDup(&first.buf[1]);
-                  while (address.buf) {
-                    rfb::CharArray part;
-                    rfb::strSplit(address.buf, '.', &part.buf, &address.buf);
+                  std::vector<std::string> parts;
+                  parts = rfb::strSplit(&patterns[i][1], '.');
+                  for (size_t j = 0; j < parts.size(); j++) {
                     if (bits)
                       strcat(pattern, ".");
-                    if (strlen(part.buf) > 3)
+                    if (parts[j].size() > 3)
                       throw rdr::Exception("Invalid IP address part");
-                    if (strlen(part.buf) > 0) {
-                      strcat(pattern, part.buf);
+                    if (!parts[j].empty()) {
+                      strcat(pattern, parts[j].c_str());
                       bits += 8;
                     }
                   }