Merge lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction into lp:unity-scopes-api/devel

[Marcus Tomlinson]

Don't assume the port to be at the very end of a url string.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:227
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/140/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/140
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/140
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/140/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/140

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/140/rebuild

review: Approve (continuous-integration)

[Michi Henning (michihenning)]

Strictly speaking, for strings, you should use string::size_type, not uint64_t. (For example, on a 32-bit platform, uint64_t needlessly forces a 64-bit type when a 32-bit type would be sufficient.)

    // find the last occurrence of ':' in the url in order to extract the port number
    // * ignore the colon after "http"/"https"

    const size_t hier_pos = strlen("https");

    uint64_t port_pos = base_url.find_last_of(':');
    if (port_pos != std::string::npos && port_pos > hier_pos)

What happens if the URL is of the form "http://somewhere.org/some/path"? port_pos will be 4 in that case, which is not greater than hier_pos.

Just checking. I find it almost impossible to understand string parsing code such as this by just reading it. Not your fault, of course. The C++ string class would have to be one of the worst string classes anyone has ever come up with :-(

review: Needs Information

[lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction]

228. By Marcus Tomlinson on 2014-02-24

Use std::string::size_type for string find positions

[Marcus Tomlinson (marcustomlinson)]

> Strictly speaking, for strings, you should use string::size_type, not
> uint64_t. (For example, on a 32-bit platform, uint64_t needlessly forces a
> 64-bit type when a 32-bit type would be sufficient.)

ok, I've made that change.

> // find the last occurrence of ':' in the url in order to extract the port
> number
> // * ignore the colon after "http"/"https"
>
> const size_t hier_pos = strlen("https");
>
> uint64_t port_pos = base_url.find_last_of(':');
> if (port_pos != std::string::npos && port_pos > hier_pos)
>
> What happens if the URL is of the form "http://somewhere.org/some/path"?
> port_pos will be 4 in that case, which is not greater than hier_pos.

This is correct. That if statement is determining if there IS a port in the url. As "http://somewhere.org/some/path" does not have a port in the name, the else logic is executed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:228
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/142/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/142
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/142
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/142/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/142

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/142/rebuild

review: Approve (continuous-integration)

[lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction]

229. By Marcus Tomlinson on 2014-02-24

Added some more comments

230. By Marcus Tomlinson on 2014-02-24

Further clarification of string logic

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:229
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/144/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/144
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/144
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/144/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/144

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/144/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:230
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/145/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/145
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/145
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/145/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/145

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/145/rebuild

review: Approve (continuous-integration)

[Paweł Stołowski (stolowski)]

I agree, it's very hard to tell if this parsing is correct and handles all cases; making any adjustments to it without risking breaking something in the future will be tricky . Could you please add some tests that exercise this parser with different uris (including broken ones), with and without SMART_SCOPES_SERVER env var set and with default and non-default port?
Perhaps it would make sense to move this parser out from SmartScopesClient to a utility function to make testing easy (and also in case we need another SmartScopesClient ctor)? But I'm not going to insist on that, you can very well just extend existing SmartScopesClient tests.

review: Needs Fixing

[lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction]

231. By Marcus Tomlinson on 2014-02-25

Moved url parsing into set_url() method, and added set_port(), port(), and url() methods.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:231
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/152/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/152
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/152
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/152/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/152

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/152/rebuild

review: Approve (continuous-integration)

[lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction]

232. By Marcus Tomlinson on 2014-02-25

reset_port() and reset_url() better describes what these methods are doing.

233. By Marcus Tomlinson on 2014-02-25

Added valgrind suppressions for putenv and getenv.
Added tests for url parsing to SmartScopesClientTest.

234. By Marcus Tomlinson on 2014-02-25

Spelling mistake in comments

235. By Marcus Tomlinson on 2014-02-25

Removed "TODO" comment

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:234
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/153/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/153
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/153
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/153/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/153

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/153/rebuild

review: Approve (continuous-integration)

[Paweł Stołowski (stolowski)]

Good stuff, thanks for the tests!

One more minor thing:

> What happens if the URL is of the form "http://somewhere.org/some/path"?
> port_pos will be 4 in that case, which is not greater than hier_pos.
> This is correct. That if statement is determining if there IS a port in the url. As > > >"http://somewhere.org/some/path" does not have a port in the name, the else logic is >executed.

Could you please add a comment in the code explaining it's ok for http as well?

review: Needs Fixing

[lp:~marcustomlinson/unity-scopes-api/fix_url_port_extraction]

236. By Marcus Tomlinson on 2014-02-25

Added comment better explaining how we determine that there is a port in the url.

[Marcus Tomlinson (marcustomlinson)]

> Good stuff, thanks for the tests!
>
> One more minor thing:
>
> > What happens if the URL is of the form "http://somewhere.org/some/path"?
> > port_pos will be 4 in that case, which is not greater than hier_pos.
> > This is correct. That if statement is determining if there IS a port in the
> url. As > > >"http://somewhere.org/some/path" does not have a port in the
> name, the else logic is >executed.
>
> Could you please add a comment in the code explaining it's ok for http as
> well?

Ok I've added "// if there is a port specified in the url (i.e. a colon occurs after "http:" / "https:")" before the if statement that checks for port.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:235
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/154/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/154
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/154/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/154

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/154/rebuild

review: Approve (continuous-integration)

[Paweł Stołowski (stolowski)]

+1

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:236
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/155/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-amd64-ci/155
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/155
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-armhf-ci/155/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-trusty-i386-ci/155

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/155/rebuild

review: Approve (continuous-integration)

=== modified file 'include/unity/scopes/internal/smartscopes/SmartScopesClient.h'
--- include/unity/scopes/internal/smartscopes/SmartScopesClient.h	2014-02-20 19:19:25 +0000
+++ include/unity/scopes/internal/smartscopes/SmartScopesClient.h	2014-02-25 09:40:06 +0000
@@ -128,6 +128,11 @@
 
     virtual ~SmartScopesClient();
 
+    void reset_url(std::string const& url = "");
+    void reset_port(uint port = 0);
+    std::string url();
+    uint port();
+
     bool get_remote_scopes(std::vector<RemoteScope>& scopes, std::string const& locale = "", bool caching_enabled = true);
 
     SearchHandle::UPtr search(std::string const& base_url,

=== modified file 'src/scopes/internal/smartscopes/SmartScopesClient.cpp'
--- src/scopes/internal/smartscopes/SmartScopesClient.cpp	2014-02-20 19:19:25 +0000
+++ src/scopes/internal/smartscopes/SmartScopesClient.cpp	2014-02-25 09:40:06 +0000
@@ -93,38 +93,32 @@
                                      uint port)
     : http_client_(http_client)
     , json_node_(json_node)
-    , url_(url)
-    , port_(port)
+    , port_(0)
     , have_latest_cache_(false)
     , query_counter_(0)
 {
-    // initialise url_
-    if (url_.empty())
-    {
-        char* base_url_env = ::getenv("SMART_SCOPES_SERVER");
-        std::string base_url = base_url_env ? base_url_env : "";
-        if (!base_url.empty())
-        {
-            // find the last occurrence of ':' in the url in order to extract the port number
-            // * ignore the colon after "http"/"https"
-
-            const size_t hier_pos = strlen("https");
-
-            uint64_t found = base_url.find_last_of(':');
-            if (found != std::string::npos && found > hier_pos)
-            {
-                url_ = base_url.substr(0, found);
-                port_ = std::stoi(base_url.substr(found + 1));
-            }
-            else
-            {
-                url_ = base_url;
-            }
-        }
-        else
-        {
-            url_ = c_base_url;
-        }
+    try
+    {
+        // try to set url from url supplied
+        reset_url(url);
+    }
+    catch(...)
+    {
+        try
+        {
+            // url supplied failed, try to set url automatically
+            reset_url();
+        }
+        catch(...)
+        {
+            std::cerr << "SmartScopesClient::SmartScopesClient: Failed to initialise SSS url" << std::endl;
+        }
+    }
+
+    // force set a port only if one was explicitly provided
+    if (port != 0)
+    {
+        reset_port(port);
     }
 
     // initialise cached_scopes_
@@ -135,6 +129,88 @@
 {
 }
 
+void SmartScopesClient::reset_url(std::string const& url)
+{
+    std::string base_url = url;
+
+    // if a url was not provided, get the environment variable
+    if (base_url.empty())
+    {
+        char* sss_url_env = ::getenv("SMART_SCOPES_SERVER");
+        base_url = sss_url_env ? sss_url_env : "";
+
+        // if the env var was not provided, use the c_base_url constant
+        if (base_url.empty())
+        {
+            base_url = c_base_url;
+        }
+    }
+
+    // find the last occurrence of ':' in the url in order to extract the port number
+    // * ignore the colon after "http" / "https"
+
+    const size_t hier_pos = strlen("https");
+    std::string::size_type port_pos = base_url.find_last_of(':');
+
+    // if there is a port specified in the url (i.e. a colon occurs after "http:" / "https:")
+    if (port_pos != std::string::npos && port_pos > hier_pos)
+    {
+        url_ = base_url.substr(0, port_pos);
+        std::string port_str;
+        std::string::size_type url_cont = base_url.find('/', port_pos);
+
+        // if the address continues after the port
+        if (url_cont != std::string::npos)
+        {
+            // extract port, and add the rest of the address to url_
+            url_ += base_url.substr(url_cont);
+            port_str = base_url.substr(port_pos + 1, url_cont - port_pos - 1);
+        }
+        // else if the remainder of the string is just the port
+        else
+        {
+            // extract port
+            port_str = base_url.substr(port_pos + 1);
+        }
+
+        // check if port_str is actually a number before setting port_
+        if (!port_str.empty() &&
+                std::find_if(begin(port_str), end(port_str),
+                             [](char const& c){ return !std::isdigit(c); }) == port_str.end())
+        {
+            port_ = std::stoi(port_str);
+        }
+        else
+        {
+            // the port supplied is not a number, don't trust the url either
+            url_ = "";
+            port_ = 0;
+            throw unity::InvalidArgumentException("Invalid url: " + url);
+        }
+    }
+    // else if there is no port specified in the url
+    else
+    {
+        url_ = base_url;
+        port_ = 0;
+    }
+}
+
+void SmartScopesClient::reset_port(uint port)
+{
+    port_ = port;
+}
+
+std::string SmartScopesClient::url()
+{
+    return url_;
+}
+
+uint SmartScopesClient::port()
+{
+    return port_;
+}
+
 // returns false if cache used
 bool SmartScopesClient::get_remote_scopes(std::vector<RemoteScope>& remote_scopes, std::string const& locale, bool caching_enabled)
 {

=== modified file 'test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp'
--- test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp	2014-02-20 19:19:25 +0000
+++ test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp	2014-02-25 09:40:06 +0000
@@ -190,4 +190,152 @@
     EXPECT_EQ(2, results.size());
 }
 
+TEST_F(SmartScopesClientTest, url_parsing)
+{
+    // check initial values to be expected test values
+    EXPECT_EQ(server_.port_, ssc_->port());
+    EXPECT_EQ("http://127.0.0.1", ssc_->url());
+
+    // empty the environment var (in case there already is one set)
+    std::string server_url_env = "SMART_SCOPES_SERVER=";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    // reset url and check that we now have contant values
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("https://productsearch.ubuntu.com/smartscopes/v2", ssc_->url());
+
+    // http addr, no port
+    EXPECT_NO_THROW(ssc_->reset_url("http://hello.com/there"));
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("http://hello.com/there", ssc_->url());
+
+    // https addr, no port
+    EXPECT_NO_THROW(ssc_->reset_url("https://hello.com/there"));
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(1500));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("https://hello.com/there", ssc_->url());
+
+    // http addr, with port (end)
+    EXPECT_NO_THROW(ssc_->reset_url("http://hello.com:1500"));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("http://hello.com", ssc_->url());
+
+    // https addr, with port (end)
+    EXPECT_NO_THROW(ssc_->reset_url("https://hello.com:1500"));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(2000));
+    EXPECT_EQ(2000, ssc_->port());
+    EXPECT_EQ("https://hello.com", ssc_->url());
+
+    // http addr, with port (mid)
+    EXPECT_NO_THROW(ssc_->reset_url("http://hello.com:1500/there"));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("http://hello.com/there", ssc_->url());
+
+    // https addr, with port (mid)
+    EXPECT_NO_THROW(ssc_->reset_url("https://hello.com:1500/there"));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("https://hello.com/there", ssc_->url());
+
+    // https addr, with broken port
+    EXPECT_THROW(ssc_->reset_url("https://hello.com:15d00/there"), std::exception);
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("", ssc_->url());
+
+    // https addr, with floating colon
+    EXPECT_THROW(ssc_->reset_url("https://hello.com:1500/the:re"), std::exception);
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(1500));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("", ssc_->url());
+}
+
+TEST_F(SmartScopesClientTest, url_parsing_env_var)
+{
+    // check initial values to be expected test values
+    EXPECT_EQ(server_.port_, ssc_->port());
+    EXPECT_EQ("http://127.0.0.1", ssc_->url());
+
+    // empty the environment var (in case there already is one set)
+    std::string server_url_env = "SMART_SCOPES_SERVER=";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    // reset url and check that we now have contant values
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("https://productsearch.ubuntu.com/smartscopes/v2", ssc_->url());
+
+    // env var, http addr, no port
+    server_url_env = "SMART_SCOPES_SERVER=http://hello.com/there";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("http://hello.com/there", ssc_->url());
+
+    // env var, https addr, no port
+    server_url_env = "SMART_SCOPES_SERVER=https://hello.com/there";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(1500));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("https://hello.com/there", ssc_->url());
+
+    // env var, http addr, with port (end)
+    server_url_env = "SMART_SCOPES_SERVER=http://hello.com:1500";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("http://hello.com", ssc_->url());
+
+    // env var, https addr, with port (end)
+    server_url_env = "SMART_SCOPES_SERVER=https://hello.com:1500";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(2000));
+    EXPECT_EQ(2000, ssc_->port());
+    EXPECT_EQ("https://hello.com", ssc_->url());
+
+    // env var, http addr, with port (mid)
+    server_url_env = "SMART_SCOPES_SERVER=http://hello.com:1500/there";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("http://hello.com/there", ssc_->url());
+
+    // env var, https addr, with port (mid)
+    server_url_env = "SMART_SCOPES_SERVER=https://hello.com:1500/there";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_NO_THROW(ssc_->reset_url());
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("https://hello.com/there", ssc_->url());
+
+    // env var, https addr, with broken port
+    server_url_env = "SMART_SCOPES_SERVER=https://hello.com:15d00/there";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_THROW(ssc_->reset_url(), std::exception);
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_EQ("", ssc_->url());
+
+    // env var, https addr, with floating colon
+    server_url_env = "SMART_SCOPES_SERVER=https://hello.com:1500/the:re";
+    ::putenv(const_cast<char*>(server_url_env.c_str()));
+
+    EXPECT_THROW(ssc_->reset_url(), std::exception);
+    EXPECT_EQ(0, ssc_->port());
+    EXPECT_NO_THROW(ssc_->reset_port(1500));
+    EXPECT_EQ(1500, ssc_->port());
+    EXPECT_EQ("", ssc_->url());
+}
+
 } // namespace

=== modified file 'test/gtest/scopes/internal/smartscopes/smartscopesproxy/smartscopesproxy_test.cpp'
--- test/gtest/scopes/internal/smartscopes/smartscopesproxy/smartscopesproxy_test.cpp	2014-02-20 19:19:25 +0000
+++ test/gtest/scopes/internal/smartscopes/smartscopesproxy/smartscopesproxy_test.cpp	2014-02-25 09:40:06 +0000
@@ -41,8 +41,6 @@
 using namespace unity::scopes::internal::smartscopes;
 using namespace unity::test::scopes::internal::smartscopes;
 
-///! TODO: more tests
-
 namespace
 {
 

=== modified file 'valgrind-suppress'
--- valgrind-suppress	2014-02-03 11:29:47 +0000
+++ valgrind-suppress	2014-02-25 09:40:06 +0000
@@ -72,7 +72,6 @@
     obj:/lib/x86_64-linux-gnu/ld-2.18.so
 }
 
-
 # False positives for memory leaks in Qt
 
 {
@@ -99,7 +98,6 @@
    ...
    fun:_ZN21QNetworkAccessManager13createRequestENS_9OperationERK15QNetworkRequestP9QIODevice
    fun:_ZN21QNetworkAccessManager3getERK15QNetworkRequest
-   fun:_ZN18HttpClientQtThread3runEv
 }
 
 {
@@ -109,3 +107,21 @@
    ...
    fun:_ZNK14QFactoryLoader8instanceEi
 }
+
+# Bogus "invalid read" reports for ::putenv and ::genenv
+
+{
+   putenv_read
+   Memcheck:Addr1
+   ...
+   fun:putenv
+   ...
+}
+
+{
+   getenv_read
+   Memcheck:Addr2
+   ...
+   fun:getenv
+   ...
+}