Implement MediaMetadata artwork in content

content/common/media/media_metadata_sanitizer.cc

Index: content/common/media/media_metadata_sanitizer.cc
diff --git a/content/common/media/media_metadata_sanitizer.cc b/content/common/media/media_metadata_sanitizer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b0b837b62213da7b4c947c8d6f3b1dace880690d
--- /dev/null
+++ b/content/common/media/media_metadata_sanitizer.cc
@@ -0,0 +1,116 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/common/media/media_metadata_sanitizer.h"
+
+#include <algorithm>
+#include <string>
+
+namespace content {
+
+namespace {
+
+// Maximum length for all the strings inside the MediaMetadata when it is sent
+// over IPC. The renderer process should truncate the strings before sending
+// the MediaMetadata and the browser process must do the same when receiving
+// it.
+const size_t kMaxIPCStringLength = 4 * 1024;
+
+// Maximum length of artwork src.
+const size_t kMaxArtworkSrcLength = 64 * 1024;

[dcheng, 2016/07/05 02:36:55]

FWIW, there's also a kMaxURLChars (which is 2MB). I don't know what a reasonable
limit for media metadata is though, so if 64KB is a reasonable upper bound, that
seems fine with me.

[Zhiqiang Zhang (Slow), 2016/07/05 15:30:30]

Let's use kMaxURLChars.

+
+// Maximum type length of Artwork, which conforms to RFC 4288
+// (https://tools.ietf.org/html/rfc4288).
+const size_t kMaxArtworkTypeLength = 2 * 127 + 1;
+
+// Maximum number of artwork images inside the MediaMetadata.
+const size_t kMaxNumberOfArtworkImages = 10;
+
+// Maximum of sizes in an artwork image.
+const size_t kMaxNumberOfArtworkSizes = 10;
+
+bool CheckArtworkSrcSanity(const GURL& src) {
+  if (!src.is_valid() || !src.IsStandard())

[dcheng, 2016/07/05 02:36:55]

The standard scheme check seems redundant in conjunction with the HTTPOrHTTPS
check below.

[Zhiqiang Zhang (Slow), 2016/07/05 15:30:30]

Done.

+    return false;
+  if (!src.SchemeIsHTTPOrHTTPS() && src.scheme() != url::kDataScheme)

[Zhiqiang Zhang (Slow), 2016/07/01 21:05:34]

palmer@, actually I found that WebContents::DownloadImage() does not support
file urls. Since it sends real HTTP requests. I wrote some tests in
https://codereview.chromium.org/2113893005/.

Also, DownloadImage() supports data URIs so increased the artwork src limit.

Could we land this CL first and land the DownloadImage() tests later?

+    return false;
+  if (src.spec().size() > kMaxArtworkSrcLength)
+    return false;
+
+  return true;
+}
+
+bool CheckArtworkSanity(const MediaMetadata::Artwork& artwork) {
+  if (!CheckArtworkSrcSanity(artwork.src))
+    return false;
+  if (artwork.type.is_null())
+    return false;
+  if (artwork.type.string().size() > kMaxArtworkTypeLength)
+    return false;
+  if (artwork.sizes.size() > kMaxNumberOfArtworkSizes)
+    return false;
+
+  return true;
+}
+
+// Sanitize artwork. The method should not be called if |artwork.src| is bad.
+MediaMetadata::Artwork SanitizeArtwork(const MediaMetadata::Artwork& artwork) {
+  MediaMetadata::Artwork sanitized_artwork;
+
+  sanitized_artwork.src = artwork.src;
+  sanitized_artwork.type = artwork.type.is_null() ?
+      base::NullableString16() :
+      base::NullableString16(
+          artwork.type.string().substr(0, kMaxArtworkTypeLength), false);
+  for (const auto& size : artwork.sizes()) {
+    sanitized_artwork.sizes.push_back(artwork.sizes[i]);
+    if (sanitized_artwork.sizes.size() > kMaxNumberOfArtworkSizes)
+      break;
+  }
+
+  return sanitized_artwork;
+}
+
+}  // anonymous namespace
+
+bool MediaMetadataSanitizer::CheckSanity(const MediaMetadata& metadata) {
+  if (metadata.title.size() > kMaxIPCStringLength)
+    return false;
+  if (metadata.artist.size() > kMaxIPCStringLength)
+    return false;
+  if (metadata.album.size() > kMaxIPCStringLength)
+    return false;
+  if (metadata.artwork.size() > kMaxNumberOfArtworkImages)
+    return false;
+
+  for (const auto& artwork : metadata.artwork) {
+    if (CheckArtworkSanity(artwork))
+        return false;
+  }
+
+  return true;
+}
+
+MediaMetadata MediaMetadataSanitizer::Sanitize(const MediaMetadata& metadata) {
+  MediaMetadata sanitized_metadata;
+
+  sanitized_metadata.title = metadata.title.substr(0, kMaxIPCStringLength);
+  sanitized_metadata.artist = metadata.artist.substr(0, kMaxIPCStringLength);
+  sanitized_metadata.album = metadata.album.substr(0, kMaxIPCStringLength);
+
+  for (const auto& artwork : metadata.artwork) {
+    if (!CheckArtworkSrcSanity(artwork.src))

[dcheng, 2016/07/05 02:36:55]

CheckArtworkSanity already does this check.

[Zhiqiang Zhang (Slow), 2016/07/05 15:30:30]

This check is needed here.

If artwork has a good src but bad other fields (super long type/sizes), we will
do sanitize it. If artwork has a bad src, then we just abandon it.

+      continue;
+
+    sanitized_metadata.artwork.push_back(
+        CheckArtworkSanity(artwork) ? artwork : SanitizeArtwork(artwork));
+
+    if (sanitized_metadata.artwork.size() > kMaxNumberOfArtworkImages)

[dcheng, 2016/07/05 02:36:55]

This check should be ==: otherwise, you can create sanitized media metadata that
won't pass the sanity check.

[Zhiqiang Zhang (Slow), 2016/07/05 15:30:30]

Done.

+      break;
+  }
+
+  return sanitized_metadata;
+}
+
+}  // namespace content