Title: [234603] trunk/Source/WebCore
Revision
234603
Author
ctur...@igalia.com
Date
2018-08-06 09:47:21 -0700 (Mon, 06 Aug 2018)

Log Message

Return extracted key ids as an optional
https://bugs.webkit.org/show_bug.cgi?id=188303

Reviewed by Darin Adler.

An empty list of extracted key ids was being considered a failure
case before this patch. In the PSSH boxes from the CENC standard,
it's not uncommon for the box to be version 0, meaning it has no
embedded key ids, so the case when there's an empty list should
not be treated as an error. Given this, the interface should be
more general and allow for a sentinel value indicating a parsing
error rather than an absence of key ids.

Covered by existing tests.

* Modules/encryptedmedia/InitDataRegistry.cpp:
(WebCore::extractKeyIDsKeyids): Change return type to be wrapped
in an optional, and make parsing errors return a nullopt rather
than an empty vector.
(WebCore::sanitizeKeyids): Use the new optional interface, return
a null RefPtr in the case of a parsing error, this method may now
return an empty vector.
(WebCore::extractKeyIDsCenc): Not implemented, so return an error
value rather than an empty vector.
(WebCore::extractKeyIDsWebM): Ditto.
(WebCore::InitDataRegistry::extractKeyIDs): Ditto.
* Modules/encryptedmedia/InitDataRegistry.h: Update the interface
to use an optional return type.
* platform/graphics/avfoundation/CDMFairPlayStreaming.cpp:
(WebCore::CDMPrivateFairPlayStreaming::extractKeyIDsSinf): Update
to use the new interface.
(WebCore::CDMPrivateFairPlayStreaming::extractKeyIDsSkd): Ditto.
* platform/graphics/avfoundation/CDMFairPlayStreaming.h: Ditto.
* platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:
(WebCore::CDMInstanceFairPlayStreamingAVFObjC::keyIDs): Convert
the optional value into a vector, since it is assumed you can not
have an empty vector of key ids in the init datas FPS supports.
* testing/MockCDMFactory.cpp:
(WebCore::MockCDMInstance::requestLicense): Only return an error
if there really was a parsing error, rather than the case of there
being zero key ids in the init data payload.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (234602 => 234603)


--- trunk/Source/WebCore/ChangeLog	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/ChangeLog	2018-08-06 16:47:21 UTC (rev 234603)
@@ -1,3 +1,47 @@
+2018-08-06  Charlie Turner  <ctur...@igalia.com>
+
+        Return extracted key ids as an optional
+        https://bugs.webkit.org/show_bug.cgi?id=188303
+
+        Reviewed by Darin Adler.
+
+        An empty list of extracted key ids was being considered a failure
+        case before this patch. In the PSSH boxes from the CENC standard,
+        it's not uncommon for the box to be version 0, meaning it has no
+        embedded key ids, so the case when there's an empty list should
+        not be treated as an error. Given this, the interface should be
+        more general and allow for a sentinel value indicating a parsing
+        error rather than an absence of key ids.
+
+        Covered by existing tests.
+
+        * Modules/encryptedmedia/InitDataRegistry.cpp:
+        (WebCore::extractKeyIDsKeyids): Change return type to be wrapped
+        in an optional, and make parsing errors return a nullopt rather
+        than an empty vector.
+        (WebCore::sanitizeKeyids): Use the new optional interface, return
+        a null RefPtr in the case of a parsing error, this method may now
+        return an empty vector.
+        (WebCore::extractKeyIDsCenc): Not implemented, so return an error
+        value rather than an empty vector.
+        (WebCore::extractKeyIDsWebM): Ditto.
+        (WebCore::InitDataRegistry::extractKeyIDs): Ditto.
+        * Modules/encryptedmedia/InitDataRegistry.h: Update the interface
+        to use an optional return type.
+        * platform/graphics/avfoundation/CDMFairPlayStreaming.cpp:
+        (WebCore::CDMPrivateFairPlayStreaming::extractKeyIDsSinf): Update
+        to use the new interface.
+        (WebCore::CDMPrivateFairPlayStreaming::extractKeyIDsSkd): Ditto.
+        * platform/graphics/avfoundation/CDMFairPlayStreaming.h: Ditto.
+        * platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:
+        (WebCore::CDMInstanceFairPlayStreamingAVFObjC::keyIDs): Convert
+        the optional value into a vector, since it is assumed you can not
+        have an empty vector of key ids in the init datas FPS supports.
+        * testing/MockCDMFactory.cpp:
+        (WebCore::MockCDMInstance::requestLicense): Only return an error
+        if there really was a parsing error, rather than the case of there
+        being zero key ids in the init data payload.
+
 2018-08-06  Frederic Wang  <fw...@igalia.com>
 
         Make two-arguments versions of scrollBy/scrollTo depend on the one-argument versions

Modified: trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp (234602 => 234603)


--- trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp	2018-08-06 16:47:21 UTC (rev 234603)
@@ -37,25 +37,25 @@
 
 namespace WebCore {
 
-static Vector<Ref<SharedBuffer>> extractKeyIDsKeyids(const SharedBuffer& buffer)
+static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsKeyids(const SharedBuffer& buffer)
 {
     // 1. Format
     // https://w3c.github.io/encrypted-media/format-registry/initdata/keyids.html#format
     if (buffer.size() > std::numeric_limits<unsigned>::max())
-        return { };
+        return std::nullopt;
     String json { buffer.data(), static_cast<unsigned>(buffer.size()) };
 
     RefPtr<JSON::Value> value;
     if (!JSON::Value::parseJSON(json, value))
-        return { };
+        return std::nullopt;
 
     RefPtr<JSON::Object> object;
     if (!value->asObject(object))
-        return { };
+        return std::nullopt;
 
     RefPtr<JSON::Array> kidsArray;
     if (!object->getArray("kids", kidsArray))
-        return { };
+        return std::nullopt;
 
     Vector<Ref<SharedBuffer>> keyIDs;
     for (auto& value : *kidsArray) {
@@ -78,13 +78,13 @@
 {
     // 1. Format
     // https://w3c.github.io/encrypted-media/format-registry/initdata/keyids.html#format
-    Vector<Ref<SharedBuffer>> keyIDBuffer = extractKeyIDsKeyids(buffer);
-    if (keyIDBuffer.isEmpty())
+    auto keyIDBuffer = extractKeyIDsKeyids(buffer);
+    if (!keyIDBuffer)
         return nullptr;
 
     auto object = JSON::Object::create();
     auto kidsArray = JSON::Array::create();
-    for (auto& buffer : keyIDBuffer)
+    for (auto& buffer : keyIDBuffer.value())
         kidsArray->pushString(WTF::base64URLEncode(buffer->data(), buffer->size()));
     object->setArray("kids", WTFMove(kidsArray));
 
@@ -100,12 +100,12 @@
     return buffer.copy();
 }
 
-static Vector<Ref<SharedBuffer>> extractKeyIDsCenc(const SharedBuffer&)
+static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsCenc(const SharedBuffer&)
 {
     // 4. Common SystemID and PSSH Box Format
     // https://w3c.github.io/encrypted-media/format-registry/initdata/cenc.html#common-system
     notImplemented();
-    return { };
+    return std::nullopt;
 }
 
 static RefPtr<SharedBuffer> sanitizeWebM(const SharedBuffer& buffer)
@@ -116,12 +116,12 @@
     return buffer.copy();
 }
 
-static Vector<Ref<SharedBuffer>> extractKeyIDsWebM(const SharedBuffer&)
+static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsWebM(const SharedBuffer&)
 {
     // 1. Format
     // https://w3c.github.io/encrypted-media/format-registry/initdata/webm.html#format
     notImplemented();
-    return { };
+    return std::nullopt;
 }
 
 InitDataRegistry& InitDataRegistry::shared()
@@ -147,11 +147,11 @@
     return iter->value.sanitizeInitData(buffer);
 }
 
-Vector<Ref<SharedBuffer>> InitDataRegistry::extractKeyIDs(const AtomicString& initDataType, const SharedBuffer& buffer)
+std::optional<Vector<Ref<SharedBuffer>>> InitDataRegistry::extractKeyIDs(const AtomicString& initDataType, const SharedBuffer& buffer)
 {
     auto iter = m_types.find(initDataType);
     if (iter == m_types.end() || !iter->value.sanitizeInitData)
-        return { };
+        return std::nullopt;
     return iter->value.extractKeyIDs(buffer);
 }
 

Modified: trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.h (234602 => 234603)


--- trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.h	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.h	2018-08-06 16:47:21 UTC (rev 234603)
@@ -29,6 +29,7 @@
 
 #include <wtf/Function.h>
 #include <wtf/HashMap.h>
+#include <wtf/Optional.h>
 #include <wtf/Ref.h>
 #include <wtf/RefPtr.h>
 #include <wtf/Vector.h>
@@ -45,11 +46,11 @@
     friend class NeverDestroyed<InitDataRegistry>;
 
     RefPtr<SharedBuffer> sanitizeInitData(const AtomicString& initDataType, const SharedBuffer&);
-    WEBCORE_EXPORT Vector<Ref<SharedBuffer>> extractKeyIDs(const AtomicString& initDataType, const SharedBuffer&);
+    WEBCORE_EXPORT std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDs(const AtomicString& initDataType, const SharedBuffer&);
 
     struct InitDataTypeCallbacks {
         using SanitizeInitDataCallback = Function<RefPtr<SharedBuffer>(const SharedBuffer&)>;
-        using ExtractKeyIDsCallback = Function<Vector<Ref<SharedBuffer>>(const SharedBuffer&)>;
+        using ExtractKeyIDsCallback = Function<std::optional<Vector<Ref<SharedBuffer>>>(const SharedBuffer&)>;
 
         SanitizeInitDataCallback sanitizeInitData;
         ExtractKeyIDsCallback extractKeyIDs;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.cpp (234602 => 234603)


--- trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.cpp	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.cpp	2018-08-06 16:47:21 UTC (rev 234603)
@@ -158,9 +158,8 @@
     return result;
 }
 
-Vector<Ref<SharedBuffer>> CDMPrivateFairPlayStreaming::extractKeyIDsSinf(const SharedBuffer& buffer)
+std::optional<Vector<Ref<SharedBuffer>>> CDMPrivateFairPlayStreaming::extractKeyIDsSinf(const SharedBuffer& buffer)
 {
-
     Vector<Ref<SharedBuffer>> keyIDs;
     auto results = extractSchemeAndKeyIdFromSinf(buffer);
 
@@ -187,7 +186,7 @@
     return buffer.copy();
 }
 
-Vector<Ref<SharedBuffer>> CDMPrivateFairPlayStreaming::extractKeyIDsSkd(const SharedBuffer& buffer)
+std::optional<Vector<Ref<SharedBuffer>>> CDMPrivateFairPlayStreaming::extractKeyIDsSkd(const SharedBuffer& buffer)
 {
     // In the 'skd' scheme, the init data is the key ID.
     Vector<Ref<SharedBuffer>> keyIDs;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.h (234602 => 234603)


--- trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.h	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/CDMFairPlayStreaming.h	2018-08-06 16:47:21 UTC (rev 234603)
@@ -68,11 +68,11 @@
     std::optional<String> sanitizeSessionId(const String&) const override;
 
     static const AtomicString& sinfName();
-    static Vector<Ref<SharedBuffer>> extractKeyIDsSinf(const SharedBuffer&);
+    static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsSinf(const SharedBuffer&);
     static RefPtr<SharedBuffer> sanitizeSinf(const SharedBuffer&);
 
     static const AtomicString& skdName();
-    static Vector<Ref<SharedBuffer>> extractKeyIDsSkd(const SharedBuffer&);
+    static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsSkd(const SharedBuffer&);
     static RefPtr<SharedBuffer> sanitizeSkd(const SharedBuffer&);
 };
 

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm (234602 => 234603)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm	2018-08-06 16:47:21 UTC (rev 234603)
@@ -241,8 +241,12 @@
         return Vector<Ref<SharedBuffer>>::from(SharedBuffer::create([(NSString *)m_request.get().identifier dataUsingEncoding:NSUTF8StringEncoding]));
     if ([m_request.get().identifier isKindOfClass:[NSData class]])
         return Vector<Ref<SharedBuffer>>::from(SharedBuffer::create((NSData *)m_request.get().identifier));
-    if (m_request.get().initializationData)
-        return CDMPrivateFairPlayStreaming::extractKeyIDsSinf(SharedBuffer::create(m_request.get().initializationData));
+    if (m_request.get().initializationData) {
+        auto sinfKeyIDs = CDMPrivateFairPlayStreaming::extractKeyIDsSinf(SharedBuffer::create(m_request.get().initializationData));
+        if (!sinfKeyIDs)
+            return { };
+        return WTFMove(sinfKeyIDs.value());
+    }
     return { };
 }
 

Modified: trunk/Source/WebCore/testing/MockCDMFactory.cpp (234602 => 234603)


--- trunk/Source/WebCore/testing/MockCDMFactory.cpp	2018-08-06 16:13:19 UTC (rev 234602)
+++ trunk/Source/WebCore/testing/MockCDMFactory.cpp	2018-08-06 16:47:21 UTC (rev 234603)
@@ -283,13 +283,13 @@
     }
 
     auto keyIDs = InitDataRegistry::shared().extractKeyIDs(initDataType, initData);
-    if (keyIDs.isEmpty()) {
+    if (!keyIDs || keyIDs.value().isEmpty()) {
         callback(SharedBuffer::create(), emptyString(), false, SuccessValue::Failed);
         return;
     }
 
     String sessionID = createCanonicalUUIDString();
-    factory->addKeysToSessionWithID(sessionID, WTFMove(keyIDs));
+    factory->addKeysToSessionWithID(sessionID, WTFMove(keyIDs.value()));
 
     CString license { "license" };
     callback(SharedBuffer::create(license.data(), license.length()), sessionID, false, SuccessValue::Succeeded);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to