Sanitise asset names after potentially failed %-based substitutions (#945).

If there's a %x in the format string, where %x is not recognised, we
should at least remove the %.

diff --git a/src/lib/util.cc b/src/lib/util.cc
index 6339cb6a64b24afaa6b79e029e51a5436fe750bb..1ce288686faec17ca20cf47a55a0b6fd2396bfc6 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -733,9 +733,9 @@ asset_filename (shared_ptr<dcp::Asset> asset, string type, int reel_index, int r
        values['r'] = raw_convert<string>(reel_index + 1);
        values['n'] = raw_convert<string>(reel_count);
        if (summary) {
-               values['c'] = careful_string_filter(summary.get());
+               values['c'] = summary.get();
        }
-       return Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + extension);
+       return careful_string_filter(Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + extension));
 }
 
 

diff --git a/test/file_naming_test.cc b/test/file_naming_test.cc
index 1961bfcfe7676ccee975af87b359c736a6d8e079..1b5871bd2c80d008dbc738a9fa1a885066fdefc4 100644 (file)
--- a/test/file_naming_test.cc
+++ b/test/file_naming_test.cc
@@ -197,3 +197,23 @@ BOOST_AUTO_TEST_CASE (subtitle_file_naming)
        BOOST_CHECK_EQUAL(got, 1);
 }
 
+
+BOOST_AUTO_TEST_CASE(remove_bad_characters_from_template)
+{
+       ConfigRestorer cr;
+
+       /* %z is not recognised, so the % should be discarded so it won't trip
+        * an invalid URI check in make_and_verify_dcp
+        */
+       Config::instance()->set_dcp_asset_filename_format(dcp::NameFormat("%c%z"));
+
+       auto content = content_factory("test/data/flat_red.png");
+       auto film = new_test_film2("remove_bad_characters_from_template", content);
+       make_and_verify_dcp(
+               film,
+               {
+                       dcp::VerificationNote::Code::MISSING_FFMC_IN_FEATURE,
+                       dcp::VerificationNote::Code::MISSING_FFEC_IN_FEATURE
+               });
+}
+