limit and validate responses before logging the text (#292)

Improvement: When logging HTTP responses, the body is now sanity checked to ensure it is human readable, and the length of the output is capped to prevent inadvertent log bloat

5 files changed, 111 insertions, 1 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f947bb0d..5777c2463 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,5 @@
 ##
+- Improvement: When logging HTTP responses, the body is now sanity checked to ensure it is human readable, and the length of the output is capped to prevent inadvertent log bloat
 - **EXPERIMENTAL** `zen builds`
   - Improvement: Better error reporting and faster exit on error
   - Improvement: Downloads without `--clean` now preserves untracked files in target folder
diff --git a/src/zencore/include/zencore/string.h b/src/zencore/include/zencore/string.h
index e2ef1c1a0..68129b691 100644
--- a/src/zencore/include/zencore/string.h
+++ b/src/zencore/include/zencore/string.h
@@ -522,6 +522,9 @@ public:
 
 //////////////////////////////////////////////////////////////////////////
 
+bool							 IsValidUtf8(const std::string_view& str);
+std::string_view::const_iterator FindFirstInvalidUtf8Byte(const std::string_view& str);
+
 void		 Utf8ToWide(const char8_t* str, WideStringBuilderBase& out);
 void		 Utf8ToWide(const std::u8string_view& wstr, WideStringBuilderBase& out);
 void		 Utf8ToWide(const std::string_view& wstr, WideStringBuilderBase& out);
diff --git a/src/zencore/string.cpp b/src/zencore/string.cpp
index 242d41abe..a0d8c927f 100644
--- a/src/zencore/string.cpp
+++ b/src/zencore/string.cpp
@@ -99,6 +99,20 @@ FilepathFindExtension(const std::string_view& Path, const char* ExtensionToMatch
 
 //////////////////////////////////////////////////////////////////////////
 
+bool
+IsValidUtf8(const std::string_view& str)
+{
+	return utf8::is_valid(begin(str), end(str));
+}
+
+std::string_view::const_iterator
+FindFirstInvalidUtf8Byte(const std::string_view& str)
+{
+	return utf8::find_invalid(begin(str), end(str));
+}
+
+//////////////////////////////////////////////////////////////////////////
+
 void
 Utf8ToWide(const char8_t* Str8, WideStringBuilderBase& OutString)
 {
diff --git a/src/zenhttp/httpclient.cpp b/src/zenhttp/httpclient.cpp
index 7f7e70fef..e4c6d243d 100644
--- a/src/zenhttp/httpclient.cpp
+++ b/src/zenhttp/httpclient.cpp
@@ -1422,6 +1422,52 @@ HttpClient::Response::ThrowError(std::string_view ErrorPrefix)
 
 #if ZEN_WITH_TESTS
 
+TEST_CASE("responseformat")
+{
+	using namespace std::literals;
+
+	SUBCASE("identity")
+	{
+		BodyLogFormatter _{"abcd"};
+		CHECK_EQ(_.GetText(), "abcd"sv);
+	}
+
+	SUBCASE("very long")
+	{
+		std::string_view LongView =
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
+			"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz";
+
+		BodyLogFormatter _{LongView};
+
+		CHECK(_.GetText().size() < LongView.size());
+		CHECK(_.GetText().starts_with("[truncated"sv));
+	}
+
+	SUBCASE("invalid text")
+	{
+		std::string_view BadText = "totobaba\xff\xfe";
+
+		BodyLogFormatter _{BadText};
+
+		CHECK_EQ(_.GetText(), "totobaba");
+	}
+}
+
 TEST_CASE("httpclient")
 {
 	using namespace std::literals;
diff --git a/src/zenhttp/include/zenhttp/formatters.h b/src/zenhttp/include/zenhttp/formatters.h
index 538136238..0fa5dc6da 100644
--- a/src/zenhttp/include/zenhttp/formatters.h
+++ b/src/zenhttp/include/zenhttp/formatters.h
@@ -13,6 +13,50 @@ ZEN_THIRD_PARTY_INCLUDES_START
 #include <fmt/format.h>
 ZEN_THIRD_PARTY_INCLUDES_END
 
+namespace zen {
+
+struct BodyLogFormatter
+{
+private:
+	std::string_view				  ResponseText;
+	zen::ExtendableStringBuilder<128> ModifiedResponse;
+
+public:
+	explicit BodyLogFormatter(std::string_view InResponseText) : ResponseText(InResponseText)
+	{
+		using namespace std::literals;
+
+		const int TextSizeLimit = 1024;
+
+		// Trim invalid UTF8
+
+		auto InvalidIt = zen::FindFirstInvalidUtf8Byte(ResponseText);
+
+		if (InvalidIt != end(ResponseText))
+		{
+			ResponseText = ResponseText.substr(0, InvalidIt - begin(ResponseText));
+		}
+
+		if (ResponseText.empty())
+		{
+			ResponseText = "<suppressed non-text response>"sv;
+		}
+
+		if (ResponseText.size() > TextSizeLimit)
+		{
+			const auto TruncatedString = "[truncated response] "sv;
+			ModifiedResponse.Append(TruncatedString);
+			ModifiedResponse.Append(ResponseText.data(), TextSizeLimit - TruncatedString.size());
+
+			ResponseText = ModifiedResponse;
+		}
+	}
+
+	inline std::string_view GetText() const { return ResponseText; }
+};
+
+}  // namespace zen
+
 template<>
 struct fmt::formatter<cpr::Response>
 {
@@ -57,6 +101,8 @@ struct fmt::formatter<cpr::Response>
 			}
 			else
 			{
+				zen::BodyLogFormatter Body(Response.text);
+
 				return fmt::format_to(Ctx.out(),
 									  "Url: {}, Status: {}, Bytes: {}/{} (Up/Down), Elapsed: {}s, Reponse: '{}', Reason: '{}'",
 									  Response.url.str(),
@@ -64,7 +110,7 @@ struct fmt::formatter<cpr::Response>
 									  Response.uploaded_bytes,
 									  Response.downloaded_bytes,
 									  Response.elapsed,
-									  Response.text,
+									  Body.GetText(),
 									  Response.reason);
 			}
 		}