- Move HttpClient to cpputils

- Fix VersionChecker behaviour when the returned json is invalid
- Add test cases for VersionChecker

CMakeLists.txt

@@ -12,8 +12,6 @@ ADD_BOOST(program_options chrono)
 
 ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64)
 
-TARGET_LINK_LIBRARIES(${BII_BLOCK_TARGET} INTERFACE curl)
-
 GIT_VERSION_INIT()
 
 ENABLE_STYLE_WARNINGS()

src/cli/HttpClient.cpp

@@ -1,50 +0,0 @@
-// Base version taken from https://techoverflow.net/blog/2013/03/15/c-simple-http-download-using-libcurl-easy-api/
-/**
- * HTTPDownloader.cpp
- *
- * A simple C++ wrapper for the libcurl easy API.
- *
- * Written by Uli Köhler (techoverflow.net)
- * Published under CC0 1.0 Universal (public domain)
- */
-#include "HttpClient.h"
-#include <sstream>
-#include <iostream>
-#include <curl/curl.h>
-#include <curl/easy.h>
-
-using boost::none;
-using boost::optional;
-using std::string;
-using std::ostringstream;
-
-size_t HttpClient::write_data(void *ptr, size_t size, size_t nmemb, ostringstream *stream) {
-    stream->write((const char*)ptr, size*nmemb);
-    return size * nmemb;
-}
-
-HttpClient::HttpClient() {
-    curl = curl_easy_init();
-}
-
-HttpClient::~HttpClient() {
-    curl_easy_cleanup(curl);
-}
-
-optional<string> HttpClient::get(const string& url) {
-    curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
-    // example.com is redirected, so we tell libcurl to follow redirection
-    curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
-    curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1); //Prevent "longjmp causes uninitialized stack frame" bug
-    curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, "deflate");
-    ostringstream out;
-    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &HttpClient::write_data);
-    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &out);
-    // Perform the request, res will get the return code
-    CURLcode res = curl_easy_perform(curl);
-    // Check for errors
-    if (res != CURLE_OK) {
-        return none;
-    }
-    return out.str();
-}

src/cli/HttpClient.h

@@ -1,41 +0,0 @@
-// Base version taken from https://techoverflow.net/blog/2013/03/15/c-simple-http-download-using-libcurl-easy-api/
-/**
- * HTTPDownloader.hpp
- *
- * A simple C++ wrapper for the libcurl easy API.
- *
- * Written by Uli Köhler (techoverflow.net)
- * Published under CC0 1.0 Universal (public domain)
- */
-#pragma once
-#ifndef MESSMER_CRYFS_SRC_CLI_HTTPCLIENT_HPP
-#define MESSMER_CRYFS_SRC_CLI_HTTPCLIENT_HPP
-
-#include <string>
-#include <boost/optional.hpp>
-#include <messmer/cpp-utils/macros.h>
-
-//TODO Test
-
-/**
- * A non-threadsafe simple libcURL-easy based HTTP downloader
- */
-class HttpClient final {
-public:
-    HttpClient();
-    ~HttpClient();
-    /**
-     * Download a file using HTTP GET and store in in a std::string
-     * @param url The URL to download
-     * @return The download result
-     */
-    boost::optional<std::string> get(const std::string& url);
-private:
-    void* curl;
-
-    static size_t write_data(void *ptr, size_t size, size_t nmemb, std::ostringstream *stream);
-
-    DISALLOW_COPY_AND_ASSIGN(HttpClient);
-};
-
-#endif

src/cli/VersionChecker.cpp

@@ -1,6 +1,6 @@
 #include "VersionChecker.h"
 #include <sstream>
-#include "HttpClient.h"
+#include <messmer/cpp-utils/network/CurlHttpClient.h>
 #include <boost/property_tree/json_parser.hpp>
 #include <messmer/cpp-utils/logging/logging.h>
 #include <boost/foreach.hpp>
@@ -8,14 +8,22 @@
 using boost::optional;
 using boost::none;
 using std::string;
+using std::shared_ptr;
+using std::make_shared;
+using cpputils::HttpClient;
+using cpputils::CurlHttpClient;
 using boost::property_tree::ptree;
 using boost::property_tree::json_parser_error;
 using namespace cpputils::logging;
 
-#include <iostream>
 namespace cryfs {
 
-    VersionChecker::VersionChecker(): _versionInfo(_getVersionInfo()) {
+    VersionChecker::VersionChecker()
+            :VersionChecker(make_shared<CurlHttpClient>()) {
+    }
+
+    VersionChecker::VersionChecker(shared_ptr<HttpClient> httpClient)
+            : _versionInfo(_getVersionInfo(std::move(httpClient))) {
     }
 
     optional<string> VersionChecker::newestVersion() const {
@@ -33,7 +41,11 @@ namespace cryfs {
         if (_versionInfo == none) {
             return none;
         }
-        BOOST_FOREACH(const ptree::value_type &v, _versionInfo->get_child("warnings")) {
+        auto warnings = _versionInfo->get_child_optional("warnings");
+        if (warnings == none) {
+            return none;
+        }
+        BOOST_FOREACH(const ptree::value_type &v, *warnings) {
             if(v.first == version) {
                 return v.second.get_value<std::string>();
             }
@@ -41,10 +53,9 @@ namespace cryfs {
         return none;
     }
 
-    optional<ptree> VersionChecker::_getVersionInfo() {
+    optional<ptree> VersionChecker::_getVersionInfo(shared_ptr<HttpClient> httpClient) {
-        optional<string> response = HttpClient().get("http://www.cryfs.org/version_info.json");
+        optional<string> response = httpClient->get("http://www.cryfs.org/version_info.json");
         if (response == none) {
-            std::cout << "no response" << std::endl;
             return none;
         }
         return _parseJson(*response);

src/cli/VersionChecker.h

@@ -5,6 +5,7 @@
 #include <string>
 #include <boost/optional.hpp>
 #include <boost/property_tree/ptree.hpp>
+#include <messmer/cpp-utils/network/HttpClient.h>
 
 namespace cryfs {
     //TODO Test
@@ -12,11 +13,13 @@ namespace cryfs {
     class VersionChecker final {
     public:
         VersionChecker();
+        //TODO Write a cpputils::shared_ref and use it
+        VersionChecker(std::shared_ptr<cpputils::HttpClient> httpClient);
 
         boost::optional<std::string> newestVersion() const;
         boost::optional<std::string> securityWarningFor(const std::string &version) const;
     private:
-        static boost::optional<boost::property_tree::ptree> _getVersionInfo();
+        static boost::optional<boost::property_tree::ptree> _getVersionInfo(std::shared_ptr<cpputils::HttpClient> httpClient);
         static boost::optional<boost::property_tree::ptree> _parseJson(const std::string &json);
 
         boost::optional<boost::property_tree::ptree> _versionInfo;

test/cli/VersionCheckerTest.cpp

@@ -0,0 +1,125 @@
+#include <google/gtest/gtest.h>
+#include "../../src/cli/VersionChecker.h"
+#include <messmer/cpp-utils/network/FakeHttpClient.h>
+#include <messmer/cpp-utils/pointer/unique_ref.h>
+
+using std::shared_ptr;
+using std::make_shared;
+using std::string;
+using cpputils::FakeHttpClient;
+using cpputils::unique_ref;
+using cpputils::make_unique_ref;
+using boost::none;
+using namespace cryfs;
+
+class VersionCheckerTest: public ::testing::Test {
+public:
+    unique_ref<VersionChecker> versionChecker() {
+        return make_unique_ref<VersionChecker>(http);
+    }
+
+    void setVersionInfo(const string &versionInfo) {
+        http->addWebsite("http://www.cryfs.org/version_info.json", versionInfo);
+    }
+
+private:
+    shared_ptr<FakeHttpClient> http = make_shared<FakeHttpClient>();
+};
+
+TEST_F(VersionCheckerTest, NewestVersion_NoInternet) {
+    EXPECT_EQ(none, versionChecker()->newestVersion());
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_NoInternet) {
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8"));
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_NoWarnings_1) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"}}");
+    EXPECT_EQ("0.8.2", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_NoWarnings_2) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.3\"}}");
+    EXPECT_EQ("0.8.3", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_EmptyWarnings) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{}}");
+    EXPECT_EQ("0.8.2", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_WarningsOtherVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.1\": \"warning\"}}");
+    EXPECT_EQ("0.8.2", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_WarningsSameVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.2\": \"warning\"}}");
+    EXPECT_EQ("0.8.2", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_WarningsSameAndOtherVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.1\": \"warning1\", \"0.8.2\": \"warning2\", \"0.8.3\": \"warning3\"}}");
+    EXPECT_EQ("0.8.2", versionChecker()->newestVersion().value());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_BlankVersionInfo) {
+    setVersionInfo("");
+    EXPECT_EQ(none, versionChecker()->newestVersion());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_EmptyVersionInfo) {
+    setVersionInfo("{}");
+    EXPECT_EQ(none, versionChecker()->newestVersion());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_InvalidVersionInfo) {
+    setVersionInfo("invalid-json");
+    EXPECT_EQ(none, versionChecker()->newestVersion());
+}
+
+TEST_F(VersionCheckerTest, NewestVersion_MissingKey) {
+    setVersionInfo("{\"warnings\":{}");
+    EXPECT_EQ(none, versionChecker()->newestVersion());
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_NoWarnings) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"}}");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_EmptyWarnings) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{}}");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_WarningsOtherVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.1\": \"warning\"}}");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_WarningsSameVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.2\": \"warning\"}}");
+    EXPECT_EQ("warning", versionChecker()->securityWarningFor("0.8.2").value());
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_WarningsSameAndOtherVersion) {
+    setVersionInfo("{\"version_info\":{\"current\":\"0.8.2\"},\"warnings\":{\"0.8.1\": \"warning1\", \"0.8.2\": \"warning2\", \"0.8.3\": \"warning3\"}}");
+    EXPECT_EQ("warning2", versionChecker()->securityWarningFor("0.8.2").value());
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_BlankVersionInfo) {
+    setVersionInfo("");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_EmptyVersionInfo) {
+    setVersionInfo("{}");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}
+
+TEST_F(VersionCheckerTest, SecurityWarningFor_InvalidVersionInfo) {
+    setVersionInfo("invalid-json");
+    EXPECT_EQ(none, versionChecker()->securityWarningFor("0.8.2"));
+}