Fix bug where a comma in the base directory name would make the file system fail to mount, https://github.com/cryfs/cryfs/issues/326

.circleci/config.yml

@@ -517,7 +517,7 @@ jobs:
       OMP_NUM_THREADS: "1"
       CXXFLAGS: "-O2 -fsanitize=thread -fno-omit-frame-pointer"
       BUILD_TYPE: "Debug"
-      GTEST_ARGS: "--gtest_filter=-LoggingTest.LoggingAlsoWorksAfterFork:AssertTest_*:BacktraceTest.*:SignalCatcherTest.*_thenDies:SignalHandlerTest.*_thenDies:SignalHandlerTest.givenMultipleSigIntHandlers_whenRaising_thenCatchesCorrectSignal:CliTest_Setup.*:CliTest_IntegrityCheck.*:*/CliTest_WrongEnvironment.*:CliTest_Unmount.*"
+      GTEST_ARGS: "--gtest_filter=-LoggingTest.LoggingAlsoWorksAfterFork:AssertTest_*:BacktraceTest.*:SignalCatcherTest.*_thenDies:SignalHandlerTest.*_thenDies:SignalHandlerTest.givenMultipleSigIntHandlers_whenRaising_thenCatchesCorrectSignal:CliTest_Setup.*:CliTest_IntegrityCheck.*:*/CliTest_WrongEnvironment.*:CliTest_Unmount.*:CliTest.WorksWithCommasInBasedir"
       CMAKE_FLAGS: ""
       RUN_TESTS: true
   clang_tidy:

ChangeLog.txt

@@ -1,3 +1,8 @@
+Version 0.10.3 (unreleased)
+---------------
+Fixed bugs:
+* A comma in the base directory name would make the file system fail to mount, https://github.com/cryfs/cryfs/issues/326
+
 Version 0.10.2
 ---------------
 Fixed bugs:

src/fspp/fuse/Fuse.cpp

@@ -12,6 +12,7 @@
 #include <csignal>
 #include "InvalidFilesystem.h"
 #include <codecvt>
+#include <boost/algorithm/string/replace.hpp>
 
 #if defined(_MSC_VER)
 #include <codecvt>
@@ -290,7 +291,9 @@ vector<char *> Fuse::_build_argv(const bf::path &mountdir, const vector<string>
     argv.push_back(_create_c_string(option));
   }
   _add_fuse_option_if_not_exists(&argv, "subtype", _fstype);
-  _add_fuse_option_if_not_exists(&argv, "fsname", _fsname.get_value_or(_fstype));
+  auto fsname = _fsname.get_value_or(_fstype);
+  boost::replace_all(fsname, ",", "\\,"); // Avoid fuse options parser bug where a comma in the fsname is misinterpreted as an options delimiter, see https://github.com/cryfs/cryfs/issues/326
+  _add_fuse_option_if_not_exists(&argv, "fsname", fsname);
 #ifdef __APPLE__
   // Make volume name default to mountdir on macOS
   _add_fuse_option_if_not_exists(&argv, "volname", mountdir.filename().string());

test/cryfs-cli/CliTest_Setup.cpp

@@ -2,6 +2,8 @@
 
 using cpputils::TempFile;
 
+namespace bf = boost::filesystem;
+
 //Tests that cryfs is correctly setup according to the CLI parameters specified
 using CliTest_Setup = CliTest;
 
@@ -36,4 +38,12 @@ TEST_F(CliTest_Setup, FuseOptionGiven) {
     //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
     EXPECT_RUN_SUCCESS({basedir.string().c_str(), mountdir.string().c_str(), "-f", "--cipher", "aes-256-gcm", "--", "-f"}, mountdir);
-}
+}
+
+TEST_F(CliTest, WorksWithCommasInBasedir) {
+    // This test makes sure we don't regress on https://github.com/cryfs/cryfs/issues/326
+    //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
+    auto basedir_ = basedir / "pathname,with,commas";
+    bf::create_directory(basedir_);
+    EXPECT_RUN_SUCCESS({basedir_.string().c_str(), mountdir.string().c_str(), "-f"}, mountdir);
+}

test/cryfs-cli/testutils/CliTest.h

@@ -75,12 +75,21 @@ public:
         //TODO Make this work when run in background
         ASSERT(std::find(args.begin(), args.end(), string("-f")) != args.end(), "Currently only works if run in foreground");
 
-        FilesystemOutput filesystem_output = run_filesystem(args, mountDir, std::move(onMounted));
+        bool successfully_mounted = false;
+
+        FilesystemOutput filesystem_output = run_filesystem(args, mountDir, [&] {
+            successfully_mounted = true;
+            onMounted();
+        });
 
         EXPECT_EQ(0, filesystem_output.exit_code);
         if (!std::regex_search(filesystem_output.stdout_, std::regex("Mounting filesystem"))) {
-          std::cerr << filesystem_output.stdout_ << std::endl;
+          std::cerr << "STDOUT:\n" << filesystem_output.stdout_ << "STDERR:\n" << filesystem_output.stderr_ << std::endl;
-          EXPECT_TRUE(false);
+          EXPECT_TRUE(false) << "Filesystem did not output the 'Mounting filesystem' message, probably wasn't successfully mounted.";
+        }
+
+        if (!successfully_mounted) {
+            EXPECT_TRUE(false) << "Filesystem did not call onMounted callback, probably wasn't successfully mounted.";
         }
     }