Be more careful about allowing possibly-trouble-causing characters in DCP filenames.

diff --git a/src/lib/film.cc b/src/lib/film.cc
index 2dcdf4eefbae6432f9553241da0a44a276d3fe5b..dd31388b6fd7adf58210cd49fa72e3432822830e 100644 (file)
--- a/src/lib/film.cc
+++ b/src/lib/film.cc
@@ -791,24 +791,10 @@ Film::dcp_name (bool if_created_now) const
 {
        string unfiltered;
        if (use_isdcf_name()) {
-               unfiltered = isdcf_name (if_created_now);
-       } else {
-               unfiltered = name ();
-       }
-
-       /* Filter out `bad' characters which cause problems with some systems.
-          There's no apparent list of what really is allowed, so this is a guess.
-       */
-
-       string filtered;
-       string const allowed = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-_";
-       for (size_t i = 0; i < unfiltered.size(); ++i) {
-               if (allowed.find (unfiltered[i]) != string::npos) {
-                       filtered += unfiltered[i];
-               }
+               return careful_string_filter (isdcf_name (if_created_now));
        }
 
-       return filtered;
+       return careful_string_filter (name ());
 }
 
 void

diff --git a/src/lib/util.cc b/src/lib/util.cc
index 3f0c34a15004a9da88806366bee51967278700d5..38770f4e3214f6d30d10c5f94e4567d38e6b6fd3 100644 (file)
--- a/src/lib/util.cc
+++ b/src/lib/util.cc
@@ -658,7 +658,7 @@ video_asset_filename (shared_ptr<dcp::PictureAsset> asset, int reel_index, int r
        values['r'] = raw_convert<string> (reel_index + 1);
        values['n'] = raw_convert<string> (reel_count);
        if (summary) {
-               values['c'] = summary.get();
+               values['c'] = careful_string_filter (summary.get());
        }
        return Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + ".mxf");
 }
@@ -671,7 +671,7 @@ audio_asset_filename (shared_ptr<dcp::SoundAsset> asset, int reel_index, int ree
        values['r'] = raw_convert<string> (reel_index + 1);
        values['n'] = raw_convert<string> (reel_count);
        if (summary) {
-               values['c'] = summary.get();
+               values['c'] = careful_string_filter (summary.get());
        }
        return Config::instance()->dcp_asset_filename_format().get(values, "_" + asset->id() + ".mxf");
 }
@@ -687,3 +687,22 @@ relaxed_string_to_float (string s)
                return lexical_cast<float> (s);
        }
 }
+
+string
+careful_string_filter (string s)
+{
+       /* Filter out `bad' characters which `may' cause problems with some systems (either for DCP name or filename).
+          There's no apparent list of what really is allowed, so this is a guess.
+          Safety first and all that.
+       */
+
+       string out;
+       string const allowed = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-_%.";
+       for (size_t i = 0; i < s.size(); ++i) {
+               if (allowed.find (s[i]) != string::npos) {
+                       out += s[i];
+               }
+       }
+
+       return out;
+}

diff --git a/src/lib/util.h b/src/lib/util.h
index a93ca53ac50ce66460b9770bfb19524c01c5e586..4e6e50dd7b32b8a513802a3ceb486bde84b55b82 100644 (file)
--- a/src/lib/util.h
+++ b/src/lib/util.h
@@ -82,5 +82,6 @@ extern std::map<std::string, std::string> split_get_request (std::string url);
 extern std::string video_asset_filename (boost::shared_ptr<dcp::PictureAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
 extern std::string audio_asset_filename (boost::shared_ptr<dcp::SoundAsset> asset, int reel_index, int reel_count, boost::optional<std::string> content_summary);
 extern float relaxed_string_to_float (std::string);
+extern std::string careful_string_filter (std::string);
 
 #endif

diff --git a/src/wx/name_format_editor.cc b/src/wx/name_format_editor.cc
index 4a93edb1841c3b690653efc21940b6ec41d82757..1f7ca109bbb802f3eaaf6f5f23b7f803a8d68239 100644 (file)
--- a/src/wx/name_format_editor.cc
+++ b/src/wx/name_format_editor.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2016 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2016-2017 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -20,6 +20,7 @@
 
 #include "name_format_editor.h"
 #include "wx_util.h"
+#include "lib/util.h"
 
 using std::string;
 
@@ -62,7 +63,7 @@ NameFormatEditor::changed ()
 void
 NameFormatEditor::update_example ()
 {
-       _name.set_specification (wx_to_std (_specification->GetValue ()));
+       _name.set_specification (careful_string_filter (wx_to_std (_specification->GetValue ())));
 
        wxString example = wxString::Format (_("e.g. %s"), std_to_wx (_name.get (_examples, _suffix)));
        wxString wrapped;

diff --git a/test/data b/test/data
index 2e0db02e882f18d9a0923c19350eb0bac2abfefa..15c96cb4240f710cebc9e34f7561a0d54f53948d 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit 2e0db02e882f18d9a0923c19350eb0bac2abfefa
+Subproject commit 15c96cb4240f710cebc9e34f7561a0d54f53948d

diff --git a/test/file_naming_test.cc b/test/file_naming_test.cc
index 054685a7e32c42ca64ec4f4477729cbc01c3c6c4..3ee967a9cef1438c76e539c35af724eb284d4c83 100644 (file)
--- a/test/file_naming_test.cc
+++ b/test/file_naming_test.cc
@@ -24,13 +24,20 @@
 #include "lib/ffmpeg_content.h"
 #include "lib/dcp_content_type.h"
 #include <boost/test/unit_test.hpp>
+#include <boost/regex.hpp>
 
+using std::string;
 using boost::shared_ptr;
 
+static shared_ptr<Film>
+make_test_film (string name)
+{
+}
+
 BOOST_AUTO_TEST_CASE (file_naming_test)
 {
-       dcp::NameFormat nf ("%c");
-       Config::instance()->set_dcp_metadata_filename_format (dcp::NameFormat ("%c"));
+       Config::instance()->set_dcp_asset_filename_format (dcp::NameFormat ("%c"));
+
        shared_ptr<Film> film = new_test_film ("file_naming_test");
        film->set_name ("file_naming_test");
        film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR"));
@@ -46,7 +53,59 @@ BOOST_AUTO_TEST_CASE (file_naming_test)
        film->make_dcp ();
        wait_for_jobs ();
 
-       BOOST_CHECK (boost::filesystem::exists (film->file (film->dcp_name() + "/flat_red.png.mxf")));
-       BOOST_CHECK (boost::filesystem::exists (film->file (film->dcp_name() + "/flat_green.png.mxf")));
-       BOOST_CHECK (boost::filesystem::exists (film->file (film->dcp_name() + "/flat_blue.png.mxf")));
+       int got[3] = { 0, 0, 0 };
+       for (
+               boost::filesystem::directory_iterator i = boost::filesystem::directory_iterator (film->file(film->dcp_name()));
+               i != boost::filesystem::directory_iterator();
+               ++i) {
+               if (boost::regex_match(i->path().string(), boost::regex(".*flat_red\\.png_.*\\.mxf"))) {
+                       ++got[0];
+               } else if (boost::regex_match(i->path().string(), boost::regex(".*flat_green\\.png_.*\\.mxf"))) {
+                       ++got[1];
+               } else if (boost::regex_match(i->path().string(), boost::regex(".*flat_blue\\.png_.*\\.mxf"))) {
+                       ++got[2];
+               }
+       }
+
+       for (int i = 0; i < 3; ++i) {
+               BOOST_CHECK (got[i] == 2);
+       }
+}
+
+BOOST_AUTO_TEST_CASE (file_naming_test2)
+{
+       Config::instance()->set_dcp_asset_filename_format (dcp::NameFormat ("%c"));
+
+       shared_ptr<Film> film = new_test_film ("file_naming_test2");
+       film->set_name ("file_naming_test2");
+       film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR"));
+       shared_ptr<FFmpegContent> r (new FFmpegContent (film, "test/data/flät_red.png"));
+       film->examine_and_add_content (r);
+       shared_ptr<FFmpegContent> g (new FFmpegContent (film, "test/data/flat_green.png"));
+       film->examine_and_add_content (g);
+       shared_ptr<FFmpegContent> b (new FFmpegContent (film, "test/data/flat_blue.png"));
+       film->examine_and_add_content (b);
+       wait_for_jobs ();
+
+       film->set_reel_type (REELTYPE_BY_VIDEO_CONTENT);
+       film->make_dcp ();
+       wait_for_jobs ();
+
+       int got[3] = { 0, 0, 0 };
+       for (
+               boost::filesystem::directory_iterator i = boost::filesystem::directory_iterator (film->file(film->dcp_name()));
+               i != boost::filesystem::directory_iterator();
+               ++i) {
+               if (boost::regex_match(i->path().string(), boost::regex(".*flt_red\\.png_.*\\.mxf"))) {
+                       ++got[0];
+               } else if (boost::regex_match(i->path().string(), boost::regex(".*flat_green\\.png_.*\\.mxf"))) {
+                       ++got[1];
+               } else if (boost::regex_match(i->path().string(), boost::regex(".*flat_blue\\.png_.*\\.mxf"))) {
+                       ++got[2];
+               }
+       }
+
+       for (int i = 0; i < 3; ++i) {
+               BOOST_CHECK (got[i] == 2);
+       }
 }