convert-to: be more tolerant of unusual filenames.

But re-write them anyway to be more sensible.

Change-Id: Ie146f4f84b539ab7e826a1c1b947497acde7e384
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>

common/Util.cpp

@@ -609,6 +609,34 @@ namespace Util
         return result;
     }
 
+    std::string replaceAllOf(const std::string &str, const std::string& match, const std::string& repl)
+    {
+        std::ostringstream os;
+
+        assert(match.size() == repl.size());
+        if (match.size() != repl.size())
+            return std::string("replaceAllOf failed");
+
+        const std::size_t strSize = str.size();
+        for (size_t i = 0; i < strSize; ++i)
+        {
+            auto pos = match.find(str[i]);
+            if (pos != std::string::npos)
+                os << repl[pos];
+            else
+                os << str[i];
+        }
+
+        return os.str();
+    }
+
+    std::string cleanupFilename(const std::string &filename)
+    {
+        static const std::string mtch(",/?:@&=+$#'\"");
+        static const std::string repl("------------");
+        return replaceAllOf(filename, mtch, repl);
+    }
+
     std::string formatLinesForLog(const std::string& s)
     {
         std::string r;

common/Util.hpp

@@ -282,6 +282,9 @@ namespace Util
     /// Replace substring @a in string @s with string @b.
     std::string replace(std::string s, const std::string& a, const std::string& b);
 
+    /// Replace any characters in @a matching characters in @b with replacement chars in @c and return
+    std::string replaceAllOf(const std::string &str, const std::string& match, const std::string& repl);
+
     std::string formatLinesForLog(const std::string& s);
 
     void setThreadName(const std::string& s);
@@ -1096,6 +1099,10 @@ int main(int argc, char**argv)
     /// Decode a URI encoded with encodeURIComponent.
     std::string decodeURIComponent(const std::string& uri);
 
+    /// Cleanup a filename replacing anything potentially problematic
+    /// either for a URL or for a file path
+    std::string cleanupFilename(const std::string &filename);
+
     /// Anonymize a sensitive string to avoid leaking it.
     /// Called on strings to be logged or exposed.
     std::string anonymize(const std::string& text, const std::uint64_t nAnonymizationSalt);

test/WhiteBoxTests.cpp

@@ -46,6 +46,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testPathPrefixTrimming);
     CPPUNIT_TEST(testMessageAbbreviation);
     CPPUNIT_TEST(testReplace);
+    CPPUNIT_TEST(testReplaceAllOf);
     CPPUNIT_TEST(testRegexListMatcher);
     CPPUNIT_TEST(testRegexListMatcher_Init);
     CPPUNIT_TEST(testEmptyCellCursor);
@@ -77,6 +78,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     void testPathPrefixTrimming();
     void testMessageAbbreviation();
     void testReplace();
+    void testReplaceAllOf();
     void testRegexListMatcher();
     void testRegexListMatcher_Init();
     void testEmptyCellCursor();
@@ -463,6 +465,14 @@ void WhiteBoxTests::testReplace()
     LOK_ASSERT_EQUAL(std::string("test one two flee"), Util::replace("test one two flee", "", "X"));
 }
 
+void WhiteBoxTests::testReplaceAllOf()
+{
+    constexpr auto testname = __func__;
+
+    LOK_ASSERT_EQUAL(std::string("humvee"), Util::replaceAllOf("humans","san", "eve"));
+    LOK_ASSERT_EQUAL(std::string("simple.odt"), Util::replaceAllOf("s#&-le.odt", "#&-", "imp"));
+}
+
 void WhiteBoxTests::testRegexListMatcher()
 {
     constexpr auto testname = __func__;

wsd/COOLWSD.cpp

@@ -787,9 +787,14 @@ public:
             + '/');
         LOG_TRC("Created temporary convert-to/insert path: " << tempPath.toString());
 
-        // Prevent user inputting anything funny here.
+        // Prevent user inputing anything funny here.
+        std::string fileParam = params.get("filename");
+        std::string cleanFilename = Util::cleanupFilename(fileParam);
+        if (fileParam != cleanFilename)
+            LOG_DBG("Unexpected characters in conversion filename '" << fileParam << "' cleaned to '" << cleanFilename << "'");
+
         // A "filename" should always be a filename, not a path
-        const Path filenameParam(params.get("filename"));
+        const Path filenameParam(cleanFilename);
         if (filenameParam.getFileName() == "callback:")
             tempPath.setFileName("incoming_file"); // A sensible name.
         else