[LLDB] Fix parsing of IPv6 host:port inside brackets

[emrekultursay]

Summary

When using IPv6 host:port pairs, typically the host is put inside
brackets, such as [2601:1234:...:0213]:5555, and the UriParser
can handle this format.

However, the Android infrastructure in LLDB assumes an additional
brackets around the host:port pair, such that the entire host:port
string can be treated as the host (which is used as an Android Serial
Number), and UriParser cannot handle multiple brackets. Parsing
inputs with such extra backets requires searching the closing bracket
from the right.

Test: BracketedHostnameWithPortIPv6 covers the case mentioned above

[shafik]

We need tests it looks like UriParserTest.cpp is the right place.

[emrekultursay]

Added unit tests.

[kwk]

LGTM but I haven't tested it.

[labath]

Looks good. You already mention that in the commit message, but it may be nice to also mention somewhere near the test case that these kinds of "URL"s can occur when and connecting to and android device over IP (in which case the port is part of the "hostname"). So one isn't left wondering why are we testing such "nonsensical" hostnames...

Also, do you have commit access?

[emrekultursay]

• Added comments to Android-specific test cases, as suggested by labath@.
• Reformatted lines that exceeded 80 chars.

[emrekultursay]

I don't have commit access.

lldb/source/Utility/UriParser.cpp

bool UriParser::Parse(llvm::StringRef uri, llvm::StringRef &scheme,
 
   auto host_port = uri.substr(
       host_pos,
       ((path_pos != std::string::npos) ? path_pos : uri.size()) - host_pos);
 
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
     // hostname is enclosed with square brackets.
-    pos = host_port.find(']');
+    pos = host_port.rfind(']');
     if (pos == std::string::npos)
       return false;
 
     tmp_hostname = host_port.substr(1, pos - 1);
     host_port = host_port.drop_front(pos + 1);
     if (!host_port.empty() && !host_port.consume_front(":"))
       return false;
   } else {

lldb/unittests/Utility/UriParserTest.cpp

TEST(UriParserTest, MinimalPortPath) {
   VALIDATE
 }
 
 TEST(UriParserTest, LongPath) {
   const UriTestCase testCase("x://y/abc/def/xyz", "x", "y", -1, "/abc/def/xyz");
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
                              "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase testCase(
+      "connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+      "2601:600:107f:db64:a42b:4faa:284:3082", 5432, "/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
                              "192.168.100.132", 5432, "/");
   llvm::StringRef scheme(kAsdf);
   llvm::StringRef hostname(kAsdf);
   int port(1138);
   llvm::StringRef path(kAsdf);
   bool result = UriParser::Parse(testCase.m_uri, scheme, hostname, port, path);
   EXPECT_EQ(testCase.m_result, result);
 
   EXPECT_STREQ(testCase.m_scheme, scheme.str().c_str());
   EXPECT_STREQ(testCase.m_hostname, hostname.str().c_str());
   EXPECT_EQ(testCase.m_port, port);
   EXPECT_STREQ(testCase.m_path, path.str().c_str());
 }
 
 TEST(UriParserTest, BracketedHostname) {
   const UriTestCase testCase("connect://[192.168.100.132]", "connect",
                              "192.168.100.132", -1, "/");
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  // Android device over IPv4: port is a part of the hostname.
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+                             "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  // Android device over IPv6: port is a part of the hostname.
+  const UriTestCase testCase(
+      "connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+      "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:5555]:1234", "connect",
                              "192.168.100.132:5555", 1234, "/");
   VALIDATE
 }
 
 TEST(UriParserTest, SchemeHostSeparator) {
   const UriTestCase testCase("x:/y");