Implement MediaMetadata artwork in content

content/public/common/media_metadata.cc

Index: content/public/common/media_metadata.cc
diff --git a/content/public/common/media_metadata.cc b/content/public/common/media_metadata.cc
index dc3824c25b0cdc82b515387caec50f9ea356f6b7..82045653411b442e880ff9264fbf544bdc0e0cc5 100644
--- a/content/public/common/media_metadata.cc
+++ b/content/public/common/media_metadata.cc
@@ -4,20 +4,46 @@
 
 #include "content/public/common/media_metadata.h"
 
+#include <algorithm>
+
 namespace content {
 
 const size_t MediaMetadata::kMaxIPCStringLength = 4 * 1024;
+const int MediaMetadata::kMaxIconSize = 4 * 1024;
 
 MediaMetadata::MediaMetadata() = default;
 
 MediaMetadata::~MediaMetadata() = default;
 
 bool MediaMetadata::operator==(const MediaMetadata& other) const {
-  return title == other.title && artist == other.artist && album == other.album;
+  return title == other.title && artist == other.artist &&
+      album == other.album && artwork == other.artwork;
 }
 
 bool MediaMetadata::operator!=(const MediaMetadata& other) const {
   return !this->operator==(other);
 }
 
+base::Optional<MediaMetadata::Artwork> MediaMetadata::SanitizeArtwork(
+    const MediaMetadata::Artwork& artwork) {
+  if (!artwork.src.is_valid() || !artwork.src.IsStandard() ||
+      (!artwork.src.SchemeIsHTTPOrHTTPS() &&
+       !artwork.src.SchemeIsFile())) {

[palmer, 2016/06/20 21:34:21]

Is there any chance that allowing file: URLs here could enable media metadata
authors to induce the calling process to read a file that the calling process
can read but which the metadata author cannot?

And then, is there some way that the calling process might accidentally leak or
reveal information in/about that file to the metadata author in some way?

For example, what if web content could say that a media object's artwork is
file:///etc/passwd (or file:///data/app-private/...), and then the browser
process opened the file, and gave a descriptor or the raw bytes to the web
content?

If that scenario is plausible, we should not allow file: URLs, or at least not
all file URLs. If the scenario is implausible, can you explain what stops it
from happening? Thanks!

[Zhiqiang Zhang (Slow), 2016/06/21 11:08:42]

The image is fetched via WebContents::DownloadImage(), which basically passes
the URL to the renderer, and eventually the request is initiated in blink, where
regular security checks should apply I think?

+    return base::nullopt;
+  }
+  Artwork sanitized_artwork;
+  sanitized_artwork.src = artwork.src;
+  sanitized_artwork.type = artwork.type.is_null() ?
+      base::NullableString16() :
+      base::NullableString16(
+          artwork.type.string().substr(0, kMaxIPCStringLength),

[palmer, 2016/06/20 21:34:21]

Is the range of valid types really the set of strings containing 0 to 4096
arbitrary byte values?

[Zhiqiang Zhang (Slow), 2016/06/21 11:08:42]

Hmm, we actually allow any string here (see Manifest::Icon), but the size limit
for MIME types actually 127*2+1 (RFC 4288).

Fixed.

+          false);
+  std::copy_if(artwork.sizes.begin(), artwork.sizes.end(),
+               sanitized_artwork.sizes.begin(),
+               [](const gfx::Size& size) {
+                 return size.width() >= 0 && size.width() <= kMaxIconSize &&
+                     size.height() >= 0 && size.height() <= kMaxIconSize; });
+  return sanitized_artwork;
+}
+
 }  // namespace content