From 619ada437a274ec00453cf88d40dab186cc99894 Mon Sep 17 00:00:00 2001 From: Alexander van Gessel Date: Thu, 5 Oct 2017 15:12:34 +0200 Subject: [PATCH 1/3] Add filename listing for case conflicts --- src/addon/validation.cpp | 49 +++++++++++++++++-------- src/addon/validation.hpp | 17 ++++++++- src/campaign_server/campaign_server.cpp | 10 +++-- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/addon/validation.cpp b/src/addon/validation.cpp index 961a9f5d517f..e713c5baa943 100644 --- a/src/addon/validation.cpp +++ b/src/addon/validation.cpp @@ -111,6 +111,38 @@ bool check_names_legal_internal(const config& dir, std::string current_prefix, s return badlist ? badlist->empty() : true; } +bool check_case_insensitive_duplicates_internal(const config& dir, std::string prefix, std::vector* badlist){ + std::set filenames; + bool inserted; + for (const config &path : dir.child_range("file")) { + const config::attribute_value &filename = path["name"]; + std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); + if (!inserted){ + if(badlist){ + badlist->push_back(prefix + filename.str()); + } else { + return false; + } + } + } + for (const config &path : dir.child_range("dir")) { + const config::attribute_value &filename = path["name"]; + std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); + if (!inserted) { + if(badlist){ + badlist->push_back(prefix + filename.str()); + } else { + return false; + } + } + if (!check_case_insensitive_duplicates_internal(path, prefix + filename + "/", badlist) && !badlist){ + return false; + } + } + + return badlist ? badlist->empty() : true; +} + } // end unnamed namespace 3 bool check_names_legal(const config& dir, std::vector* badlist) @@ -122,21 +154,8 @@ bool check_names_legal(const config& dir, std::vector* badlist) return check_names_legal_internal(dir, "", badlist); } -bool check_case_insensitive_duplicates(const config& dir){ - std::set filenames; - bool inserted; - for (const config &path : dir.child_range("file")) { - const config::attribute_value &filename = path["name"]; - std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); - if (!inserted) return false; - } - for (const config &path : dir.child_range("dir")) { - const config::attribute_value &filename = path["name"]; - std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); - if (!inserted) return false; - if (!check_case_insensitive_duplicates(path)) return false; - } - return true; +bool check_case_insensitive_duplicates(const config& dir, std::vector* badlist){ + return check_case_insensitive_duplicates_internal(dir, "", badlist); } ADDON_TYPE get_addon_type(const std::string& str) diff --git a/src/addon/validation.hpp b/src/addon/validation.hpp index 4c25621ade91..009cb56e581b 100644 --- a/src/addon/validation.hpp +++ b/src/addon/validation.hpp @@ -77,8 +77,21 @@ bool addon_filename_legal(const std::string& name); * @returns True if no illegal names were found. */ bool check_names_legal(const config& dir, std::vector* badlist = nullptr); -/** Probes an add-on archive for case-conflicts on case-insensitive filesystems. */ -bool check_case_insensitive_duplicates(const config& dir); +/** + * Scans an add-on archive for case-conflicts. + * + * Case conflicts may cause trouble on case-insensitive filesystems. + * + * @param dir The WML container for the root [dir] node where the search + * should begin. + * @param badlist If provided and not null, any case conflicts encountered will + * be added to this list. This also makes the archive scan more + * thorough instead of returning on the first conflict + * encountered. + * + * @returns True if no conflicts were found. + */ +bool check_case_insensitive_duplicates(const config& dir, std::vector* badlist = nullptr); std::string encode_binary(const std::string& str); std::string unencode_binary(const std::string& str); diff --git a/src/campaign_server/campaign_server.cpp b/src/campaign_server/campaign_server.cpp index 9f124486575c..14c70cff6327 100644 --- a/src/campaign_server/campaign_server.cpp +++ b/src/campaign_server/campaign_server.cpp @@ -692,9 +692,13 @@ void server::handle_upload(const server::request& req) "Add-on rejected: The add-on contains files or directories with illegal names. " "File or directory names may not contain whitespace or any of the following characters: '/ \\ : ~'", filelist, req.sock); - } else if(!check_case_insensitive_duplicates(data)) { - LOG_CS << "Upload aborted - case conflict in add-on data.\n"; - send_error("Add-on rejected: Two files have a case conflict", req.sock); + } else if(!check_case_insensitive_duplicates(data, &badnames)) { + const std::string& filelist = utils::join(badnames, "\n"); + LOG_CS << "Upload aborted - case conflict in add-on data (" << badnames.size() << " entries).\n"; + send_error( + "Add-on rejected: The add-on contains files or directories with case conflicts. " + "File or directory names may not be differently-cased versions of the same string.", + filelist, req.sock); } else if(campaign && !authenticate(*campaign, upload["passphrase"])) { LOG_CS << "Upload aborted - incorrect passphrase.\n"; send_error("Add-on rejected: The add-on already exists, and your passphrase was incorrect.", req.sock); From 927fcb208419e7d185cb40b8ae49834f97ad0f5a Mon Sep 17 00:00:00 2001 From: Alexander van Gessel Date: Tue, 17 Oct 2017 18:23:54 +0200 Subject: [PATCH 2/3] Add pre-upload check for case conflicts --- src/addon/client.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/addon/client.cpp b/src/addon/client.cpp index 22a84710477e..5e39a4262695 100644 --- a/src/addon/client.cpp +++ b/src/addon/client.cpp @@ -152,6 +152,13 @@ bool addons_client::upload_addon(const std::string& id, std::string& response_me this->last_error_data_ = utils::join(badnames, "\n"); return false; } + if(!check_case_insensitive_duplicates(addon_data, &badnames)){ + this->last_error_ = + vgettext("The add-on $addon_title contains files or directories with case conflicts. " + "File or directory names may not be differently-cased versions of the same string.", i18n_symbols); + this->last_error_data_ = utils::join(badnames, "\n"); + return false; + } config request_buf, response_buf; request_buf.add_child("upload", cfg).add_child("data", addon_data); From 4bde2d8b654e3c47a004c11c573804a77d694acb Mon Sep 17 00:00:00 2001 From: Alexander van Gessel Date: Tue, 17 Oct 2017 18:24:19 +0200 Subject: [PATCH 3/3] List all filenames that contribute to the case conflicts --- src/addon/validation.cpp | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/addon/validation.cpp b/src/addon/validation.cpp index e713c5baa943..352f4eb9b923 100644 --- a/src/addon/validation.cpp +++ b/src/addon/validation.cpp @@ -112,14 +112,24 @@ bool check_names_legal_internal(const config& dir, std::string current_prefix, s } bool check_case_insensitive_duplicates_internal(const config& dir, std::string prefix, std::vector* badlist){ - std::set filenames; + typedef std::pair printed_and_original; + std::map filenames; bool inserted; + bool printed; + std::string original; for (const config &path : dir.child_range("file")) { const config::attribute_value &filename = path["name"]; - std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); + const std::string lowercase = boost::algorithm::to_lower_copy(filename.str(), std::locale::classic()); + const std::string with_prefix = prefix + filename.str(); + std::tie(std::ignore, inserted) = filenames.emplace(lowercase, std::make_pair(false, with_prefix)); if (!inserted){ if(badlist){ - badlist->push_back(prefix + filename.str()); + std::tie(printed, original) = filenames[lowercase]; + if(!printed){ + badlist->push_back(original); + filenames[lowercase] = make_pair(true, std::string()); + } + badlist->push_back(with_prefix); } else { return false; } @@ -127,10 +137,17 @@ bool check_case_insensitive_duplicates_internal(const config& dir, std::string p } for (const config &path : dir.child_range("dir")) { const config::attribute_value &filename = path["name"]; - std::tie(std::ignore, inserted) = filenames.insert(boost::algorithm::to_lower_copy(filename.str(), std::locale::classic())); + const std::string lowercase = boost::algorithm::to_lower_copy(filename.str(), std::locale::classic()); + const std::string with_prefix = prefix + filename.str(); + std::tie(std::ignore, inserted) = filenames.emplace(lowercase, std::make_pair(false, with_prefix)); if (!inserted) { if(badlist){ - badlist->push_back(prefix + filename.str()); + std::tie(printed, original) = filenames[lowercase]; + if(!printed){ + badlist->push_back(original); + filenames[lowercase] = make_pair(true, std::string()); + } + badlist->push_back(with_prefix); } else { return false; }