From 151c38b05ce581c6ebb629070d1d864f6965006f Mon Sep 17 00:00:00 2001 From: huangkangping Date: Thu, 22 Nov 2018 14:58:21 +0800 Subject: [PATCH 1/3] Fix bug of #307 There is a bug to use symbolic link directory as storage root path. It is a problem that whether the path is canonical --- be/src/http/download_action.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/be/src/http/download_action.cpp b/be/src/http/download_action.cpp index 039c97370eaf6a..d82137ccfcd12c 100644 --- a/be/src/http/download_action.cpp +++ b/be/src/http/download_action.cpp @@ -26,6 +26,7 @@ #include #include "boost/lexical_cast.hpp" +#include #include "agent/cgroups_mgr.h" #include "http/http_channel.h" @@ -38,6 +39,8 @@ #include "util/filesystem_util.h" #include "runtime/exec_env.h" +using boost::filesystem::canonical; + namespace doris { const std::string FILE_PARAMETER = "file"; @@ -243,8 +246,9 @@ Status DownloadAction::check_token(HttpRequest *req) { Status DownloadAction::check_path_is_allowed(const std::string& file_path) { DCHECK_EQ(_download_type, NORMAL); + std::string canonical_file_path = canonical(file_path).string(); for (auto& allow_path : _allow_paths) { - if (FileSystemUtil::contain_path(allow_path, file_path)) { + if (FileSystemUtil::contain_path(canonical(allow_path).string(), canonical_file_path)) { return Status::OK; } } @@ -254,7 +258,8 @@ Status DownloadAction::check_path_is_allowed(const std::string& file_path) { Status DownloadAction::check_log_path_is_allowed(const std::string& file_path) { DCHECK_EQ(_download_type, ERROR_LOG); - if (FileSystemUtil::contain_path(_error_log_root_dir, file_path)) { + std::string canonical_file_path = canonical(file_path).string(); + if (FileSystemUtil::contain_path(canonical(_error_log_root_dir).string(), canonical_file_path)) { return Status::OK; } From 7840f1dd395c71a2bae45d7518e2668cef1ac3d3 Mon Sep 17 00:00:00 2001 From: huangkangping Date: Thu, 22 Nov 2018 16:24:39 +0800 Subject: [PATCH 2/3] Fix bug of using symbolic link dir as storage path In DownloadAction, checking fails by comparing canonical path with noncanonical path. So fix the bug by convert all path to canonical path before comparison --- be/src/http/download_action.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/be/src/http/download_action.cpp b/be/src/http/download_action.cpp index d82137ccfcd12c..378cd07f55c0d4 100644 --- a/be/src/http/download_action.cpp +++ b/be/src/http/download_action.cpp @@ -50,16 +50,16 @@ const std::string TOKEN_PARAMETER = "token"; DownloadAction::DownloadAction(ExecEnv* exec_env, const std::vector& allow_dirs) : _exec_env(exec_env), - _download_type(NORMAL), - _allow_paths(allow_dirs) { - + _download_type(NORMAL) { + for (auto& dir : allow_dirs) { + _allow_paths.emplace_back(std::move(canonical(dir).string())); + } } DownloadAction::DownloadAction(ExecEnv* exec_env, const std::string& error_log_root_dir) : _exec_env(exec_env), - _download_type(ERROR_LOG), - _error_log_root_dir(error_log_root_dir) { - + _download_type(ERROR_LOG) { + _error_log_root_dir = canonical(error_log_root_dir).string(); } void DownloadAction::handle_normal( @@ -248,7 +248,7 @@ Status DownloadAction::check_path_is_allowed(const std::string& file_path) { DCHECK_EQ(_download_type, NORMAL); std::string canonical_file_path = canonical(file_path).string(); for (auto& allow_path : _allow_paths) { - if (FileSystemUtil::contain_path(canonical(allow_path).string(), canonical_file_path)) { + if (FileSystemUtil::contain_path(allow_path, canonical_file_path)) { return Status::OK; } } @@ -259,7 +259,7 @@ Status DownloadAction::check_path_is_allowed(const std::string& file_path) { Status DownloadAction::check_log_path_is_allowed(const std::string& file_path) { DCHECK_EQ(_download_type, ERROR_LOG); std::string canonical_file_path = canonical(file_path).string(); - if (FileSystemUtil::contain_path(canonical(_error_log_root_dir).string(), canonical_file_path)) { + if (FileSystemUtil::contain_path(_error_log_root_dir, canonical_file_path)) { return Status::OK; } From 546513a7c74734ee68d2fc69567ae1c3719921cd Mon Sep 17 00:00:00 2001 From: huangkangping Date: Thu, 22 Nov 2018 17:02:14 +0800 Subject: [PATCH 3/3] Delete unnecessary std::move --- be/src/http/download_action.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/http/download_action.cpp b/be/src/http/download_action.cpp index 378cd07f55c0d4..f7aa130713e24a 100644 --- a/be/src/http/download_action.cpp +++ b/be/src/http/download_action.cpp @@ -52,7 +52,7 @@ DownloadAction::DownloadAction(ExecEnv* exec_env, const std::vector _exec_env(exec_env), _download_type(NORMAL) { for (auto& dir : allow_dirs) { - _allow_paths.emplace_back(std::move(canonical(dir).string())); + _allow_paths.emplace_back(canonical(dir).string()); } }