Commit 853ea0ae authored by Ted Meyer's avatar Ted Meyer Committed by Commit Bot

Truncate media log urls, and add tests for it

Sometimes websites can set massive data encodings as the player URL and
this can cause out of memory crashes.

Bug: 1090752
Change-Id: I783daeb7d9ae48061dc299c09952719dffa65197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239373
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777559}
parent 4e4491b3
......@@ -41,4 +41,13 @@ std::string MediaLogEventToString(MediaLogEvent level) {
return "";
}
std::string TruncateUrlString(std::string url) {
constexpr size_t kMaxUrlLength = 1000;
if (url.length() > kMaxUrlLength) {
url.resize(kMaxUrlLength);
url.replace(url.end() - 3, url.end(), "...");
}
return url;
}
} // namespace media
......@@ -67,6 +67,9 @@ enum class MediaLogEvent {
// instead of using macro stringification.
MEDIA_EXPORT std::string MediaLogEventToString(MediaLogEvent level);
// Sometimes URLs can have encoded data that can be exteremly large.
MEDIA_EXPORT std::string TruncateUrlString(std::string url);
// These events can be triggered with no extra associated data.
MEDIA_LOG_EVENT_TYPELESS(kPlay);
MEDIA_LOG_EVENT_TYPELESS(kPause);
......@@ -77,12 +80,15 @@ MEDIA_LOG_EVENT_TYPELESS(kWebMediaPlayerCreated);
// These events can be triggered with the extra data / names as defined here.
// Note that some events can be defined multiple times.
MEDIA_LOG_EVENT_NAMED_DATA(kLoad, std::string, "url");
MEDIA_LOG_EVENT_NAMED_DATA(kSeek, double, "seek_target");
MEDIA_LOG_EVENT_NAMED_DATA(kVideoSizeChanged, gfx::Size, "dimensions");
MEDIA_LOG_EVENT_NAMED_DATA(kDurationChanged, base::TimeDelta, "duration");
MEDIA_LOG_EVENT_NAMED_DATA(kWebMediaPlayerCreated, std::string, "origin_url");
MEDIA_LOG_EVENT_NAMED_DATA(kPipelineStateChange, std::string, "pipeline_state");
MEDIA_LOG_EVENT_NAMED_DATA_OP(kLoad, std::string, "url", TruncateUrlString);
MEDIA_LOG_EVENT_NAMED_DATA_OP(kWebMediaPlayerCreated,
std::string,
"origin_url",
TruncateUrlString);
// Each type of buffering state gets a different name.
MEDIA_LOG_EVENT_NAMED_DATA(
......
......@@ -46,6 +46,16 @@ struct MediaLogEventTypeSupport {};
static std::string TypeName() { return #EVENT; } \
}
#define MEDIA_LOG_EVENT_NAMED_DATA_OP(EVENT, TYPE, DISPLAY, OP) \
template <> \
struct MediaLogEventTypeSupport<MediaLogEvent::EVENT, TYPE> { \
static void AddExtraData(base::Value* params, const TYPE& t) { \
DCHECK(params); \
params->SetKey(DISPLAY, MediaSerialize<TYPE>(OP(t))); \
} \
static std::string TypeName() { return #EVENT; } \
}
// Specifically do not create the Convert or DisplayName methods
#define MEDIA_LOG_EVENT_TYPELESS(EVENT) \
template <> \
......
......@@ -16,15 +16,11 @@ namespace media {
// Friend class of MediaLog for access to internal constants.
class MediaLogTest : public testing::Test {
public:
static constexpr size_t kMaxUrlLength = MediaLog::kMaxUrlLength;
protected:
MediaLog media_log;
};
constexpr size_t MediaLogTest::kMaxUrlLength;
std::unique_ptr<MockMediaLog> root_log_;
void CreateLog() { root_log_ = std::make_unique<MockMediaLog>(); }
};
TEST_F(MediaLogTest, EventsAreForwarded) {
// Make sure that |root_log_| receives events.
......@@ -47,10 +43,61 @@ TEST_F(MediaLogTest, EventsAreNotForwardedAfterInvalidate) {
TEST_F(MediaLogTest, ClonedLogsInhertParentPlayerId) {
std::unique_ptr<MockMediaLog> root_log(std::make_unique<MockMediaLog>());
std::unique_ptr<MediaLog> child_media_log(root_log->Clone());
EXPECT_CALL(*root_log, DoAddLogRecordLogString(_)).Times(1);
child_media_log->AddMessage(MediaLogMessageLevel::kERROR, "test");
auto event = root_log->take_most_recent_event();
EXPECT_NE(event, nullptr);
EXPECT_EQ(event->id, root_log->id());
}
TEST_F(MediaLogTest, DontTruncateShortUrlString) {
CreateLog();
EXPECT_CALL(*root_log_, DoAddLogRecordLogString(_)).Times(2);
const std::string short_url("chromium.org");
// Verify that LoadEvent does not truncate the short URL.
root_log_->AddEvent<MediaLogEvent::kLoad>(short_url);
auto event = root_log_->take_most_recent_event();
EXPECT_NE(event, nullptr);
EXPECT_EQ(*event->params.FindStringPath("url"), "chromium.org");
// Verify that CreatedEvent does not truncate the short URL.
root_log_->AddEvent<MediaLogEvent::kWebMediaPlayerCreated>(short_url);
event = root_log_->take_most_recent_event();
EXPECT_NE(event, nullptr);
EXPECT_EQ(*event->params.FindStringPath("origin_url"), "chromium.org");
}
TEST_F(MediaLogTest, TruncateLongUrlStrings) {
CreateLog();
EXPECT_CALL(*root_log_, DoAddLogRecordLogString(_)).Times(2);
// Build a long string that exceeds the URL length limit.
std::stringstream string_builder;
constexpr size_t kLongStringLength = 1010;
for (size_t i = 0; i < kLongStringLength; i++) {
string_builder << "c";
}
const std::string long_url = string_builder.str();
std::stringstream expected_string_builder;
constexpr size_t kMaxLength = 1000;
for (size_t i = 0; i < kMaxLength - 3; i++) {
expected_string_builder << "c";
}
expected_string_builder << "...";
const std::string expected_url = expected_string_builder.str();
// Verify that LoadEvent does not truncate the short URL.
root_log_->AddEvent<MediaLogEvent::kLoad>(long_url);
auto event = root_log_->take_most_recent_event();
EXPECT_NE(event, nullptr);
EXPECT_EQ(*event->params.FindStringPath("url"), expected_url);
// Verify that CreatedEvent does not truncate the short URL.
root_log_->AddEvent<MediaLogEvent::kWebMediaPlayerCreated>(long_url);
event = root_log_->take_most_recent_event();
EXPECT_NE(event, nullptr);
EXPECT_EQ(*event->params.FindStringPath("origin_url"), expected_url);
}
} // namespace media
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment