Encrypted Media: Handle empty init_data in AesDecryptor::GenerateKeyRequest().

media/crypto/aes_decryptor.cc

 // Copyright (c) 2012 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 "media/crypto/aes_decryptor.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ skipping 121 matching lines @@
 AesDecryptor::~AesDecryptor() {
   STLDeleteValues(&key_map_);
 }
 
 bool AesDecryptor::GenerateKeyRequest(const std::string& key_system,
                                       const std::string& type,
                                       const uint8* init_data,
                                       int init_data_length) {
   std::string session_id_string(base::UintToString(next_session_id_++));
 
+  if (!init_data || !init_data_length) {
+    DVLOG(1) << "Could not generate key request with empty init_data.";

[ddorwin, 2012/11/29 17:55:08]

nit: How about "init_data required to generate a key request." This tells the
reader what they need to do rather than that the code could not do something.

[xhwang, 2012/11/29 20:34:37]

Done.

+    client_->KeyError(key_system, "", Decryptor::kUnknownError, 0);
+    return false;

[ddorwin, 2012/11/29 17:55:08]

Is false going to cause some error to be reported synchronously? What about in
external Clear Key?

[xhwang, 2012/11/29 20:34:37]

We discussed offline that decryptors other than ProxyDecryptor should not return
"false" on GKR(). I looked the code more closely and found returning "false" IS
desired in some cases. For example, in PpapiDecryptor, after it returns false,
it'll assume AddKey() will never be called:
http://code.google.com/searchframe#OAMlx_jo-ck/src/webkit/media/crypto/ppapi_...
If we return true here, we'll have to check |plugin_cdm_delegate_| in a lot of
places (e.g. in AddKey()).

So my gut feeling is that for now we should return MediaKeyException as possible
as we can, and fire KeyError only if we can't return MediaKeyException. (Yes, we
shouldn't have both of then returned in one call.) WDYT?

Ultimately we need to solve this nicely. How about making
PPP::GenerateKeyRequest() a sync message?

[ddorwin, 2012/11/29 22:19:01]

No, I don't think we should do that. Let's file a bug to ensure behavior is the
same in internal and external Clear Key and fix it in the OO API.

Does the user of AesDecryptor ensure that an error is reported (cdm_wrapper in
the external case)?

+  }
+
   // For now, the AesDecryptor does not care about |key_system| and |type|;
   // just fire the event with the |init_data| as the request.
   int message_length = init_data_length;
   scoped_array<uint8> message(new uint8[message_length]);
   memcpy(message.get(), init_data, message_length);
 
   client_->KeyMessage(key_system, session_id_string,
                       message.Pass(), message_length, "");
   return true;
 }
@@ skipping 170 matching lines @@
 bool AesDecryptor::DecryptionKey::Init() {
   CHECK(!secret_.empty());
   decryption_key_.reset(crypto::SymmetricKey::Import(
       crypto::SymmetricKey::AES, secret_));
   if (!decryption_key_.get())
     return false;
   return true;
 }
 
 }  // namespace media