ovyalov created this revision.
ovyalov added reviewers: clayborg, tberghammer, chaoren.
ovyalov added a subscriber: lldb-commits.

UriParser:Parse expects that host name doesn't contain special URL symbols  - 
":", ";", "$",...
However, in case of adb protocol (adb://$device_serial_no:$port) it poses a 
problem -  device serial number may contain a colon ("192.168.0.1:5555").
In order to handle this situation we can pass URL encoded serial number 
(adb://192.168.100.132%3A5555:5432) and decode URL components within 
UriParser::Parse.
As an alternative, there is an option to have a adb-specific regexp in 
UriParser::Parse but I would like to keep UriParser protocol-independent.   

http://reviews.llvm.org/D12025

Files:
  lldb.xcodeproj/project.pbxproj
  source/Host/posix/ConnectionFileDescriptorPosix.cpp
  source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  source/Utility/CMakeLists.txt
  source/Utility/UriParser.cpp
  source/Utility/UrlEncoder.cpp
  source/Utility/UrlEncoder.h
  unittests/Utility/CMakeLists.txt
  unittests/Utility/UriParserTest.cpp
  unittests/Utility/UrlEncoderTest.cpp

Index: unittests/Utility/UrlEncoderTest.cpp
===================================================================
--- /dev/null
+++ unittests/Utility/UrlEncoderTest.cpp
@@ -0,0 +1,31 @@
+#include "gtest/gtest.h"
+#include "Utility/UrlEncoder.h"
+
+namespace
+{
+    class UrlEncoderTest: public ::testing::Test
+    {
+    };
+}
+
+TEST_F (UrlEncoderTest, EncodeHostPort)
+{
+    EXPECT_EQ("192.168.100.132%3A5555", UrlEncoder::Encode("192.168.100.132:5555"));
+}
+
+TEST_F (UrlEncoderTest, DecodeHostPort)
+{
+    EXPECT_EQ("192.168.100.132:5555", UrlEncoder::Decode("192.168.100.132%3A5555"));
+}
+
+TEST_F (UrlEncoderTest, DecodeBrokenInput)
+{
+    EXPECT_EQ("192.168.100.132%3", UrlEncoder::Decode("192.168.100.132%3"));
+}
+
+TEST_F (UrlEncoderTest, EncodeDecodeAscii)
+{
+    const char* url = "192.168.100.132";
+    EXPECT_EQ(url, UrlEncoder::Encode(url));
+    EXPECT_EQ(url, UrlEncoder::Decode(url));
+}
Index: unittests/Utility/UriParserTest.cpp
===================================================================
--- unittests/Utility/UriParserTest.cpp
+++ unittests/Utility/UriParserTest.cpp
@@ -85,6 +85,12 @@
     VALIDATE
 }
 
+TEST_F (UriParserTest, AdbHostPortSerial)
+{
+    const UriTestCase testCase("adb://192.168.100.132%3A5555:5432/", "adb", "192.168.100.132:5555", 5432, "/");
+    VALIDATE
+}
+
 TEST_F (UriParserTest, SchemeHostSeparator)
 {
     const UriTestCase testCase("x:/y");
@@ -132,4 +138,3 @@
     const UriTestCase testCase("x://y:0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789/");
     VALIDATE
 }
-
Index: unittests/Utility/CMakeLists.txt
===================================================================
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
   StringExtractorTest.cpp
   UriParserTest.cpp
+  UrlEncoderTest.cpp
   )
Index: source/Utility/UrlEncoder.h
===================================================================
--- /dev/null
+++ source/Utility/UrlEncoder.h
@@ -0,0 +1,27 @@
+//===-- UrlEncoder.h --------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef utility_UrlEncoder_h_
+#define utility_UrlEncoder_h_
+
+// C Includes
+// C++ Includes
+#include <string>
+
+// Other libraries and framework includes
+// Project includes
+
+class UrlEncoder
+{
+public:
+    static std::string Encode(const std::string& url);
+    static std::string Decode(const std::string& url);
+};
+
+#endif  // utility_UrlEncoder_h_
Index: source/Utility/UrlEncoder.cpp
===================================================================
--- /dev/null
+++ source/Utility/UrlEncoder.cpp
@@ -0,0 +1,72 @@
+//===-- UrlEncoder.cpp ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Utility/UrlEncoder.h"
+
+// C++ Includes
+#include <sstream>
+
+namespace {
+
+const char kUrlSpecialSyms[] = {'!', '*', '\'', '(', ')', ';', ':', '@', '&', '=', '+', '$', ',', '/', '?', '%', '#', '[', ']'};
+
+bool
+IsSpecialUrlSymbol(const char ch)
+{
+    for (const char sym: kUrlSpecialSyms)
+    {
+        if (sym == ch)
+            return true;
+    }
+    return false;
+}
+
+}
+
+std::string
+UrlEncoder::Encode(const std::string& url)
+{
+    std::ostringstream result;
+    char sym_fmt[3];
+
+    for (const char sym: url)
+    {
+        if (IsSpecialUrlSymbol(sym))
+        {
+            sprintf(sym_fmt, "%02X", uint16_t(sym));
+            result << "%" << sym_fmt;
+        }
+        else
+            result << sym;
+    }
+    return result.str();
+}
+
+std::string
+UrlEncoder::Decode(const std::string& url)
+{
+    std::ostringstream result;
+
+    for (int i = 0, len = url.length(); i < len; ++i)
+    {
+        if (url[i] == '%' &&
+            (i + 2) < len &&
+            isxdigit(url[i + 1]) && isxdigit(url[i + 2]))
+        {
+            int decoded = 0;
+            sscanf(url.substr(i + 1, 2).c_str(), "%x", &decoded);
+            result << static_cast<char>(decoded);
+            i += 2;
+        }
+        else
+            result << url[i];
+    }
+
+    return result.str();
+}
Index: source/Utility/UriParser.cpp
===================================================================
--- source/Utility/UriParser.cpp
+++ source/Utility/UriParser.cpp
@@ -14,9 +14,11 @@
 #include <stdio.h>
 
 // C++ Includes
+
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Host/StringConvert.h"
+#include "Utility/UrlEncoder.h"
 
 using namespace lldb_private;
 
@@ -35,18 +37,18 @@
     char hostname_buf[256] = {0};
     char port_buf[11] = {0}; // 10==strlen(2^32)
     char path_buf[2049] = {'/', 0};
-  
+
     bool ok = false;
-         if (4==sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]/%2047s", scheme_buf, hostname_buf, port_buf, path_buf+1)) { ok = true; }
-    else if (3==sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]", scheme_buf, hostname_buf, port_buf)) { ok = true; }
-    else if (3==sscanf(uri, "%99[^:/]://%255[^/]/%2047s", scheme_buf, hostname_buf, path_buf+1)) { ok = true; }
-    else if (2==sscanf(uri, "%99[^:/]://%255[^/]", scheme_buf, hostname_buf)) { ok = true; }
+    if (4 == sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]/%2047s", scheme_buf, hostname_buf, port_buf, path_buf+1)) { ok = true; }
+    else if (3 == sscanf(uri, "%99[^:/]://%255[^/:]:%10[^/]", scheme_buf, hostname_buf, port_buf)) { ok = true; }
+    else if (3 == sscanf(uri, "%99[^:/]://%255[^/]/%2047s", scheme_buf, hostname_buf, path_buf+1)) { ok = true; }
+    else if (2 == sscanf(uri, "%99[^:/]://%255[^/]", scheme_buf, hostname_buf)) { ok = true; }
 
     bool success = false;
     int port_tmp = -1;
     if (port_buf[0])
     {
-        port_tmp = StringConvert::ToUInt32(port_buf, UINT32_MAX, 10, &success);
+        port_tmp = StringConvert::ToUInt32(UrlEncoder::Decode(port_buf).c_str(), UINT32_MAX, 10, &success);
         if (!success || port_tmp > 65535)
         {
             // there are invalid characters in port_buf
@@ -57,10 +59,9 @@
     if (ok)
     {
         scheme.assign(scheme_buf);
-        hostname.assign(hostname_buf);
+        hostname.assign(UrlEncoder::Decode(hostname_buf));
         port = port_tmp;
-        path.assign(path_buf);
+        path.assign(UrlEncoder::Decode(path_buf));
     }
     return ok;
 }
-
Index: source/Utility/CMakeLists.txt
===================================================================
--- source/Utility/CMakeLists.txt
+++ source/Utility/CMakeLists.txt
@@ -18,4 +18,5 @@
   StringLexer.cpp
   TimeSpecTimeout.cpp
   UriParser.cpp
+  UrlEncoder.cpp
   )
Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===================================================================
--- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -31,6 +31,7 @@
 #include "lldb/Target/Target.h"
 
 #include "Utility/UriParser.h"
+#include "Utility/UrlEncoder.h"
 
 #include "Plugins/Process/Utility/GDBRemoteSignals.h"
 
@@ -49,10 +50,14 @@
     const char *override_hostname = getenv("LLDB_PLATFORM_REMOTE_GDB_SERVER_HOSTNAME");
     const char *port_offset_c_str = getenv("LLDB_PLATFORM_REMOTE_GDB_SERVER_PORT_OFFSET");
     int port_offset = port_offset_c_str ? ::atoi(port_offset_c_str) : 0;
+
+    const std::string encoded_hostname = UrlEncoder::Encode(
+        override_hostname ? override_hostname : platform_hostname.c_str());
+
     StreamString result;
     result.Printf("%s://%s:%u",
             override_scheme ? override_scheme : platform_scheme.c_str(),
-            override_hostname ? override_hostname : platform_hostname.c_str(),
+            encoded_hostname.c_str(),
             port + port_offset);
     return result.GetString();
 }
Index: source/Host/posix/ConnectionFileDescriptorPosix.cpp
===================================================================
--- source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -33,6 +33,8 @@
 #endif
 
 // C++ Includes
+#include <sstream>
+
 // Other libraries and framework includes
 #include "llvm/Support/ErrorHandling.h"
 #if defined(__APPLE__)
@@ -46,6 +48,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Interpreter/Args.h"
+#include "Utility/UriParser.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -169,10 +172,16 @@
         else if (strstr(s, "adb://") == s)
         {
             int port = -1;
-            sscanf(s, "adb://%*[^:]:%d", &port);
-            char host_and_port[sizeof("localhost:65535")];
-            snprintf(host_and_port, sizeof(host_and_port), "localhost:%d", port);
-            return ConnectTCP(host_and_port, error_ptr);
+            std::string scheme, host, path;
+            if (!UriParser::Parse(s, scheme, host, port, path))
+            {
+                if (error_ptr)
+                    error_ptr->SetErrorStringWithFormat("Failed to parse URL '%s'", s);
+                return eConnectionStatusError;
+            }
+            std::ostringstream host_and_port;
+            host_and_port << "localhost:" << port;
+            return ConnectTCP(host_and_port.str().c_str(), error_ptr);
         }
         else if (strstr(s, "connect://") == s)
         {
Index: lldb.xcodeproj/project.pbxproj
===================================================================
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -101,6 +101,8 @@
 		256CBDBC1ADD107200BC6CDC /* RegisterContextLinux_mips64.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 256CBDB81ADD107200BC6CDC /* RegisterContextLinux_mips64.cpp */; };
 		256CBDC01ADD11C000BC6CDC /* RegisterContextPOSIX_arm.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 256CBDBE1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.cpp */; };
 		257E47171AA56C2000A62F81 /* ModuleCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 257E47151AA56C2000A62F81 /* ModuleCache.cpp */; };
+		259323B71B7D8EF300C11BD6 /* UrlEncoder.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 259323B51B7D8EF300C11BD6 /* UrlEncoder.cpp */; };
+		259323B81B7D8EF300C11BD6 /* UrlEncoder.h in Headers */ = {isa = PBXBuildFile; fileRef = 259323B61B7D8EF300C11BD6 /* UrlEncoder.h */; };
 		25EF23781AC09B3700908DF0 /* AdbClient.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 25EF23751AC09AD800908DF0 /* AdbClient.cpp */; };
 		260157C61885F51C00F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
 		260157C81885F53100F875CF /* libpanel.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 260157C41885F4FF00F875CF /* libpanel.dylib */; };
@@ -1176,6 +1178,8 @@
 		256CBDBF1ADD11C000BC6CDC /* RegisterContextPOSIX_arm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterContextPOSIX_arm.h; path = Utility/RegisterContextPOSIX_arm.h; sourceTree = "<group>"; };
 		257E47151AA56C2000A62F81 /* ModuleCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ModuleCache.cpp; path = source/Utility/ModuleCache.cpp; sourceTree = "<group>"; };
 		257E47161AA56C2000A62F81 /* ModuleCache.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ModuleCache.h; path = source/Utility/ModuleCache.h; sourceTree = "<group>"; };
+		259323B51B7D8EF300C11BD6 /* UrlEncoder.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = UrlEncoder.cpp; path = source/Utility/UrlEncoder.cpp; sourceTree = "<group>"; };
+		259323B61B7D8EF300C11BD6 /* UrlEncoder.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = UrlEncoder.h; path = source/Utility/UrlEncoder.h; sourceTree = "<group>"; };
 		25EF23751AC09AD800908DF0 /* AdbClient.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AdbClient.cpp; sourceTree = "<group>"; };
 		25EF23761AC09AD800908DF0 /* AdbClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AdbClient.h; sourceTree = "<group>"; };
 		260157C41885F4FF00F875CF /* libpanel.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libpanel.dylib; path = /usr/lib/libpanel.dylib; sourceTree = "<absolute>"; };
@@ -3526,6 +3530,8 @@
 		2682F168115ED9C800CCFF99 /* Utility */ = {
 			isa = PBXGroup;
 			children = (
+				259323B51B7D8EF300C11BD6 /* UrlEncoder.cpp */,
+				259323B61B7D8EF300C11BD6 /* UrlEncoder.h */,
 				257E47151AA56C2000A62F81 /* ModuleCache.cpp */,
 				257E47161AA56C2000A62F81 /* ModuleCache.h */,
 				33064C9B1A5C7A490033D415 /* UriParser.h */,
@@ -5473,6 +5479,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				260A63171861008E00FECF8E /* IOHandler.h in Headers */,
+				259323B81B7D8EF300C11BD6 /* UrlEncoder.h in Headers */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -6306,6 +6313,7 @@
 				942AFF0519F84ABF007B43B4 /* LibCxxVector.cpp in Sources */,
 				268900F313353E6F00698AC0 /* StackFrame.cpp in Sources */,
 				268900F413353E6F00698AC0 /* StackFrameList.cpp in Sources */,
+				259323B71B7D8EF300C11BD6 /* UrlEncoder.cpp in Sources */,
 				268900F513353E6F00698AC0 /* StackID.cpp in Sources */,
 				268900F613353E6F00698AC0 /* StopInfo.cpp in Sources */,
 				256CBDB41ADD0EFD00BC6CDC /* RegisterContextPOSIXCore_arm.cpp in Sources */,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to