Fix toString() for authority-less URIs

Summary: Uri("foo:bar").str() would incorrectly return "foo://bar"

Test Plan: test added

Reviewed By: savasp@fb.com, markisaa@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant

FB internal diff: D2107530

Tasks: 7248055

Signature: t1:2107530:1432837143:c100f148c07b5b141cc036b1b39e6c8317e9bbd6

diff --git a/folly/Uri-inl.h b/folly/Uri-inl.h
index e9ec73215f8269080423e1561b9b9aa32ac9ee54..812c400a7960dc4803127ba342fb4a7242598739 100644 (file)
--- a/folly/Uri-inl.h
+++ b/folly/Uri-inl.h
@@ -25,15 +25,19 @@ namespace folly {
 template <class String>
 String Uri::toString() const {
   String str;
-  toAppend(scheme_, "://", &str);
-  if (!password_.empty()) {
-    toAppend(username_, ":", password_, "@", &str);
-  } else if (!username_.empty()) {
-    toAppend(username_, "@", &str);
-  }
-  toAppend(host_, &str);
-  if (port_ != 0) {
-    toAppend(":", port_, &str);
+  if (hasAuthority_) {
+    toAppend(scheme_, "://", &str);
+    if (!password_.empty()) {
+      toAppend(username_, ":", password_, "@", &str);
+    } else if (!username_.empty()) {
+      toAppend(username_, "@", &str);
+    }
+    toAppend(host_, &str);
+    if (port_ != 0) {
+      toAppend(":", port_, &str);
+    }
+  } else {
+    toAppend(scheme_, ":", &str);
   }
   toAppend(path_, &str);
   if (!query_.empty()) {

diff --git a/folly/Uri.cpp b/folly/Uri.cpp
index 6d5befb95e3ad511194aa3708ab21d207b9d500e..9bff152b1c45f518830fdb4b600492c845aad4fd 100644 (file)
--- a/folly/Uri.cpp
+++ b/folly/Uri.cpp
@@ -37,7 +37,7 @@ void toLower(String& s) {
 
 }  // namespace
 
-Uri::Uri(StringPiece str) : port_(0) {
+Uri::Uri(StringPiece str) : hasAuthority_(false), port_(0) {
   static const boost::regex uriRegex(
       "([a-zA-Z][a-zA-Z0-9+.-]*):"  // scheme:
       "([^?#]*)"                    // authority and path
@@ -60,6 +60,7 @@ Uri::Uri(StringPiece str) : port_(0) {
                           authorityAndPathMatch,
                           authorityAndPathRegex)) {
     // Does not start with //, doesn't have authority
+    hasAuthority_ = false;
     path_ = authorityAndPath.fbstr();
   } else {
     static const boost::regex authorityRegex(
@@ -84,6 +85,7 @@ Uri::Uri(StringPiece str) : port_(0) {
       port_ = to<uint16_t>(port);
     }
 
+    hasAuthority_ = true;
     username_ = submatch(authorityMatch, 1);
     password_ = submatch(authorityMatch, 2);
     host_ = submatch(authorityMatch, 3);

diff --git a/folly/Uri.h b/folly/Uri.h
index c8712160a97c479fefdfea4e5078432ac22a624e..739fc38f8e53f7e5c54224889e0572e905f0ea9c 100644 (file)
--- a/folly/Uri.h
+++ b/folly/Uri.h
@@ -76,7 +76,10 @@ class Uri {
   std::string str() const { return toString<std::string>(); }
   fbstring fbstr() const { return toString<fbstring>(); }
 
-  void setPort(uint16_t port) {port_ = port;}
+  void setPort(uint16_t port) {
+    hasAuthority_ = true;
+    port_ = port;
+  }
 
   /**
    * Get query parameters as key-value pairs.
@@ -105,6 +108,7 @@ class Uri {
   fbstring username_;
   fbstring password_;
   fbstring host_;
+  bool hasAuthority_;
   uint16_t port_;
   fbstring path_;
   fbstring query_;

diff --git a/folly/test/UriTest.cpp b/folly/test/UriTest.cpp
index 418bf2596f23d0d017c517f9efd5b9db00f61051..12d8eafbecaecc0a134c336ae1d2e909a626d7c7 100644 (file)
--- a/folly/test/UriTest.cpp
+++ b/folly/test/UriTest.cpp
@@ -239,7 +239,7 @@ TEST(Uri, Simple) {
     EXPECT_EQ("/etc/motd", u.path());
     EXPECT_EQ("", u.query());
     EXPECT_EQ("", u.fragment());
-    EXPECT_EQ("file:///etc/motd", u.fbstr());
+    EXPECT_EQ("file:/etc/motd", u.fbstr());
   }
 
   {
@@ -389,6 +389,29 @@ TEST(Uri, Simple) {
       // success
     }
   }
+
+  // No authority (no "//") is valid
+  {
+    fbstring s("this:is/a/valid/uri");
+    Uri u(s);
+    EXPECT_EQ("this", u.scheme());
+    EXPECT_EQ("is/a/valid/uri", u.path());
+    EXPECT_EQ(s, u.fbstr());
+  }
+  {
+    fbstring s("this:is:another:valid:uri");
+    Uri u(s);
+    EXPECT_EQ("this", u.scheme());
+    EXPECT_EQ("is:another:valid:uri", u.path());
+    EXPECT_EQ(s, u.fbstr());
+  }
+  {
+    fbstring s("this:is@another:valid:uri");
+    Uri u(s);
+    EXPECT_EQ("this", u.scheme());
+    EXPECT_EQ("is@another:valid:uri", u.path());
+    EXPECT_EQ(s, u.fbstr());
+  }
 }
 
 /**