wwbmmm commented on code in PR #3529:
URL: https://github.com/apache/brpc/pull/3529#discussion_r3949871344
##########
test/brpc_builtin_service_unittest.cpp:
##########
@@ -686,6 +697,58 @@ 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;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
Review Comment:
Done in 4619d7fd: `req`/`res` are now declared inside each `{ ... }` block
so every `default_method` invocation gets fresh, empty request/response
objects. Full brpc_builtin_service_unittest suite still passes (20/20).
---
🤖 This reply was automatically generated by brpc-oncall
##########
test/brpc_builtin_service_unittest.cpp:
##########
@@ -686,6 +697,58 @@ 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;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ 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;
+ 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);
Review Comment:
Done in 4619d7fd: `req`/`res` are now declared inside each `{ ... }` block
so every `default_method` invocation gets fresh, empty request/response
objects. Full brpc_builtin_service_unittest suite still passes (20/20).
---
🤖 This reply was automatically generated by brpc-oncall
##########
test/brpc_builtin_service_unittest.cpp:
##########
@@ -686,6 +697,58 @@ 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;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ 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;
+ 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;
+ 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);
Review Comment:
Done in 4619d7fd: `req`/`res` are now declared inside each `{ ... }` block
so every `default_method` invocation gets fresh, empty request/response
objects. Full brpc_builtin_service_unittest suite still passes (20/20).
---
🤖 This reply was automatically generated by brpc-oncall
##########
test/brpc_builtin_service_unittest.cpp:
##########
@@ -686,6 +697,58 @@ 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;
+ brpc::FlagsRequest req;
+ brpc::FlagsResponse res;
+ 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;
+ 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;
+ 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;
+ 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);
Review Comment:
Done in 4619d7fd: `req`/`res` are now declared inside each `{ ... }` block
so every `default_method` invocation gets fresh, empty request/response
objects. Full brpc_builtin_service_unittest suite still passes (20/20).
---
🤖 This reply was automatically generated by brpc-oncall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
