Fix URL compositing in Beast (RIPD-636).

beast/http/HTTP.unity.cpp

@@ -25,14 +25,13 @@
 #include <beast/http/impl/basic_url.cpp>
 #include <beast/http/impl/joyent_parser.cpp>
 #include <beast/http/impl/method.cpp>
-#include <beast/http/impl/ParsedURL.cpp>
 #include <beast/http/impl/raw_parser.cpp>
 #include <beast/http/impl/URL.cpp>
 
 #include <beast/http/tests/basic_url.test.cpp>
 #include <beast/http/tests/client_session.test.cpp>
 #include <beast/http/tests/parser.test.cpp>
-#include <beast/http/tests/ParsedURL.cpp>
 #include <beast/http/tests/rfc2616.test.cpp>
+#include <beast/http/tests/URL.test.cpp>
 #include <beast/http/tests/urls_large_data.cpp>
 

beast/http/ParsedURL.h

@@ -1,51 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of Beast: https://github.com/vinniefalco/Beast
-    Copyright 2013, Vinnie Falco <vinnie.falco@gmail.com>
-
-    Permission to use, copy, modify, and/or distribute this software for any
-    purpose  with  or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#ifndef BEAST_HTTP_PARSEDURL_H_INCLUDED
-#define BEAST_HTTP_PARSEDURL_H_INCLUDED
-
-#include <beast/Strings.h>
-#include <beast/http/URL.h>
-
-namespace beast {
-
-/** Parses a String containing a URL. */
-class ParsedURL
-{
-public:
-    ParsedURL ();
-    explicit ParsedURL (String const& url);
-    ParsedURL (int error, URL const& url);
-    ParsedURL (ParsedURL const& other);
-    ParsedURL& operator= (ParsedURL const& other);
-
-    /** Zero for success, else a non zero value indicating failure. */
-    int error () const;
-
-    /** The parsed URL if there was no error. */
-    URL url () const;
-
-private:
-    int m_error;
-    URL m_url;
-};
-
-}
-
-#endif

beast/http/URL.h

@@ -20,8 +20,6 @@
 #ifndef BEAST_HTTP_URL_H_INCLUDED
 #define BEAST_HTTP_URL_H_INCLUDED
 
-#include <beast/strings/String.h>
-
 #include <ios>
 
 namespace beast {
@@ -32,94 +30,137 @@ namespace beast {
 class URL
 {
 public:
-    /** Construct an empty URL. */
-    explicit URL ();
-
     /** Construct a URL from it's components. */
     URL (
-        String schema_,
+        std::string schema_,
-        String host_,
+        std::string host_,
         std::uint16_t port_,
-        String port_string_,
+        std::string port_string_,
-        String path_,
+        std::string path_,
-        String query_ = "",
+        std::string query_ = "",
-        String fragment_ = "",
+        std::string fragment_ = "",
-        String userinfo_ = "");
+        std::string userinfo_ = "");
+
+    /** Construct an empty URL. */
+    explicit URL () = default;
 
     /** Copy construct a URL. */
-    URL (URL const& other);
+    URL (URL const& other) = default;
 
     /** Copy assign a URL. */
-    URL& operator= (URL const& other);
+    URL& operator= (URL const& other) = default;
+
+    /** Move construct a URL. */
+    URL (URL&& other) = default;
 
     /** Returns `true` if this is an empty URL. */
-    bool empty () const;
+    bool
+    empty () const;
 
     /** Returns the scheme of the URL.
         If no scheme was specified, the string will be empty.
     */
-    String scheme () const;
+    std::string const&
+    scheme () const;
 
     /** Returns the host of the URL.
         If no host was specified, the string will be empty.
     */
-    String host () const;
+    std::string const&
+    host () const;
 
     /** Returns the port number as an integer.
         If no port was specified, the value will be zero.
     */
-    std::uint16_t port () const;
+    std::uint16_t
+    port () const;
 
     /** Returns the port number as a string.
         If no port was specified, the string will be empty.
     */
-    String port_string () const;
+    std::string const&
+    port_string () const;
 
     /** Returns the path of the URL.
         If no path was specified, the string will be empty.
     */
-    String path () const;
+    std::string const&
+    path () const;
 
     /** Returns the query parameters portion of the URL.
         If no query parameters were present, the string will be empty.
     */
-    String query () const;
+    std::string const&
+    query () const;
 
     /** Returns the URL fragment, if any. */
-    String fragment () const;
+    std::string const&
+    fragment () const;
 
     /** Returns the user information, if any. */
-    String userinfo () const;
+    std::string const&
-
+    userinfo () const;
-    /** Retrieve the full URL as a single string. */
-    /** @{ */
-    String toString () const;
-    std::string to_string() const;
-    /** @} */
 
 private:
-    String m_scheme;
+    std::string m_scheme;
-    String m_host;
+    std::string m_host;
-    std::uint16_t m_port;
+    std::uint16_t m_port = 0;
-    String m_port_string;
+    std::string m_port_string;
-    String m_path;
+    std::string m_path;
-    String m_query;
+    std::string m_query;
-    String m_fragment;
+    std::string m_fragment;
-    String m_userinfo;
+    std::string m_userinfo;
 };
 
+/** Attempt to parse a string into a URL */
+std::pair<bool, URL>
+parse_URL(std::string const&);
+
+/** Retrieve the full URL as a single string. */
+std::string
+to_string(URL const& url);
+
+/** Output stream conversion. */
+std::ostream&
+operator<< (std::ostream& os, URL const& url);
+
 /** URL comparisons. */
 /** @{ */
-inline bool operator== (URL const& lhs, URL const& rhs) { return    lhs.toString() == rhs.toString(); }
+inline bool
-inline bool operator!= (URL const& lhs, URL const& rhs) { return ! (lhs.toString() == rhs.toString()); }
+operator== (URL const& lhs, URL const& rhs)
-inline bool operator<  (URL const& lhs, URL const& rhs) { return    lhs.toString() <  rhs.toString(); }
+{
-inline bool operator>  (URL const& lhs, URL const& rhs) { return    rhs.toString() <  lhs.toString(); }
+    return to_string (lhs) == to_string (rhs);
-inline bool operator<= (URL const& lhs, URL const& rhs) { return ! (rhs.toString() <  lhs.toString()); }
+}
-inline bool operator>= (URL const& lhs, URL const& rhs) { return ! (lhs.toString() <  rhs.toString()); }
-/** @} */
 
-/** Output stream conversion. */
+inline bool
-std::ostream& operator<< (std::ostream& os, URL const& url);
+operator!= (URL const& lhs, URL const& rhs)
+{
+    return to_string (lhs) != to_string (rhs);
+}
+
+inline bool
+operator<  (URL const& lhs, URL const& rhs)
+{
+    return to_string (lhs) < to_string (rhs);
+}
+
+inline bool operator>  (URL const& lhs, URL const& rhs)
+{
+    return to_string (rhs) < to_string (lhs);
+}
+
+inline bool
+operator<= (URL const& lhs, URL const& rhs)
+{
+    return ! (to_string (rhs) < to_string (lhs));
+}
+
+inline bool
+operator>= (URL const& lhs, URL const& rhs)
+{
+    return ! (to_string (lhs) < to_string (rhs));
+}
+/** @} */
 
 /** boost::hash support */
 template <class Hasher>
@@ -128,7 +169,7 @@ void
 hash_append (Hasher& h, URL const& url)
 {
     using beast::hash_append;
-    hash_append (h, url.toString());
+    hash_append (h, to_string (url));
 }
 
 }
@@ -140,8 +181,10 @@ namespace std {
 template <>
 struct hash <beast::URL>
 {
-    std::size_t operator() (beast::URL const& v) const
+    std::size_t operator() (beast::URL const& url) const
-        { return v.toString().hash(); }
+    {
+        return std::hash<std::string>{} (to_string (url));
+    }
 };
 
 }

beast/http/impl/ParsedURL.cpp

@@ -1,150 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of Beast: https://github.com/vinniefalco/Beast
-    Copyright 2013, Vinnie Falco <vinnie.falco@gmail.com>
-
-    Permission to use, copy, modify, and/or distribute this software for any
-    purpose  with  or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#include <beast/http/ParsedURL.h>
-#include <beast/strings/String.h>
-
-#include <beast/http/impl/joyent_parser.h>
-
-#include <cstdint>
-
-namespace beast {
-
-ParsedURL::ParsedURL ()
-    : m_error (0)
-{
-}
-
-ParsedURL::ParsedURL (String const& url)
-{
-    std::string const ss (url.toStdString ());
-    std::size_t const buflen (ss.size ());
-    char const* const buf (ss.c_str ());
-
-    joyent::http_parser_url u;
-
-    m_error = joyent::http_parser_parse_url (buf, buflen, false, &u);
-
-    String scheme_;
-    String host_;
-    std::uint16_t port_ (0);
-    String port_string_;
-    String path_;
-    String query_;
-    String fragment_;
-    String userinfo_;
-
-    if (m_error == 0)
-    {
-        if ((u.field_set & (1<<joyent::UF_SCHEMA)) != 0)
-        {
-            scheme_ = String (
-                buf + u.field_data [joyent::UF_SCHEMA].off,
-                    u.field_data [joyent::UF_SCHEMA].len);
-        }
-
-        if ((u.field_set & (1<<joyent::UF_HOST)) != 0)
-        {
-            host_ = String (
-                buf + u.field_data [joyent::UF_HOST].off,
-                    u.field_data [joyent::UF_HOST].len);
-        }
-
-        if ((u.field_set & (1<<joyent::UF_PORT)) != 0)
-        {
-            port_ = u.port;
-            port_string_ = String (
-                buf + u.field_data [joyent::UF_PORT].off,
-                    u.field_data [joyent::UF_PORT].len);
-        }
-        else
-        {
-            port_ = 0;
-        }
-
-        if ((u.field_set & (1<<joyent::UF_PATH)) != 0)
-        {
-            path_ = String (
-                buf + u.field_data [joyent::UF_PATH].off,
-                    u.field_data [joyent::UF_PATH].len);
-        }
-
-        if ((u.field_set & (1<<joyent::UF_QUERY)) != 0)
-        {
-            query_ = String (
-                buf + u.field_data [joyent::UF_QUERY].off,
-                    u.field_data [joyent::UF_QUERY].len);
-        }
-
-        if ((u.field_set & (1<<joyent::UF_FRAGMENT)) != 0)
-        {
-            fragment_ = String (
-                buf + u.field_data [joyent::UF_FRAGMENT].off,
-                    u.field_data [joyent::UF_FRAGMENT].len);
-        }
-
-        if ((u.field_set & (1<<joyent::UF_USERINFO)) != 0)
-        {
-            userinfo_ = String (
-                buf + u.field_data [joyent::UF_USERINFO].off,
-                    u.field_data [joyent::UF_USERINFO].len);
-        }
-
-        m_url = URL (
-            scheme_,
-            host_,
-            port_,
-            port_string_,
-            path_,
-            query_,
-            fragment_,
-            userinfo_);
-    }
-}
-
-ParsedURL::ParsedURL (int error, URL const& url)
-    : m_error (error)
-    , m_url (url)
-{
-}
-
-ParsedURL::ParsedURL (ParsedURL const& other)
-    : m_error (other.m_error)
-    , m_url (other.m_url)
-{
-}
-
-ParsedURL& ParsedURL::operator= (ParsedURL const& other)
-{
-    m_error = other.m_error;
-    m_url = other.m_url;
-    return *this;
-}
-
-int ParsedURL::error () const
-{
-    return m_error;
-}
-
-URL ParsedURL::url () const
-{
-    return m_url;
-}
-
-}

beast/http/impl/URL.cpp

@@ -21,19 +21,15 @@
 
 namespace beast {
 
-URL::URL ()
-    : m_port (0)
-{
-}
 URL::URL (
-    String scheme_,
+    std::string scheme_,
-    String host_,
+    std::string host_,
     std::uint16_t port_,
-    String port_string_,
+    std::string port_string_,
-    String path_,
+    std::string path_,
-    String query_,
+    std::string query_,
-    String fragment_,
+    std::string fragment_,
-    String userinfo_)
+    std::string userinfo_)
         : m_scheme (scheme_)
         , m_host (host_)
         , m_port (port_)
@@ -45,122 +41,189 @@ URL::URL (
 {
 }
 
-URL::URL (URL const& other)
-    : m_scheme (other.m_scheme)
-    , m_host (other.m_host)
-    , m_port (other.m_port)
-    , m_port_string (other.m_port_string)
-    , m_path (other.m_path)
-    , m_query (other.m_query)
-    , m_fragment (other.m_fragment)
-    , m_userinfo (other.m_userinfo)
-{
-}
-
-URL& URL::operator= (URL const& other)
-{
-    m_scheme = other.m_scheme;
-    m_host = other.m_host;
-    m_port = other.m_port;
-    m_port_string = other.m_port_string;
-    m_path = other.m_path;
-    m_query = other.m_query;
-    m_fragment = other.m_fragment;
-    m_userinfo = other.m_userinfo;
-    return *this;
-}
-
 //------------------------------------------------------------------------------
 
-bool URL::empty () const
+bool
+URL::empty () const
 {
-    return m_scheme == String::empty;
+    return m_scheme.empty ();
 }
 
-String URL::scheme () const
+std::string
+const& URL::scheme () const
 {
     return m_scheme;
 }
 
-String URL::host () const
+std::string
+const& URL::host () const
 {
     return m_host;
 }
 
-String URL::port_string () const
+std::string
+const& URL::port_string () const
 {
     return m_port_string;
 }
 
-std::uint16_t URL::port () const
+std::uint16_t
+URL::port () const
 {
     return m_port;
 }
 
-String URL::path () const
+std::string
+const& URL::path () const
 {
     return m_path;
 }
 
-String URL::query () const
+std::string
+const& URL::query () const
 {
     return m_query;
 }
 
-String URL::fragment () const
+std::string
+const& URL::fragment () const
 {
     return m_fragment;
 }
 
-String URL::userinfo () const
+std::string
+const& URL::userinfo () const
 {
     return m_userinfo;
 }
 
 //------------------------------------------------------------------------------
-/*
+std::pair<bool, URL>
-    From
+parse_URL(std::string const& url)
-        http://en.wikipedia.org/wiki/URI_scheme
-
-    <scheme name> : <hierarchical part> [ ? <query> ] [ # <fragment> ]
-
-    e.g.
-
-    foo://username:password@example.com:8042/over/there/index.dtb?type=animal&name=narwhal#nose
-*/
-String URL::toString () const
 {
-    String s;
+    std::size_t const buflen (url.size ());
+    char const* const buf (url.c_str ());
 
-    s = scheme () + "://";
+    joyent::http_parser_url parser;
 
-    if (userinfo () != String::empty)
+    if (joyent::http_parser_parse_url (buf, buflen, false, &parser) != 0)
-        s = userinfo () + "@";
+        return std::make_pair (false, URL{});
 
-    s = s + host ();
+    std::string scheme;
+    std::string host;
+    std::uint16_t port (0);
+    std::string port_string;
+    std::string path;
+    std::string query;
+    std::string fragment;
+    std::string userinfo;
 
-    if (port () != 0)
+    if ((parser.field_set & (1<<joyent::UF_SCHEMA)) != 0)
-        s = s + ":" + String::fromNumber (port ());
+    {
+        scheme = std::string (
+            buf + parser.field_data [joyent::UF_SCHEMA].off,
+                parser.field_data [joyent::UF_SCHEMA].len);
+    }
 
-    s = s + path ();
+    if ((parser.field_set & (1<<joyent::UF_HOST)) != 0)
+    {
+        host = std::string (
+            buf + parser.field_data [joyent::UF_HOST].off,
+                parser.field_data [joyent::UF_HOST].len);
+    }
 
-    if (query () != String::empty)
+    if ((parser.field_set & (1<<joyent::UF_PORT)) != 0)
-        s = "?" + query ();
+    {
+        port = parser.port;
+        port_string = std::string (
+            buf + parser.field_data [joyent::UF_PORT].off,
+                parser.field_data [joyent::UF_PORT].len);
+    }
 
-    if (fragment () != String::empty)
+    if ((parser.field_set & (1<<joyent::UF_PATH)) != 0)
-        s = "#" + fragment ();
+    {
+        path = std::string (
+            buf + parser.field_data [joyent::UF_PATH].off,
+                parser.field_data [joyent::UF_PATH].len);
+    }
+
+    if ((parser.field_set & (1<<joyent::UF_QUERY)) != 0)
+    {
+        query = std::string (
+            buf + parser.field_data [joyent::UF_QUERY].off,
+                parser.field_data [joyent::UF_QUERY].len);
+    }
+
+    if ((parser.field_set & (1<<joyent::UF_FRAGMENT)) != 0)
+    {
+        fragment = std::string (
+            buf + parser.field_data [joyent::UF_FRAGMENT].off,
+                parser.field_data [joyent::UF_FRAGMENT].len);
+    }
+
+    if ((parser.field_set & (1<<joyent::UF_USERINFO)) != 0)
+    {
+        userinfo = std::string (
+            buf + parser.field_data [joyent::UF_USERINFO].off,
+                parser.field_data [joyent::UF_USERINFO].len);
+    }
+
+    return std::make_pair (true,
+        URL {scheme, host, port, port_string, path, query, fragment, userinfo});
+}
+
+std::string
+to_string (URL const& url)
+{
+    std::string s;
+
+    if (!url.empty ())
+    {
+        // Pre-allocate enough for components and inter-component separators
+        s.reserve (
+            url.scheme ().length () + url.userinfo ().length () +
+            url.host ().length () + url.port_string ().length () +
+            url.query ().length () + url.fragment ().length () + 16);
+
+        s.append (url.scheme ());
+        s.append ("://");
+        
+        if (!url.userinfo ().empty ())
+        {
+            s.append (url.userinfo ());
+            s.append ("@");
+        }
+
+        s.append (url.host ());
+
+        if (url.port ())
+        {
+            s.append (":");
+            s.append (url.port_string ());
+        }
+
+        s.append (url.path ());
+
+        if (!url.query ().empty ())
+        {
+            s.append ("?");
+            s.append (url.query ());
+        }
+
+        if (!url.fragment ().empty ())
+        {
+            s.append ("#");
+            s.append (url.fragment ());
+        }
+    }
 
     return s;
 }
 
-std::string URL::to_string() const
+std::ostream&
+operator<< (std::ostream &os, URL const& url)
 {
-    return toString().toStdString();
+    os << to_string (url);
-}
-
-std::ostream& operator<< (std::ostream &os, URL const& url)
-{
-    os << url.to_string();
     return os;
 }
 

beast/http/tests/URL.test.cpp

@@ -17,35 +17,48 @@
 */
 //==============================================================================
 
-#include <beast/http/ParsedURL.h>
+#include <beast/http/URL.h>
 
 #include <beast/unit_test/suite.h>
 
-#include <beast/http/impl/joyent_parser.h>
-
 namespace beast {
 
-class ParsedURL_test : public unit_test::suite
+class URL_test : public unit_test::suite
 {
 public:
-    void checkURL (String const& url)
+    void check_url_parsing (std::string const& url, bool expected)
     {
-        ParsedURL result (url);
+        auto result = parse_URL (url);
-        expect (result.error () == 0);
+
-        expect (result.url ().toString () == url);
+        expect (result.first == expected,
+            (expected ? "Failed to parse " : "Succeeded in parsing ") + url);
+        expect (to_string (result.second) == url);
     }
 
-    void testURL ()
+    void test_url_parsing ()
     {
-        checkURL ("http://www.boost.org/doc/libs/1_54_0/doc/html/boost_asio/reference.html");
+        char const* const urls[] = 
+        {
+            "http://en.wikipedia.org/wiki/URI#Examples_of_URI_references",
+            "ftp://ftp.funet.fi/pub/standards/RFC/rfc959.txt"
+            "ftp://test:test@example.com:21/path/specifier/is/here"
+            "http://www.boost.org/doc/libs/1_54_0/doc/html/boost_asio/reference.html",
+            "foo://username:password@example.com:8042/over/there/index.dtb?type=animal&name=narwhal#nose",
+        };
+
+        testcase ("URL parsing");
+
+        for (auto url : urls)
+            check_url_parsing (url, true);
     }
 
-    void run ()
+    void
+    run ()
     {
-        testURL ();
+        test_url_parsing ();
     }
 };
 
-BEAST_DEFINE_TESTSUITE(ParsedURL,http,beast);
+BEAST_DEFINE_TESTSUITE(URL,http,beast);
 
 }

beast/module/asio/asio.h

@@ -27,7 +27,6 @@
 #include <beast/module/asio/system/BoostIncludes.h>
 
 #include <beast/http/URL.h>
-#include <beast/http/ParsedURL.h>
 
 #include <beast/asio/IPAddressConversion.h>
 

beast/module/asio/http/HTTPClientType.cpp

@@ -148,14 +148,14 @@ public:
         if (url.port () != 0)
         {
             return Query (
-                url.host().toStdString(),
+                url.host(),
-                url.port_string().toStdString(),
+                url.port_string(),
                 Query::numeric_service);
         }
 
         return Query (
-            url.host().toStdString(),
+            url.host(),
-            url.scheme().toStdString());
+            url.scheme());
     }
 
     //--------------------------------------------------------------------------
@@ -648,7 +648,7 @@ public:
             HTTPClientBase::New (Journal(), timeoutSeconds));
 
         HTTPClientBase::result_type const& result (
-            client->get (ParsedURL (s).url ()));
+            client->get (parse_URL (s.toStdString ()).second));
 
         print (result.first, result.second);
     }
@@ -659,7 +659,7 @@ public:
         std::unique_ptr <HTTPClientBase> client (
             HTTPClientBase::New (Journal(), timeoutSeconds));
 
-        client->async_get (t.get_io_service (), ParsedURL (s).url (),
+        client->async_get (t.get_io_service (), parse_URL (s.toStdString ()).second,
             std::bind (&HTTPClient_test::handle_get, this,
                 std::placeholders::_1));