Don't keep update check connection open after update check is finished

This commit is contained in:
Sebastian Messmer 2017-09-30 18:53:03 +01:00
parent bd34a04d0c
commit 85759961ef
6 changed files with 37 additions and 32 deletions

View File

@ -72,8 +72,8 @@ using gitversion::VersionCompare;
namespace cryfs {
Cli::Cli(RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, shared_ptr<Console> console, shared_ptr<HttpClient> httpClient):
_keyGenerator(keyGenerator), _scryptSettings(scryptSettings), _console(), _httpClient(httpClient), _noninteractive(false) {
Cli::Cli(RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, shared_ptr<Console> console):
_keyGenerator(keyGenerator), _scryptSettings(scryptSettings), _console(), _noninteractive(false) {
_noninteractive = Environment::isNoninteractive();
if (_noninteractive) {
_console = make_shared<NoninteractiveConsole>(console);
@ -82,7 +82,7 @@ namespace cryfs {
}
}
void Cli::_showVersion() {
void Cli::_showVersion(unique_ref<HttpClient> httpClient) {
cout << "CryFS Version " << gitversion::VersionString() << endl;
if (gitversion::IsDevVersion()) {
cout << "WARNING! This is a development version based on git commit " << gitversion::GitCommitId() <<
@ -99,7 +99,7 @@ namespace cryfs {
} else if (Environment::isNoninteractive()) {
cout << "Automatic checking for security vulnerabilities and updates is disabled in noninteractive mode." << endl;
} else {
_checkForUpdates();
_checkForUpdates(std::move(httpClient));
}
#else
# warning Update checks are disabled. The resulting executable will not go online to check for newer versions or known security vulnerabilities.
@ -107,8 +107,8 @@ namespace cryfs {
cout << endl;
}
void Cli::_checkForUpdates() {
VersionChecker versionChecker(_httpClient);
void Cli::_checkForUpdates(unique_ref<HttpClient> httpClient) {
VersionChecker versionChecker(std::move(httpClient));
optional<string> newestVersion = versionChecker.newestVersion();
if (newestVersion == none) {
cout << "Could not check for updates." << endl;
@ -378,9 +378,9 @@ namespace cryfs {
return false;
}
int Cli::main(int argc, const char *argv[]) {
int Cli::main(int argc, const char *argv[], unique_ref<HttpClient> httpClient) {
cpputils::showBacktraceOnSigSegv();
_showVersion();
_showVersion(std::move(httpClient));
ProgramOptions options = program_options::Parser(argc, argv).parse(CryCiphers::supportedCipherNames());

View File

@ -16,11 +16,11 @@
namespace cryfs {
class Cli final {
public:
Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::shared_ptr<cpputils::Console> console, std::shared_ptr<cpputils::HttpClient> httpClient);
int main(int argc, const char *argv[]);
Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::shared_ptr<cpputils::Console> console);
int main(int argc, const char *argv[], cpputils::unique_ref<cpputils::HttpClient> httpClient);
private:
void _checkForUpdates();
void _checkForUpdates(cpputils::unique_ref<cpputils::HttpClient> httpClient);
void _runFilesystem(const program_options::ProgramOptions &options);
CryConfigLoader::ConfigLoadResult _loadOrCreateConfig(const program_options::ProgramOptions &options);
void _checkConfigIntegrity(const boost::filesystem::path& basedir, const CryConfigFile& config);
@ -32,7 +32,7 @@ namespace cryfs {
static std::string _askPasswordFromStdin(const std::string &prompt);
static bool _confirmPassword(const std::string &password);
static bool _checkPassword(const std::string &password);
void _showVersion();
void _showVersion(cpputils::unique_ref<cpputils::HttpClient> httpClient);
void _initLogfile(const program_options::ProgramOptions &options);
void _sanityChecks(const program_options::ProgramOptions &options);
void _checkMountdirDoesntContainBasedir(const program_options::ProgramOptions &options);
@ -47,7 +47,6 @@ namespace cryfs {
cpputils::RandomGenerator &_keyGenerator;
cpputils::SCryptSettings _scryptSettings;
std::shared_ptr<cpputils::Console> _console;
std::shared_ptr<cpputils::HttpClient> _httpClient;
bool _noninteractive;
DISALLOW_COPY_AND_ASSIGN(Cli);

View File

@ -8,17 +8,16 @@
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 cpputils::unique_ref;
using namespace cpputils::logging;
namespace cryfs {
VersionChecker::VersionChecker(shared_ptr<HttpClient> httpClient)
VersionChecker::VersionChecker(unique_ref<HttpClient> httpClient)
: _versionInfo(_getVersionInfo(std::move(httpClient))) {
}
@ -49,7 +48,7 @@ namespace cryfs {
return none;
}
optional<ptree> VersionChecker::_getVersionInfo(shared_ptr<HttpClient> httpClient) {
optional<ptree> VersionChecker::_getVersionInfo(unique_ref<HttpClient> httpClient) {
long timeoutMsec = 2000;
optional<string> response = httpClient->get("https://www.cryfs.org/version_info.json", timeoutMsec);
if (response == none) {

View File

@ -6,17 +6,18 @@
#include <boost/optional.hpp>
#include <boost/property_tree/ptree.hpp>
#include <cpp-utils/network/HttpClient.h>
#include <cpp-utils/pointer/unique_ref.h>
namespace cryfs {
class VersionChecker final {
public:
//TODO Write a cpputils::shared_ref and use it
VersionChecker(std::shared_ptr<cpputils::HttpClient> httpClient);
VersionChecker(cpputils::unique_ref<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(std::shared_ptr<cpputils::HttpClient> httpClient);
static boost::optional<boost::property_tree::ptree> _getVersionInfo(cpputils::unique_ref<cpputils::HttpClient> httpClient);
static boost::optional<boost::property_tree::ptree> _parseJson(const std::string &json);
boost::optional<boost::property_tree::ptree> _versionInfo;

View File

@ -9,14 +9,15 @@ using cpputils::Random;
using cpputils::SCrypt;
using cpputils::CurlHttpClient;
using cpputils::IOStreamConsole;
using cpputils::make_unique_ref;
using std::make_shared;
using std::cerr;
int main(int argc, const char *argv[]) {
try {
auto &keyGenerator = Random::OSRandom();
return Cli(keyGenerator, SCrypt::DefaultSettings, make_shared<IOStreamConsole>(),
make_shared<CurlHttpClient>()).main(argc, argv);
return Cli(keyGenerator, SCrypt::DefaultSettings, make_shared<IOStreamConsole>())
.main(argc, argv, make_unique_ref<CurlHttpClient>());
} catch (const std::exception &e) {
cerr << "Error: " << e.what();
return EXIT_FAILURE;

View File

@ -281,18 +281,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) {
EXPECT_NE(boost::none, Load());
}
TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenCorrectVersion) {
EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(0);
CreateWithVersion(gitversion::VersionString());
EXPECT_NE(boost::none, Load());
}
TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) {
EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(1).WillOnce(Return(false));
string version = olderVersion();
CreateWithVersion(version);
try {
Load();
EXPECT_TRUE(false); // expect throw
@ -307,9 +296,25 @@ TEST_F(CryConfigLoaderTest, MyClientIdIsIndeterministic) {
uint32_t myClientId = loader("mypassword", true).loadOrCreate(file1.path()).value().myClientId;
EXPECT_NE(myClientId, loader("mypassword", true).loadOrCreate(file2.path()).value().myClientId);
}
TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenCorrectVersion) {
EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(0);
CreateWithVersion(gitversion::VersionString());
EXPECT_NE(boost::none, Load());
}
TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) {
EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(1).WillOnce(Return(false));
string version = olderVersion();
CreateWithVersion(version);
TEST_F(CryConfigLoaderTest, MyClientIdIsLoadedCorrectly) {
TempFile file(false);
uint32_t myClientId = loader("mypassword", true).loadOrCreate(file.path()).value().myClientId;
EXPECT_EQ(myClientId, loader("mypassword", true).loadOrCreate(file.path()).value().myClientId);
}
// TODO Test behavior if
// - filesystem id changed
// - filesystem id correct but encryption key changed
// TODO Add cryfs-cli tests for the same thing