This is an automated email from the ASF dual-hosted git repository.
mrhhsg 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 aed8ecd889a [fix](be) Redact HTTP query details from request
diagnostics (#69506)
aed8ecd889a is described below
commit aed8ecd889a1885944954e887e4b282a0660396b
Author: Jerry Hu <[email protected]>
AuthorDate: Mon Oct 26 18:40:43 2026 +0800
[fix](be) Redact HTTP query details from request diagnostics (#69506)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: `HttpRequest::debug_string()` rendered the full request
URI and every parsed parameter key and value, so query credentials could
appear in BE diagnostics. The query-parse failure warning also included
the raw query. Render the parsed path and parameter count instead, and
omit query text from that warning. Request parsing and URI/parameter
accessors retain their existing behavior. This change is limited to
these logging paths; action-specific HTTP logs are unchanged.
### Release note
BE `HttpRequest::debug_string()` no longer emits query strings or
parameter keys and values. Query-parse failure warnings no longer emit
the raw query.
### Check List (For Author)
- Test:
- Unit Test: `HttpRequestTest.*` passed (9/9). The unmodified full BE UT
build was blocked by a pre-existing compile error in an unrelated test
translation unit; the focused suite was run with that unit temporarily
excluded from the UT build.
- Build: `BUILD_TYPE=ASAN ./build.sh --be` passed.
- Static and format checks: BE build hygiene, clang-format,
check-format, and changed-line clang-tidy passed.
- Regression test: Not run; this changes BE diagnostic log text, not SQL
or HTTP responses.
- Behavior changed: Yes (diagnostic log text only).
- Does this need documentation: No.
---
be/src/service/http/http_request.cpp | 9 ++--
be/test/service/http/http_request_test.cpp | 77 ++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 6 deletions(-)
diff --git a/be/src/service/http/http_request.cpp
b/be/src/service/http/http_request.cpp
index e511ff577fd..a9182c9e4d5 100644
--- a/be/src/service/http/http_request.cpp
+++ b/be/src/service/http/http_request.cpp
@@ -105,7 +105,7 @@ int HttpRequest::init_from_evhttp() {
struct evkeyvalq params;
auto res = evhttp_parse_query_str(query, ¶ms);
if (res < 0) {
- LOG(WARNING) << "parse query str failed, query=" << query;
+ LOG(WARNING) << "parse query str failed";
return res;
}
for (auto param = params.tqh_first; param != nullptr; param =
param->next.tqe_next) {
@@ -120,7 +120,7 @@ std::string HttpRequest::debug_string() const {
std::stringstream ss;
ss << "HttpRequest: \n"
<< "method:" << _method << "\n"
- << "uri:" << _uri << "\n"
+ << "uri:" << _raw_path << "\n"
<< "raw_path:" << _raw_path << "\n"
<< "headers: \n";
for (auto& iter : _headers) {
@@ -131,10 +131,7 @@ std::string HttpRequest::debug_string() const {
ss << "key=" << iter.first << ", value=" << iter.second << "\n";
}
}
- ss << "params: \n";
- for (auto& iter : _params) {
- ss << "key=" << iter.first << ", value=" << iter.second << "\n";
- }
+ ss << "params_count:" << _params.size() << "\n";
return ss.str();
}
diff --git a/be/test/service/http/http_request_test.cpp
b/be/test/service/http/http_request_test.cpp
index fdb9df43f75..228ceef241a 100644
--- a/be/test/service/http/http_request_test.cpp
+++ b/be/test/service/http/http_request_test.cpp
@@ -18,6 +18,7 @@
#include "service/http/http_request.h"
#include <event2/http.h>
+#include <event2/http_struct.h>
#include <gtest/gtest.h>
#include <cstring>
@@ -139,4 +140,80 @@ TEST_F(HttpRequestTest,
non_sensitive_headers_are_untouched) {
EXPECT_EQ(dumped.find(kMasked), std::string::npos) << dumped;
}
+TEST_F(HttpRequestTest, query_credentials_are_not_exposed_in_debug_string) {
+ constexpr char uri[] = "/api/_download?token=synthetic-token-123&job_id=7";
+ auto* evhttp_req = evhttp_request_new(nullptr, nullptr);
+ evhttp_req->type = EVHTTP_REQ_GET;
+ evhttp_req->uri = const_cast<char*>(uri);
+ evhttp_req->uri_elems = evhttp_uri_parse(uri);
+
+ HttpRequest req(evhttp_req);
+ EXPECT_EQ(req.init_from_evhttp(), 0);
+ const std::string dumped = req.debug_string();
+ EXPECT_EQ(req.uri(), uri);
+ EXPECT_EQ(req.param("token"), "synthetic-token-123");
+ EXPECT_EQ(req.query_params().at("job_id"), "7");
+ EXPECT_NE(dumped.find("uri:/api/_download\n"), std::string::npos) <<
dumped;
+ EXPECT_NE(dumped.find("params_count:2\n"), std::string::npos) << dumped;
+ EXPECT_EQ(dumped.find("synthetic-token-123"), std::string::npos) << dumped;
+ EXPECT_EQ(dumped.find("?token="), std::string::npos) << dumped;
+ EXPECT_EQ(dumped.find("key=token"), std::string::npos) << dumped;
+ EXPECT_EQ(dumped.find("key=job_id"), std::string::npos) << dumped;
+ EXPECT_EQ(dumped.find("job_id=7"), std::string::npos) << dumped;
+
+ evhttp_uri_free(evhttp_req->uri_elems);
+ evhttp_req->uri = nullptr;
+ evhttp_req->uri_elems = nullptr;
+ evhttp_request_free(evhttp_req);
+}
+
+TEST_F(HttpRequestTest,
encoded_query_and_route_params_are_not_exposed_in_debug_string) {
+ constexpr char uri[] =
"/api/_download?Token=synthetic%2Bcredential&opaque=another-secret";
+ auto* evhttp_req = evhttp_request_new(nullptr, nullptr);
+ evhttp_req->type = EVHTTP_REQ_GET;
+ evhttp_req->uri = const_cast<char*>(uri);
+ evhttp_req->uri_elems = evhttp_uri_parse(uri);
+
+ HttpRequest req(evhttp_req);
+ EXPECT_EQ(req.init_from_evhttp(), 0);
+ req.params()->emplace("route_id", "private-route-value");
+ const std::string dumped = req.debug_string();
+ EXPECT_EQ(req.uri(), uri);
+ EXPECT_EQ(req.param("Token"), "synthetic+credential");
+ EXPECT_EQ(req.query_params().at("opaque"), "another-secret");
+ EXPECT_EQ(req.param("route_id"), "private-route-value");
+ EXPECT_NE(dumped.find("uri:/api/_download\n"), std::string::npos) <<
dumped;
+ EXPECT_NE(dumped.find("params_count:3\n"), std::string::npos) << dumped;
+ for (const std::string& secret :
+ {"synthetic%2Bcredential", "synthetic+credential", "another-secret",
"Token", "opaque",
+ "route_id", "private-route-value"}) {
+ EXPECT_EQ(dumped.find(secret), std::string::npos) << dumped;
+ }
+
+ evhttp_uri_free(evhttp_req->uri_elems);
+ evhttp_req->uri = nullptr;
+ evhttp_req->uri_elems = nullptr;
+ evhttp_request_free(evhttp_req);
+}
+
+TEST_F(HttpRequestTest, request_without_query_keeps_path_in_debug_string) {
+ constexpr char uri[] = "/api/_download";
+ auto* evhttp_req = evhttp_request_new(nullptr, nullptr);
+ evhttp_req->type = EVHTTP_REQ_GET;
+ evhttp_req->uri = const_cast<char*>(uri);
+ evhttp_req->uri_elems = evhttp_uri_parse(uri);
+
+ HttpRequest req(evhttp_req);
+ EXPECT_EQ(req.init_from_evhttp(), 0);
+ const std::string dumped = req.debug_string();
+ EXPECT_EQ(req.uri(), uri);
+ EXPECT_NE(dumped.find("uri:/api/_download\n"), std::string::npos) <<
dumped;
+ EXPECT_NE(dumped.find("params_count:0\n"), std::string::npos) << dumped;
+
+ evhttp_uri_free(evhttp_req->uri_elems);
+ evhttp_req->uri = nullptr;
+ evhttp_req->uri_elems = nullptr;
+ evhttp_request_free(evhttp_req);
+}
+
} // namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
