From 1460bda6f80b6529e31a1a63029dc0ec5f7d0ae8 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 20 Feb 2017 00:58:31 +0000 Subject: [PATCH] Be more careful about allowing possibly-trouble-causing characters in DCP filenames. --- src/lib/film.cc | 18 ++-------- src/lib/util.cc | 23 ++++++++++-- src/lib/util.h | 1 + src/wx/name_format_editor.cc | 5 +-- test/data | 2 +- test/file_naming_test.cc | 69 +++++++++++++++++++++++++++++++++--- 6 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/lib/film.cc b/src/lib/film.cc index 2dcdf4eef..dd31388b6 100644 --- 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 3f0c34a15..38770f4e3 100644 --- a/src/lib/util.cc +++ b/src/lib/util.cc @@ -658,7 +658,7 @@ video_asset_filename (shared_ptr asset, int reel_index, int r values['r'] = raw_convert (reel_index + 1); values['n'] = raw_convert (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 asset, int reel_index, int ree values['r'] = raw_convert (reel_index + 1); values['n'] = raw_convert (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 (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 a93ca53ac..4e6e50dd7 100644 --- a/src/lib/util.h +++ b/src/lib/util.h @@ -82,5 +82,6 @@ extern std::map split_get_request (std::string url); extern std::string video_asset_filename (boost::shared_ptr asset, int reel_index, int reel_count, boost::optional content_summary); extern std::string audio_asset_filename (boost::shared_ptr asset, int reel_index, int reel_count, boost::optional 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 4a93edb18..1f7ca109b 100644 --- a/src/wx/name_format_editor.cc +++ b/src/wx/name_format_editor.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2016 Carl Hetherington + Copyright (C) 2016-2017 Carl Hetherington 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 2e0db02e8..15c96cb42 160000 --- 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 054685a7e..3ee967a9c 100644 --- 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 +#include +using std::string; using boost::shared_ptr; +static shared_ptr +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 = 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 = new_test_film ("file_naming_test2"); + film->set_name ("file_naming_test2"); + film->set_dcp_content_type (DCPContentType::from_isdcf_name ("FTR")); + shared_ptr r (new FFmpegContent (film, "test/data/flät_red.png")); + film->examine_and_add_content (r); + shared_ptr g (new FFmpegContent (film, "test/data/flat_green.png")); + film->examine_and_add_content (g); + shared_ptr 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); + } } -- 2.30.2