This is an automated email from the ASF dual-hosted git repository.
yangzhg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 52b1bd2c81 [clone](download) fix be clone action download tablet
content length overflow (#18851)
52b1bd2c81 is described below
commit 52b1bd2c8142a0e0d5542954aec374879ec7e641
Author: Zhengguo Yang <[email protected]>
AuthorDate: Fri Apr 28 11:35:17 2023 +0800
[clone](download) fix be clone action download tablet content length
overflow (#18851)
---
be/src/common/status.h | 3 ++
be/src/http/action/download_action.cpp | 53 ++++++++++++++++++++++++++--------
be/src/http/http_client.h | 6 ++++
be/src/olap/task/engine_clone_task.cpp | 12 ++++++--
4 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/be/src/common/status.h b/be/src/common/status.h
index f33993af64..2a54dcd609 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -55,6 +55,7 @@ TStatusError(UNINITIALIZED);
TStatusError(ABORTED);
TStatusError(DATA_QUALITY_ERROR);
TStatusError(LABEL_ALREADY_EXISTS);
+TStatusError(NOT_AUTHORIZED);
#undef TStatusError
// BE internal errors
E(OS_ERROR, -100);
@@ -403,6 +404,7 @@ public:
ERROR_CTOR(Uninitialized, UNINITIALIZED)
ERROR_CTOR(Aborted, ABORTED)
ERROR_CTOR(DataQualityError, DATA_QUALITY_ERROR)
+ ERROR_CTOR(NotAuthorized, NOT_AUTHORIZED)
#undef ERROR_CTOR
template <int code>
@@ -421,6 +423,7 @@ public:
bool is_invalid_argument() const { return ErrorCode::INVALID_ARGUMENT ==
_code; }
bool is_not_found() const { return _code == ErrorCode::NOT_FOUND; }
+ bool is_not_authorized() const { return code() ==
TStatusCode::NOT_AUTHORIZED; }
// Convert into TStatus. Call this if 'status_container' contains an
optional
// TStatus field named 'status'. This also sets __isset.status.
diff --git a/be/src/http/action/download_action.cpp
b/be/src/http/action/download_action.cpp
index 074baf5ec1..d258842f65 100644
--- a/be/src/http/action/download_action.cpp
+++ b/be/src/http/action/download_action.cpp
@@ -61,15 +61,30 @@ void DownloadAction::handle_normal(HttpRequest* req, const
std::string& file_par
if (config::enable_token_check) {
status = check_token(req);
if (!status.ok()) {
- HttpChannel::send_reply(req, status.to_string());
- return;
+ std::string error_msg = status.to_string();
+ if (status.is_not_authorized()) {
+ HttpChannel::send_reply(req, HttpStatus::UNAUTHORIZED,
error_msg);
+ return;
+ } else {
+ HttpChannel::send_reply(req,
HttpStatus::INTERNAL_SERVER_ERROR, error_msg);
+ return;
+ }
}
}
status = check_path_is_allowed(file_param);
if (!status.ok()) {
- HttpChannel::send_reply(req, status.to_string());
- return;
+ std::string error_msg = status.to_string();
+ if (status.is_not_found() || status.is_io_error()) {
+ HttpChannel::send_reply(req, HttpStatus::NOT_FOUND, error_msg);
+ return;
+ } else if (status.is_not_authorized()) {
+ HttpChannel::send_reply(req, HttpStatus::UNAUTHORIZED, error_msg);
+ return;
+ } else {
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR,
error_msg);
+ return;
+ }
}
bool is_dir = false;
@@ -92,15 +107,29 @@ void DownloadAction::handle_error_log(HttpRequest* req,
const std::string& file_
Status status = check_log_path_is_allowed(absolute_path);
if (!status.ok()) {
std::string error_msg = status.to_string();
- HttpChannel::send_reply(req, error_msg);
- return;
+ if (status.is_not_authorized()) {
+ HttpChannel::send_reply(req, HttpStatus::UNAUTHORIZED, error_msg);
+ return;
+ } else {
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR,
error_msg);
+ return;
+ }
}
bool is_dir = false;
status = io::global_local_filesystem()->is_directory(absolute_path,
&is_dir);
if (!status.ok()) {
- HttpChannel::send_reply(req, status.to_string());
- return;
+ std::string error_msg = status.to_string();
+ if (status.is_not_found() || status.is_io_error()) {
+ HttpChannel::send_reply(req, HttpStatus::NOT_FOUND, error_msg);
+ return;
+ } else if (status.is_not_authorized()) {
+ HttpChannel::send_reply(req, HttpStatus::UNAUTHORIZED, error_msg);
+ return;
+ } else {
+ HttpChannel::send_reply(req, HttpStatus::INTERNAL_SERVER_ERROR,
error_msg);
+ return;
+ }
}
if (is_dir) {
std::string error_msg = "error log can only be file.";
@@ -135,11 +164,11 @@ void DownloadAction::handle(HttpRequest* req) {
Status DownloadAction::check_token(HttpRequest* req) {
const std::string& token_str = req->param(TOKEN_PARAMETER);
if (token_str.empty()) {
- return Status::InternalError("token is not specified.");
+ return Status::NotAuthorized("token is not specified.");
}
if (token_str != _exec_env->token()) {
- return Status::InternalError("invalid token.");
+ return Status::NotAuthorized("invalid token.");
}
return Status::OK();
@@ -156,7 +185,7 @@ Status DownloadAction::check_path_is_allowed(const
std::string& file_path) {
}
}
- return Status::InternalError("file path is not allowed: {}",
canonical_file_path);
+ return Status::NotAuthorized("file path is not allowed: {}",
canonical_file_path);
}
Status DownloadAction::check_log_path_is_allowed(const std::string& file_path)
{
@@ -168,7 +197,7 @@ Status DownloadAction::check_log_path_is_allowed(const
std::string& file_path) {
return Status::OK();
}
- return Status::InternalError("file path is not allowed: {}", file_path);
+ return Status::NotAuthorized("file path is not allowed: {}", file_path);
}
} // end namespace doris
diff --git a/be/src/http/http_client.h b/be/src/http/http_client.h
index c194c29c88..c80d0c6f56 100644
--- a/be/src/http/http_client.h
+++ b/be/src/http/http_client.h
@@ -103,6 +103,12 @@ public:
curl_off_t cl;
auto code = curl_easy_getinfo(_curl,
CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, &cl);
if (!code) {
+ if (cl < 0) {
+ return Status::InternalError(
+ fmt::format("failed to get content length, it should
be a positive value, "
+ "actrual is : {}",
+ cl));
+ }
*length = cl;
return Status::OK();
}
diff --git a/be/src/olap/task/engine_clone_task.cpp
b/be/src/olap/task/engine_clone_task.cpp
index dfb18ee376..0c7d917a50 100644
--- a/be/src/olap/task/engine_clone_task.cpp
+++ b/be/src/olap/task/engine_clone_task.cpp
@@ -281,9 +281,15 @@ Status
EngineCloneTask::_make_and_download_snapshots(DataDir& data_dir,
// TODO(zc): if snapshot path has been returned from source, it is
some strange to
// concat tablet_id and schema hash here.
std::stringstream ss;
- ss << "http://"; << get_host_port(src.host, src.http_port) <<
HTTP_REQUEST_PREFIX
- << HTTP_REQUEST_TOKEN_PARAM << token << HTTP_REQUEST_FILE_PARAM
<< *snapshot_path
- << "/" << _clone_req.tablet_id << "/" << _clone_req.schema_hash
<< "/";
+ if (snapshot_path->back() == '/') {
+ ss << "http://"; << src.host << ":" << src.http_port <<
HTTP_REQUEST_PREFIX
+ << HTTP_REQUEST_TOKEN_PARAM << token <<
HTTP_REQUEST_FILE_PARAM << *snapshot_path
+ << _clone_req.tablet_id << "/" << _clone_req.schema_hash <<
"/";
+ } else {
+ ss << "http://"; << src.host << ":" << src.http_port <<
HTTP_REQUEST_PREFIX
+ << HTTP_REQUEST_TOKEN_PARAM << token <<
HTTP_REQUEST_FILE_PARAM << *snapshot_path
+ << "/" << _clone_req.tablet_id << "/" <<
_clone_req.schema_hash << "/";
+ }
remote_url_prefix = ss.str();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
