Fix handling of relative paths and add test cases

ChangeLog.txt

@@ -1,6 +1,7 @@
 Version 0.8.2
 ---------------
 * Mount directory and base directory can be specified as relative paths.
+* Improved error messages.
 
 Version 0.8.1
 ---------------

src/cli/Cli.cpp

@@ -63,6 +63,7 @@ using boost::chrono::milliseconds;
 //TODO Improve parallelity.
 //TODO Did deadlock in bonnie++ second run (in the create files sequentially) - maybe also in a later run or different step?
 //TODO Replace ASSERTs with other error handling when it is not a programming error but an environment influence (e.g. a block is missing)
+//TODO When calling cryfs without basedir parameter (i.e. just calling "cryfs"): the option '--base-dir' is required but missing. Should be a better error message, since --base-dir is not actually a command line option.
 
 namespace cryfs {
 

src/cli/program_options/Parser.cpp

@@ -30,11 +30,19 @@ ProgramOptions Parser::parse(const vector<string> &supportedCiphers) const {
     pair<vector<char*>, vector<char*>> options = splitAtDoubleDash(_options);
     po::variables_map vm = _parseOptionsOrShowHelp(options.first, supportedCiphers);
 
-    bf::path baseDir = bf::canonical(vm["base-dir"].as<string>());
-    bf::path mountDir = bf::canonical(vm["mount-dir"].as<string>());
+    if (!vm.count("base-dir")) {
+        std::cerr << "Please specify a base directory.\n";
+        _showHelpAndExit();
+    }
+    if (!vm.count("mount-dir")) {
+        std::cerr << "Please specify a mount directory.\n";
+        _showHelpAndExit();
+    }
+    bf::path baseDir = bf::absolute(vm["base-dir"].as<string>());
+    bf::path mountDir = bf::absolute(vm["mount-dir"].as<string>());
     optional<bf::path> configfile = none;
     if (vm.count("config")) {
-        configfile = bf::canonical(vm["config"].as<string>());
+        configfile = bf::absolute(vm["config"].as<string>());
     }
     bool foreground = vm.count("foreground");
     if (foreground) {
@@ -46,7 +54,7 @@ ProgramOptions Parser::parse(const vector<string> &supportedCiphers) const {
     }
     optional<bf::path> logfile = none;
     if (vm.count("logfile")) {
-        logfile = bf::canonical(vm["logfile"].as<string>());
+        logfile = bf::absolute(vm["logfile"].as<string>());
     }
     optional<string> cipher = none;
     if (vm.count("cipher")) {
@@ -117,8 +125,8 @@ void Parser::_addPositionalOptionForBaseDir(po::options_description *desc, po::p
     positional->add("mount-dir", 1);
     po::options_description hidden("Hidden options");
     hidden.add_options()
-            ("base-dir", po::value<string>()->required(), "Base directory")
-            ("mount-dir", po::value<string>()->required(), "Mount directory")
+            ("base-dir", po::value<string>(), "Base directory")
+            ("mount-dir", po::value<string>(), "Mount directory")
             ;
     desc->add(hidden);
 }

test/cli/CliTest_ShowingHelp.cpp

@@ -19,9 +19,9 @@ TEST_F(CliTest_ShowingHelp, HelpShortOptionTogetherWithOtherOptions) {
 }
 
 TEST_F(CliTest_ShowingHelp, MissingAllOptions) {
-    EXPECT_EXIT_WITH_HELP_MESSAGE({});
+    EXPECT_EXIT_WITH_HELP_MESSAGE({}, "Please specify a base directory");
 }
 
 TEST_F(CliTest_ShowingHelp, MissingDir) {
-    EXPECT_EXIT_WITH_HELP_MESSAGE({basedir.c_str()});
+    EXPECT_EXIT_WITH_HELP_MESSAGE({basedir.c_str()}, "Please specify a mount directory");
 }

test/cli/program_options/ParserTest.cpp

@@ -6,6 +6,7 @@ using namespace cryfs;
 using namespace cryfs::program_options;
 using std::vector;
 using boost::none;
+namespace bf = boost::filesystem;
 
 class ProgramOptionsParserTest: public ProgramOptionsTestBase {
 public:
@@ -61,16 +62,36 @@ TEST_F(ProgramOptionsParserTest, NoSpecialOptions) {
     EXPECT_VECTOR_EQ({"./myExecutable", "/home/user/mountDir"}, options.fuseOptions());
 }
 
+TEST_F(ProgramOptionsParserTest, RelativeBaseDir) {
+    ProgramOptions options = parse({"./myExecutable", "baseDir", "/home/user/mountDir"});
+    EXPECT_EQ(bf::current_path() / "baseDir", options.baseDir());
+}
+
+TEST_F(ProgramOptionsParserTest, RelativeMountDir) {
+    ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "mountDir"});
+    EXPECT_EQ(bf::current_path() / "mountDir", options.mountDir());
+}
+
 TEST_F(ProgramOptionsParserTest, LogfileGiven) {
     ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "--logfile", "/home/user/mylogfile", "/home/user/mountDir"});
     EXPECT_EQ("/home/user/mylogfile", options.logFile().value());
 }
 
+TEST_F(ProgramOptionsParserTest, LogfileGiven_RelativePath) {
+    ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "--logfile", "mylogfile", "/home/user/mountDir"});
+    EXPECT_EQ(bf::current_path() / "mylogfile", options.logFile().value());
+}
+
 TEST_F(ProgramOptionsParserTest, ConfigfileGiven) {
     ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "--config", "/home/user/myconfigfile", "/home/user/mountDir"});
     EXPECT_EQ("/home/user/myconfigfile", options.configFile().value());
 }
 
+TEST_F(ProgramOptionsParserTest, ConfigfileGiven_RelativePath) {
+    ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "--config", "myconfigfile", "/home/user/mountDir"});
+    EXPECT_EQ(bf::current_path() / "myconfigfile", options.configFile().value());
+}
+
 TEST_F(ProgramOptionsParserTest, CipherGiven) {
     ProgramOptions options = parse({"./myExecutable", "/home/user/baseDir", "--cipher", "aes-256-gcm", "/home/user/mountDir"});
     EXPECT_EQ("aes-256-gcm", options.cipher().value());

test/cli/program_options/ProgramOptionsTest.cpp

@@ -5,7 +5,20 @@
 using namespace cryfs::program_options;
 using std::vector;
 using boost::none;
+using boost::optional;
+using std::ostream;
 using std::string;
+namespace bf = boost::filesystem;
+
+// This is needed for google test to work with boost::optional<boost::filesystem::path>
+namespace boost {
+    template<> ostream& operator<< <char, std::char_traits<char>, bf::path>(ostream &stream, const optional<bf::path> &path) {
+        if (path == none) {
+            return stream << "none";
+        }
+        return stream << *path;
+    }
+}
 
 class ProgramOptionsTest: public ProgramOptionsTestBase {};
 
@@ -25,7 +38,7 @@ TEST_F(ProgramOptionsTest, ConfigfileNone) {
 }
 
 TEST_F(ProgramOptionsTest, ConfigfileSome) {
-    ProgramOptions testobj("", "", string("/home/user/configfile"), true, none, none, none, none, options({"./myExecutable"}));
+    ProgramOptions testobj("", "", bf::path("/home/user/configfile"), true, none, none, none, none, options({"./myExecutable"}));
     EXPECT_EQ("/home/user/configfile", testobj.configFile().get());
 }
 
@@ -45,7 +58,7 @@ TEST_F(ProgramOptionsTest, LogfileNone) {
 }
 
 TEST_F(ProgramOptionsTest, LogfileSome) {
-    ProgramOptions testobj("", "", none, true, none, string("logfile"), none, none, options({"./myExecutable"}));
+    ProgramOptions testobj("", "", none, true, none, bf::path("logfile"), none, none, options({"./myExecutable"}));
     EXPECT_EQ("logfile", testobj.logFile().get());
 }
 

test/cli/testutils/CliTest.h

@@ -32,8 +32,8 @@ public:
         cryfs::Cli(keyGenerator, cpputils::SCrypt::TestSettings).main(_args.size(), _args.data());
     }
 
-    void EXPECT_EXIT_WITH_HELP_MESSAGE(std::vector<const char*> args) {
-        EXPECT_RUN_ERROR(args, "Usage");
+    void EXPECT_EXIT_WITH_HELP_MESSAGE(std::vector<const char*> args, const std::string &message = "") {
+        EXPECT_RUN_ERROR(args, (message+".*Usage").c_str());
     }
 
     void EXPECT_RUN_ERROR(std::vector<const char*> args, const char *message) {