diff --git a/src/cpp-utils/io/NoninteractiveConsole.cpp b/src/cpp-utils/io/NoninteractiveConsole.cpp index ebdd72c1..5c458b12 100644 --- a/src/cpp-utils/io/NoninteractiveConsole.cpp +++ b/src/cpp-utils/io/NoninteractiveConsole.cpp @@ -2,10 +2,11 @@ using std::string; using std::vector; +using std::shared_ptr; namespace cpputils { -NoninteractiveConsole::NoninteractiveConsole(unique_ref baseConsole): _baseConsole(std::move(baseConsole)) { +NoninteractiveConsole::NoninteractiveConsole(shared_ptr baseConsole): _baseConsole(std::move(baseConsole)) { } bool NoninteractiveConsole::askYesNo(const string &/*question*/, bool defaultValue) { diff --git a/src/cpp-utils/io/NoninteractiveConsole.h b/src/cpp-utils/io/NoninteractiveConsole.h index 6d38704f..fc6bf880 100644 --- a/src/cpp-utils/io/NoninteractiveConsole.h +++ b/src/cpp-utils/io/NoninteractiveConsole.h @@ -9,14 +9,14 @@ namespace cpputils { //TODO Add test cases for NoninteractiveConsole class NoninteractiveConsole final: public Console { public: - NoninteractiveConsole(unique_ref baseConsole); + NoninteractiveConsole(std::shared_ptr baseConsole); unsigned int ask(const std::string &question, const std::vector &options) override; bool askYesNo(const std::string &question, bool defaultValue) override; void print(const std::string &output) override; private: - unique_ref _baseConsole; + std::shared_ptr _baseConsole; DISALLOW_COPY_AND_ASSIGN(NoninteractiveConsole); }; diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index c4870959..147004ff 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/src/cryfs-cli/Cli.cpp @@ -75,13 +75,13 @@ using gitversion::VersionCompare; namespace cryfs { - Cli::Cli(RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, unique_ref console, shared_ptr httpClient): + Cli::Cli(RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, shared_ptr console, shared_ptr httpClient): _keyGenerator(keyGenerator), _scryptSettings(scryptSettings), _console(), _httpClient(httpClient), _noninteractive(false) { _noninteractive = Environment::isNoninteractive(); if (_noninteractive) { - _console = make_shared(std::move(console)); + _console = make_shared(console); } else { - _console = cpputils::to_unique_ptr(std::move(console)); + _console = console; } } diff --git a/src/cryfs-cli/Cli.h b/src/cryfs-cli/Cli.h index 5271cd92..84d5a20e 100644 --- a/src/cryfs-cli/Cli.h +++ b/src/cryfs-cli/Cli.h @@ -16,7 +16,7 @@ namespace cryfs { class Cli final { public: - Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, cpputils::unique_ref console, std::shared_ptr httpClient); + Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::shared_ptr console, std::shared_ptr httpClient); int main(int argc, const char *argv[]); private: diff --git a/src/cryfs-cli/main.cpp b/src/cryfs-cli/main.cpp index 3bb8b8ee..162b8c82 100644 --- a/src/cryfs-cli/main.cpp +++ b/src/cryfs-cli/main.cpp @@ -8,7 +8,6 @@ using namespace cryfs; using cpputils::Random; using cpputils::SCrypt; using cpputils::CurlHttpClient; -using cpputils::make_unique_ref; using cpputils::IOStreamConsole; using std::make_shared; using std::cerr; @@ -16,7 +15,7 @@ using std::cerr; int main(int argc, const char *argv[]) { try { auto &keyGenerator = Random::OSRandom(); - return Cli(keyGenerator, SCrypt::DefaultSettings, make_unique_ref(), + return Cli(keyGenerator, SCrypt::DefaultSettings, make_shared(), make_shared()).main(argc, argv); } catch (const std::exception &e) { cerr << "Error: " << e.what(); diff --git a/test/cryfs-cli/CliTest_WrongEnvironment.cpp b/test/cryfs-cli/CliTest_WrongEnvironment.cpp index 316dd60d..5b8ea84b 100644 --- a/test/cryfs-cli/CliTest_WrongEnvironment.cpp +++ b/test/cryfs-cli/CliTest_WrongEnvironment.cpp @@ -118,7 +118,7 @@ TEST_P(CliTest_WrongEnvironment, MountDirIsBaseDir_BothRelative) { TEST_P(CliTest_WrongEnvironment, BaseDir_DoesntExist) { _basedir.remove(); // ON_CALL and not EXPECT_CALL, because this is a death test (i.e. it is forked) and gmock EXPECT_CALL in fork children don't report to parents. - ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?")).WillByDefault(Return(false)); + ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?", _)).WillByDefault(Return(false)); Test_Run_Error("Error: base directory not found"); } @@ -126,7 +126,7 @@ TEST_P(CliTest_WrongEnvironment, BaseDir_DoesntExist_Noninteractive) { _basedir.remove(); // We can't set an EXPECT_CALL().Times(0), because this is a death test (i.e. it is forked) and gmock EXPECT_CALL in fork children don't report to parents. // So we set a default answer that shouldn't crash and check it's not called by checking that it crashes. - ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?")).WillByDefault(Return(true)); + ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?", _)).WillByDefault(Return(true)); ::setenv("CRYFS_FRONTEND", "noninteractive", 1); Test_Run_Error("Error: base directory not found"); ::unsetenv("CRYFS_FRONTEND"); @@ -135,7 +135,7 @@ TEST_P(CliTest_WrongEnvironment, BaseDir_DoesntExist_Noninteractive) { TEST_P(CliTest_WrongEnvironment, BaseDir_DoesntExist_Create) { if (!GetParam().runningInForeground) {return;} // TODO Make this work also if run in background (see CliTest::EXPECT_RUN_SUCCESS) _basedir.remove(); - ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?")).WillByDefault(Return(true)); + ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?", _)).WillByDefault(Return(true)); Test_Run_Success(); EXPECT_TRUE(bf::exists(_basedir.path()) && bf::is_directory(_basedir.path())); } @@ -176,7 +176,7 @@ TEST_P(CliTest_WrongEnvironment, BaseDir_NoPermission) { TEST_P(CliTest_WrongEnvironment, MountDir_DoesntExist) { _mountdir.remove(); // ON_CALL and not EXPECT_CALL, because this is a death test (i.e. it is forked) and gmock EXPECT_CALL in fork children don't report to parents. - ON_CALL(*console, askYesNo("Could not find mount directory. Do you want to create it?")).WillByDefault(Return(false)); + ON_CALL(*console, askYesNo("Could not find mount directory. Do you want to create it?", _)).WillByDefault(Return(false)); Test_Run_Error("Error: mount directory not found"); } @@ -184,7 +184,7 @@ TEST_P(CliTest_WrongEnvironment, MountDir_DoesntExist_Noninteractive) { _mountdir.remove(); // We can't set an EXPECT_CALL().Times(0), because this is a death test (i.e. it is forked) and gmock EXPECT_CALL in fork children don't report to parents. // So we set a default answer that shouldn't crash and check it's not called by checking that it crashes. - ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?")).WillByDefault(Return(true)); + ON_CALL(*console, askYesNo("Could not find base directory. Do you want to create it?", _)).WillByDefault(Return(true)); ::setenv("CRYFS_FRONTEND", "noninteractive", 1); Test_Run_Error("Error: mount directory not found"); ::unsetenv("CRYFS_FRONTEND"); @@ -193,7 +193,7 @@ TEST_P(CliTest_WrongEnvironment, MountDir_DoesntExist_Noninteractive) { TEST_P(CliTest_WrongEnvironment, MountDir_DoesntExist_Create) { if (!GetParam().runningInForeground) {return;} // TODO Make this work also if run in background (see CliTest::EXPECT_RUN_SUCCESS) _mountdir.remove(); - ON_CALL(*console, askYesNo("Could not find mount directory. Do you want to create it?")).WillByDefault(Return(true)); + ON_CALL(*console, askYesNo("Could not find mount directory. Do you want to create it?", _)).WillByDefault(Return(true)); Test_Run_Success(); EXPECT_TRUE(bf::exists(_mountdir.path()) && bf::is_directory(_mountdir.path())); } diff --git a/test/cryfs/config/CryConfigConsoleTest.cpp b/test/cryfs/config/CryConfigConsoleTest.cpp index 7746b0c7..853cfe57 100644 --- a/test/cryfs/config/CryConfigConsoleTest.cpp +++ b/test/cryfs/config/CryConfigConsoleTest.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "../testutils/MockConsole.h" using namespace cryfs; @@ -10,6 +11,7 @@ using namespace cryfs; using boost::optional; using boost::none; using cpputils::Console; +using cpputils::NoninteractiveConsole; using cpputils::unique_ref; using cpputils::make_unique_ref; using std::string; @@ -28,8 +30,8 @@ class CryConfigConsoleTest: public ::testing::Test { public: CryConfigConsoleTest() : console(make_shared()), - cryconsole(console, false), - noninteractiveCryconsole(console, true) { + cryconsole(console), + noninteractiveCryconsole(make_shared(console)) { } shared_ptr console; CryConfigConsole cryconsole; @@ -39,11 +41,11 @@ public: class CryConfigConsoleTest_Cipher: public CryConfigConsoleTest {}; #define EXPECT_ASK_FOR_CIPHER() \ - EXPECT_CALL(*console, askYesNo("Use default settings?")).Times(1).WillOnce(Return(false)); \ + EXPECT_CALL(*console, askYesNo("Use default settings?", _)).Times(1).WillOnce(Return(false)); \ EXPECT_CALL(*console, ask(HasSubstr("block cipher"), UnorderedElementsAreArray(CryCiphers::supportedCipherNames()))).Times(1) #define EXPECT_ASK_FOR_BLOCKSIZE() \ - EXPECT_CALL(*console, askYesNo("Use default settings?")).Times(1).WillOnce(Return(false)); \ + EXPECT_CALL(*console, askYesNo("Use default settings?", _)).Times(1).WillOnce(Return(false)); \ EXPECT_CALL(*console, ask(HasSubstr("block size"), _)).Times(1) #define EXPECT_ASK_FOR_MISSINGBLOCKISINTEGRITYVIOLATION() \ @@ -56,14 +58,14 @@ TEST_F(CryConfigConsoleTest_Cipher, AsksForCipher) { } TEST_F(CryConfigConsoleTest_Cipher, ChooseDefaultCipher) { - EXPECT_CALL(*console, askYesNo("Use default settings?")).Times(1).WillOnce(Return(true)); + EXPECT_CALL(*console, askYesNo("Use default settings?", _)).Times(1).WillOnce(Return(true)); EXPECT_CALL(*console, ask(HasSubstr("block cipher"), _)).Times(0); string cipher = cryconsole.askCipher(); EXPECT_EQ(CryConfigConsole::DEFAULT_CIPHER, cipher); } TEST_F(CryConfigConsoleTest_Cipher, ChooseDefaultCipherWhenNoninteractiveEnvironment) { - EXPECT_CALL(*console, askYesNo(HasSubstr("default"))).Times(0); + EXPECT_CALL(*console, askYesNo(HasSubstr("default"), _)).Times(0); EXPECT_CALL(*console, ask(HasSubstr("block cipher"), _)).Times(0); string cipher = noninteractiveCryconsole.askCipher(); EXPECT_EQ(CryConfigConsole::DEFAULT_CIPHER, cipher); @@ -80,7 +82,7 @@ TEST_F(CryConfigConsoleTest_Cipher, AsksForMissingBlockIsIntegrityViolation) { } TEST_F(CryConfigConsoleTest_Cipher, ChooseDefaultBlocksizeWhenNoninteractiveEnvironment) { - EXPECT_CALL(*console, askYesNo(HasSubstr("default"))).Times(0); + EXPECT_CALL(*console, askYesNo(HasSubstr("default"), _)).Times(0); EXPECT_CALL(*console, ask(HasSubstr("block size"), _)).Times(0); uint32_t blocksize = noninteractiveCryconsole.askBlocksizeBytes(); EXPECT_EQ(CryConfigConsole::DEFAULT_BLOCKSIZE_BYTES, blocksize); @@ -92,11 +94,11 @@ public: optional cipherWarning = CryCiphers::find(cipherName).warning(); void EXPECT_DONT_SHOW_WARNING() { - EXPECT_CALL(*console, askYesNo(_)).Times(0); + EXPECT_CALL(*console, askYesNo(_, _)).Times(0); } void EXPECT_SHOW_WARNING(const string &warning) { - EXPECT_CALL(*console, askYesNo(HasSubstr(warning))).WillOnce(Return(true)); + EXPECT_CALL(*console, askYesNo(HasSubstr(warning), _)).WillOnce(Return(true)); } }; diff --git a/test/cryfs/config/CryConfigCreatorTest.cpp b/test/cryfs/config/CryConfigCreatorTest.cpp index 7cf444e8..f67d03ee 100644 --- a/test/cryfs/config/CryConfigCreatorTest.cpp +++ b/test/cryfs/config/CryConfigCreatorTest.cpp @@ -4,6 +4,7 @@ #include #include #include "../testutils/MockConsole.h" +#include #include using namespace cryfs; @@ -11,6 +12,7 @@ using namespace cryfs; using boost::optional; using boost::none; using cpputils::Console; +using cpputils::NoninteractiveConsole; using cpputils::unique_ref; using cpputils::make_unique_ref; using std::string; @@ -26,9 +28,9 @@ using ::testing::UnorderedElementsAreArray; using ::testing::WithParamInterface; #define EXPECT_ASK_TO_USE_DEFAULT_SETTINGS() \ - EXPECT_CALL(*console, askYesNo("Use default settings?")).Times(1) + EXPECT_CALL(*console, askYesNo("Use default settings?", true)).Times(1) #define EXPECT_DOES_NOT_ASK_TO_USE_DEFAULT_SETTINGS() \ - EXPECT_CALL(*console, askYesNo("Use default settings?")).Times(0) + EXPECT_CALL(*console, askYesNo("Use default settings?", true)).Times(0) #define EXPECT_ASK_FOR_CIPHER() \ EXPECT_CALL(*console, ask(HasSubstr("block cipher"), UnorderedElementsAreArray(CryCiphers::supportedCipherNames()))).Times(1) #define EXPECT_DOES_NOT_ASK_FOR_CIPHER() \ @@ -48,8 +50,8 @@ class CryConfigCreatorTest: public ::testing::Test { public: CryConfigCreatorTest() : console(make_shared()), - creator(console, cpputils::Random::PseudoRandom(), false), - noninteractiveCreator(console, cpputils::Random::PseudoRandom(), true) { + creator(console, cpputils::Random::PseudoRandom()), + noninteractiveCreator(make_shared(console), cpputils::Random::PseudoRandom()) { EXPECT_CALL(*console, ask(HasSubstr("block cipher"), _)).WillRepeatedly(ChooseAnyCipher()); EXPECT_CALL(*console, ask(HasSubstr("block size"), _)).WillRepeatedly(Return(0)); } diff --git a/test/cryfs/config/CryConfigLoaderTest.cpp b/test/cryfs/config/CryConfigLoaderTest.cpp index c4064236..1a83aeca 100644 --- a/test/cryfs/config/CryConfigLoaderTest.cpp +++ b/test/cryfs/config/CryConfigLoaderTest.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -13,10 +14,12 @@ using cpputils::make_unique_ref; using cpputils::TempFile; using cpputils::SCrypt; using cpputils::DataFixture; +using cpputils::NoninteractiveConsole; using boost::optional; using boost::none; using std::string; using std::ostream; +using std::make_shared; using ::testing::Return; using ::testing::_; using ::testing::HasSubstr; @@ -39,7 +42,13 @@ public: CryConfigLoader loader(const string &password, bool noninteractive, const optional &cipher = none) { auto askPassword = [password] { return password;}; - return CryConfigLoader(console, cpputils::Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, cipher, none, none, noninteractive); + if(noninteractive) { + return CryConfigLoader(make_shared(console), cpputils::Random::PseudoRandom(), SCrypt::TestSettings, askPassword, + askPassword, cipher, none, none); + } else { + return CryConfigLoader(console, cpputils::Random::PseudoRandom(), SCrypt::TestSettings, askPassword, + askPassword, cipher, none, none); + } } CryConfigFile Create(const string &password = "mypassword", const optional &cipher = none, bool noninteractive = false) { @@ -224,7 +233,7 @@ TEST_F(CryConfigLoaderTest, FilesystemID_Create) { } TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerYes) { - EXPECT_CALL(*console, askYesNo(HasSubstr("should not be opened with older versions"))).Times(1).WillOnce(Return(true)); + EXPECT_CALL(*console, askYesNo(HasSubstr("should not be opened with older versions"), false)).Times(1).WillOnce(Return(true)); string version = newerVersion(); CreateWithVersion(version); @@ -232,7 +241,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerYes) { } TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerNo) { - EXPECT_CALL(*console, askYesNo(HasSubstr("should not be opened with older versions"))).Times(1).WillOnce(Return(false)); + EXPECT_CALL(*console, askYesNo(HasSubstr("should not be opened with older versions"), false)).Times(1).WillOnce(Return(false)); string version = newerVersion(); CreateWithVersion(version); @@ -245,7 +254,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerNo) { } TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) { - EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"))).Times(1).WillOnce(Return(true)); + EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(1).WillOnce(Return(true)); string version = olderVersion(); CreateWithVersion(version); @@ -253,14 +262,14 @@ TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) { } TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenCorrectVersion) { - EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"))).Times(0); + 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?"))).Times(1).WillOnce(Return(false)); + EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it?"), false)).Times(1).WillOnce(Return(false)); string version = olderVersion(); CreateWithVersion(version); diff --git a/test/cryfs/filesystem/CryFsTest.cpp b/test/cryfs/filesystem/CryFsTest.cpp index fa12d840..d5ec51c2 100644 --- a/test/cryfs/filesystem/CryFsTest.cpp +++ b/test/cryfs/filesystem/CryFsTest.cpp @@ -11,12 +11,14 @@ #include #include #include "../testutils/TestWithFakeHomeDirectory.h" +#include //TODO (whole project) Make constructors explicit when implicit construction not needed using ::testing::Test; using ::testing::Return; using ::testing::_; +using std::make_shared; using cpputils::TempDir; using cpputils::TempFile; using cpputils::dynamic_pointer_move; @@ -27,6 +29,7 @@ using cpputils::Random; using cpputils::SCrypt; using cpputils::Data; using cpputils::system::FakeHomeDirectoryRAII; +using cpputils::NoninteractiveConsole; using blockstore::ondisk::OnDiskBlockStore; using boost::none; @@ -40,7 +43,7 @@ public: CryConfigFile loadOrCreateConfig() { auto askPassword = [] {return "mypassword";}; - return CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none, none, none, true).loadOrCreate(config.path()).value().configFile; + return CryConfigLoader(make_shared(mockConsole()), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none, none, none).loadOrCreate(config.path()).value().configFile; } unique_ref blockStore() { diff --git a/test/cryfs/filesystem/FileSystemTest.cpp b/test/cryfs/filesystem/FileSystemTest.cpp index fbc094d6..b647b77a 100644 --- a/test/cryfs/filesystem/FileSystemTest.cpp +++ b/test/cryfs/filesystem/FileSystemTest.cpp @@ -1,7 +1,7 @@ #include #include #include - +#include #include #include #include "../testutils/MockConsole.h" @@ -11,11 +11,12 @@ using cpputils::unique_ref; using cpputils::make_unique_ref; using cpputils::Random; using cpputils::SCrypt; +using cpputils::NoninteractiveConsole; using fspp::Device; using ::testing::Return; using ::testing::_; using boost::none; - +using std::make_shared; using blockstore::testfake::FakeBlockStore; using namespace cryfs; @@ -29,7 +30,7 @@ public: unique_ref createDevice() override { auto blockStore = cpputils::make_unique_ref(); auto askPassword = [] {return "mypassword";}; - auto config = CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none, none, none, true) + auto config = CryConfigLoader(make_shared(mockConsole()), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none, none, none) .loadOrCreate(configFile.path()).value(); return make_unique_ref(std::move(config.configFile), std::move(blockStore), config.myClientId); } diff --git a/test/cryfs/testutils/MockConsole.h b/test/cryfs/testutils/MockConsole.h index 563a0859..420503be 100644 --- a/test/cryfs/testutils/MockConsole.h +++ b/test/cryfs/testutils/MockConsole.h @@ -9,7 +9,7 @@ class MockConsole: public cpputils::Console { public: MOCK_METHOD1(print, void(const std::string&)); MOCK_METHOD2(ask, unsigned int(const std::string&, const std::vector&)); - MOCK_METHOD1(askYesNo, bool(const std::string&)); + MOCK_METHOD2(askYesNo, bool(const std::string&, bool)); }; ACTION_P(ChooseCipher, cipherName) { @@ -24,7 +24,7 @@ public: static std::shared_ptr mockConsole() { auto console = std::make_shared(); EXPECT_CALL(*console, ask(::testing::_, ::testing::_)).WillRepeatedly(ChooseCipher("aes-256-gcm")); - EXPECT_CALL(*console, askYesNo(::testing::_)).WillRepeatedly(::testing::Return(true)); + EXPECT_CALL(*console, askYesNo(::testing::_, ::testing::_)).WillRepeatedly(::testing::Return(true)); return console; } };