This is an automated email from the ASF dual-hosted git repository.
wasphin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new bb0861e1 Escape HTML output in builtin /flags service (#3529)
bb0861e1 is described below
commit bb0861e1830c03031ab333e123c8caffa5e71b57
Author: Weibing Wang <[email protected]>
AuthorDate: Mon Sep 14 13:16:36 2026 +0800
Escape HTML output in builtin /flags service (#3529)
* Escape HTML output in builtin /flags service
The ?setvalue= confirmation message and the ?setvalue&withform page
wrote the gflag name/value into the html page without escaping, unlike
the flag list page which already escapes values with HtmlReplace.
Escape them with WebEscape like rpcz_service does. Plain text output
is unchanged.
* Use FlagSaver in flags_escaping unittest
Replace the manual save/restore of the modified gflag with
GFLAGS_NAMESPACE::FlagSaver so the flag value is restored on every
exit of the test, following the established pattern in
brpc_http_rpc_protocol_unittest.cpp.
* Scope req/res per sub-case in flags_escaping unittest
Declare FlagsRequest/FlagsResponse inside each { ... } block so no
state can leak between the default_method invocations.
---
src/brpc/builtin/flags_service.cpp | 9 +++--
test/brpc_builtin_service_unittest.cpp | 67 ++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/src/brpc/builtin/flags_service.cpp
b/src/brpc/builtin/flags_service.cpp
index 18152baa..569da313 100644
--- a/src/brpc/builtin/flags_service.cpp
+++ b/src/brpc/builtin/flags_service.cpp
@@ -117,11 +117,11 @@ void FlagsService::set_value_page(Controller* cntl,
const bool is_string = (info.type == "string");
os << "<!DOCTYPE html><html><body>"
"<form action='' method='get'>"
- " Set `" << name << "' from ";
+ " Set `" << WebEscape(name) << "' from ";
if (is_string) {
os << '"';
}
- os << info.current_value;
+ os << WebEscape(info.current_value);
if (is_string) {
os << '"';
}
@@ -177,9 +177,12 @@ void
FlagsService::default_method(::google::protobuf::RpcController* cntl_base,
return;
}
butil::IOBufBuilder os;
- os << "Set `" << constraint << "' to " << *value_str;
if (use_html) {
+ os << "Set `" << WebEscape(constraint) << "' to "
+ << WebEscape(*value_str);
os << "<br><a href='/flags'>[back to flags]</a>";
+ } else {
+ os << "Set `" << constraint << "' to " << *value_str;
}
os.move_to(cntl->response_attachment());
return;
diff --git a/test/brpc_builtin_service_unittest.cpp
b/test/brpc_builtin_service_unittest.cpp
index e496c05f..0d1ca248 100644
--- a/test/brpc_builtin_service_unittest.cpp
+++ b/test/brpc_builtin_service_unittest.cpp
@@ -60,6 +60,17 @@
DEFINE_bool(foo, false, "Flags for UT");
BRPC_VALIDATE_GFLAG(foo, brpc::PassValidate);
+// A reloadable string gflag so that FlagsService is able to modify its
+// value via ?setvalue=. String flags must register the validator manually,
+// see comments in butil/reloadable_flags.h.
+DEFINE_string(reloadable_string_flag_for_ut, "", "Flags for UT");
+static bool PassValidateStringFlag(const char*, const std::string&) {
+ return true;
+}
+const bool ALLOW_UNUSED dummy_validate_reloadable_string_flag_for_ut =
+ GFLAGS_NAMESPACE::RegisterFlagValidator(
+ &FLAGS_reloadable_string_flag_for_ut, PassValidateStringFlag);
+
namespace brpc {
DECLARE_bool(enable_rpcz);
DECLARE_bool(rpcz_hex_log_id);
@@ -686,6 +697,62 @@ TEST_F(BuiltinServiceTest, flags) {
TestFlags(true);
}
+TEST_F(BuiltinServiceTest, flags_escaping) {
+ // Save all flags and restore them on any exit of this test, since the
+ // /flags service below modifies `reloadable_string_flag_for_ut'.
+ GFLAGS_NAMESPACE::FlagSaver flag_saver;
+ brpc::FlagsService service;
+ const std::string payload = "<svg onload=alert(1)>&\"'";
+ const std::string escaped = brpc::WebEscape(payload);
+
+ // Reflected: the ?setvalue= value is echoed into the html page.
+ {
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ SetUpController(&cntl, true);
+ cntl.http_request()._unresolved_path = "reloadable_string_flag_for_ut";
+ cntl.http_request().uri().SetQuery(brpc::SETVALUE_STR, payload);
+ service.default_method(&cntl, &req, &res, &done);
+ EXPECT_FALSE(cntl.Failed());
+ const std::string& body = cntl.response_attachment().to_string();
+ EXPECT_EQ(std::string::npos, body.find(payload))
+ << "unescaped payload in html: " << body;
+ CheckContent(cntl, escaped.c_str());
+ }
+ // Stored: ?setvalue&withform renders the flag value stored above.
+ {
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ SetUpController(&cntl, true);
+ cntl.http_request()._unresolved_path = "reloadable_string_flag_for_ut";
+ cntl.http_request().uri().SetQuery(brpc::SETVALUE_STR, "");
+ cntl.http_request().uri().SetQuery("withform", "");
+ service.default_method(&cntl, &req, &res, &done);
+ EXPECT_FALSE(cntl.Failed());
+ const std::string& body = cntl.response_attachment().to_string();
+ EXPECT_EQ(std::string::npos, body.find(payload))
+ << "unescaped payload in html: " << body;
+ CheckContent(cntl, escaped.c_str());
+ }
+ // Plain text output is not html-escaped.
+ {
+ ClosureChecker done;
+ brpc::Controller cntl;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ SetUpController(&cntl, false);
+ cntl.http_request()._unresolved_path = "reloadable_string_flag_for_ut";
+ cntl.http_request().uri().SetQuery(brpc::SETVALUE_STR, payload);
+ service.default_method(&cntl, &req, &res, &done);
+ EXPECT_FALSE(cntl.Failed());
+ CheckContent(cntl, payload.c_str());
+ }
+}
+
TEST_F(BuiltinServiceTest, bad_method) {
TestBadMethod(false);
TestBadMethod(true);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
