You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/03/01 20:02:10 UTC
[3/3] kudu git commit: KUDU-1898: /varz page doesn't HTML-escape flag
values
KUDU-1898: /varz page doesn't HTML-escape flag values
This fixes the flag values are not properly HTML-escaped in the /varz web page.
Verfied the flag values will be HTML-escaped in WebserverTest.TestRedactFlagsDump.
Also I manually validated that the /varz page output is as expected.
Change-Id: I64ebb0befe6bacb0fc90d50a1e34870656041a01
Reviewed-on: http://gerrit.cloudera.org:8080/6173
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/93b54f0f
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/93b54f0f
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/93b54f0f
Branch: refs/heads/master
Commit: 93b54f0fff4df90e43bf6874122d47b325678356
Parents: 0ee93e3
Author: hahao <ha...@cloudera.com>
Authored: Tue Feb 28 18:05:45 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Mar 1 20:01:29 2017 +0000
----------------------------------------------------------------------
src/kudu/server/default-path-handlers.cc | 12 +++++---
src/kudu/server/webserver-test.cc | 7 ++++-
src/kudu/util/flag_tags-test.cc | 2 +-
src/kudu/util/flags.cc | 40 ++++++++++++++++-----------
src/kudu/util/flags.h | 10 +++++--
5 files changed, 47 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/93b54f0f/src/kudu/server/default-path-handlers.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/default-path-handlers.cc b/src/kudu/server/default-path-handlers.cc
index 064d6ea..4182c39 100644
--- a/src/kudu/server/default-path-handlers.cc
+++ b/src/kudu/server/default-path-handlers.cc
@@ -111,14 +111,18 @@ static void LogsHandler(const Webserver::WebRequest& req, std::ostringstream* ou
}
}
-// Registered to handle "/flags", and prints out all command-line flags and their values
-// If --redact is set with 'flag', the values of flags tagged as sensitive will
-// be redacted.
+// Registered to handle "/flags", and prints out all command-line flags and their HTML
+// escaped values. If --redact is set with 'flag', the values of flags tagged as
+// sensitive will be redacted. The values would not be HTML escaped if in the raw text
+// mode, e.g. "/varz?raw".
static void FlagsHandler(const Webserver::WebRequest& req, std::ostringstream* output) {
bool as_text = (req.parsed_args.find("raw") != req.parsed_args.end());
Tags tags(as_text);
+
(*output) << tags.header << "Command-line Flags" << tags.end_header;
- (*output) << tags.pre_tag << CommandlineFlagsIntoString() << tags.end_pre_tag;
+ (*output) << tags.pre_tag
+ << CommandlineFlagsIntoString(as_text ? EscapeMode::NONE : EscapeMode::HTML)
+ << tags.end_pre_tag;
}
// Registered to handle "/memz", and prints out memory allocation statistics.
http://git-wip-us.apache.org/repos/asf/kudu/blob/93b54f0f/src/kudu/server/webserver-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver-test.cc b/src/kudu/server/webserver-test.cc
index 9fd16ad..3638141 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -131,7 +131,12 @@ TEST_F(WebserverTest, TestDefaultPaths) {
}
TEST_F(WebserverTest, TestRedactFlagsDump) {
- // Test varz -- check for the sensitive flag is redacted.
+ // Test varz -- check for the sensitive flag is redacted and HTML-escaped.
+ ASSERT_OK(curl_.FetchURL(strings::Substitute("http://$0/varz", addr_.ToString()),
+ &buf_));
+ ASSERT_STR_CONTAINS(buf_.ToString(), "--test_sensitive_flag=<redacted>");
+
+ // Test varz?raw -- check for the sensitive flag is redacted and not HTML-escaped.
ASSERT_OK(curl_.FetchURL(strings::Substitute("http://$0/varz?raw=1", addr_.ToString()),
&buf_));
ASSERT_STR_CONTAINS(buf_.ToString(), strings::Substitute("--test_sensitive_flag=$0",
http://git-wip-us.apache.org/repos/asf/kudu/blob/93b54f0f/src/kudu/util/flag_tags-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flag_tags-test.cc b/src/kudu/util/flag_tags-test.cc
index 0cf850a..13ee535 100644
--- a/src/kudu/util/flag_tags-test.cc
+++ b/src/kudu/util/flag_tags-test.cc
@@ -120,7 +120,7 @@ TEST_F(FlagTagsTest, TestUnlockFlags) {
TEST_F(FlagTagsTest, TestSensitiveFlags) {
// Setting a sensitive flag should return a redacted value.
{
- ASSERT_STR_CONTAINS(CommandlineFlagsIntoString(), strings::Substitute(
+ ASSERT_STR_CONTAINS(CommandlineFlagsIntoString(EscapeMode::NONE), strings::Substitute(
"--test_sensitive_flag=$0", kRedactionMessage));
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/93b54f0f/src/kudu/util/flags.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index 875f958..c410c19 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -418,18 +418,22 @@ void CheckFlagsAllowed() {
}
// Redact the flag tagged as 'sensitive', if --redact is set
-// with 'flag'. Otherwise, return its value as-is.
-string CheckFlagAndRedact(const CommandLineFlagInfo& flag) {
- string retval;
+// with 'flag'. Otherwise, return its value as-is. If EscapeMode
+// is set to HTML, return HTML escaped string.
+string CheckFlagAndRedact(const CommandLineFlagInfo& flag, EscapeMode mode) {
+ string ret_value;
unordered_set<string> tags;
GetFlagTags(flag.name, &tags);
if (ContainsKey(tags, "sensitive") && g_should_redact_flag) {
- retval += kRedactionMessage;
+ ret_value = kRedactionMessage;
} else {
- retval += flag.current_value;
+ ret_value = flag.current_value;
}
- return retval;
+ if (mode == EscapeMode::HTML) {
+ ret_value = EscapeForHtmlToString(ret_value);
+ }
+ return ret_value;
}
void SetUmask() {
@@ -482,19 +486,23 @@ void HandleCommonFlags() {
#endif
}
-string CommandlineFlagsIntoString() {
- string retval;
+string CommandlineFlagsIntoString(EscapeMode mode) {
+ string ret_value;
vector<CommandLineFlagInfo> flags;
GetAllFlags(&flags);
for (const auto& f : flags) {
- retval += "--";
- retval += f.name;
- retval += "=";
- retval += CheckFlagAndRedact(f);
- retval += "\n";
+ ret_value += "--";
+ if (mode == EscapeMode::HTML) {
+ ret_value += EscapeForHtmlToString(f.name);
+ } else if (mode == EscapeMode::NONE) {
+ ret_value += f.name;
+ }
+ ret_value += "=";
+ ret_value += CheckFlagAndRedact(f, mode);
+ ret_value += "\n";
}
- return retval;
+ return ret_value;
}
string GetNonDefaultFlags(const GFlagsMap& default_flags) {
@@ -517,8 +525,8 @@ string GetNonDefaultFlags(const GFlagsMap& default_flags) {
// Redact the flags tagged as sensitive, if --redact is set
// with 'flag'.
- string flagValue = CheckFlagAndRedact(flag);
- args << "--" << flag.name << '=' << flagValue;
+ string flag_value = CheckFlagAndRedact(flag, EscapeMode::NONE);
+ args << "--" << flag.name << '=' << flag_value;
}
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/93b54f0f/src/kudu/util/flags.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/flags.h b/src/kudu/util/flags.h
index aa6bd94..ae20564 100644
--- a/src/kudu/util/flags.h
+++ b/src/kudu/util/flags.h
@@ -50,10 +50,16 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags);
// google::ParseCommandLineNonHelpFlags().
void HandleCommonFlags();
+enum class EscapeMode {
+ HTML,
+ NONE
+};
+
// Stick the flags into a string. If --redact is set with 'flag',
// the values of flags tagged as sensitive will be redacted. Otherwise,
-// the values will be written to the string as-is.
-std::string CommandlineFlagsIntoString();
+// the values will be written to the string as-is. The values will
+// be HTML escaped if EscapeMode is HTML.
+std::string CommandlineFlagsIntoString(EscapeMode mode);
typedef std::unordered_map<std::string, google::CommandLineFlagInfo> GFlagsMap;