Implement argument validation for chrome.cast.channel.{open,send}

chrome/browser/extensions/api/cast_channel/cast_channel_api.cc

 // Copyright 2013 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 "chrome/browser/extensions/api/cast_channel/cast_channel_api.h"
 
 #include <limits>
 
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 262 matching lines @@
   EXTENSION_FUNCTION_VALIDATE(params_.get());
   // The connect_info parameter may be a string URL like cast:// or casts:// or
   // a ConnectInfo object.
   std::string cast_url;
   switch (params_->connect_info->GetType()) {
     case base::Value::TYPE_STRING:
       CHECK(params_->connect_info->GetAsString(&cast_url));
       connect_info_.reset(new ConnectInfo);
       if (!ParseChannelUrl(GURL(cast_url), connect_info_.get())) {
         connect_info_.reset();
+        SetError("Invalid Cast URL " + cast_url);
+        return false;
       }
       break;
     case base::Value::TYPE_DICTIONARY:
       connect_info_ = ConnectInfo::FromValue(*(params_->connect_info));
       break;
     default:
       break;
   }
   if (connect_info_.get()) {
     channel_auth_ = connect_info_->auth;
     ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
-    return ip_endpoint_.get() != NULL;
+    if (!ip_endpoint_.get()) {
+      SetError("Invalid connect_info");
+    }
+  } else {
+    SetError("Invalid connect_info");
   }
-  return false;
+  return ip_endpoint_.get() != NULL;
 }
 
 void CastChannelOpenFunction::AsyncWorkStart() {
   DCHECK(api_);
   DCHECK(ip_endpoint_.get());
   scoped_ptr<CastSocket> socket = api_->CreateCastSocket(
       extension_->id(), *ip_endpoint_, channel_auth_);
   new_channel_id_ = AddSocket(socket.release());
   GetSocket(new_channel_id_)->Connect(
       base::Bind(&CastChannelOpenFunction::OnOpen, this));
 }
 
 void CastChannelOpenFunction::OnOpen(int result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   SetResultFromSocket(new_channel_id_);
   AsyncWorkCompleted();
 }
 
 CastChannelSendFunction::CastChannelSendFunction() { }
 
 CastChannelSendFunction::~CastChannelSendFunction() { }
 
 bool CastChannelSendFunction::Prepare() {
   params_ = Send::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
-  return true;
+  if (params_->message.namespace_.empty()) {
+    SetError("message_info.namespace_ is required");
+    return false;
+  }
+  if (params_->message.source_id.empty()) {
+    SetError("message_info.source_id is required");
+    return false;
+  }
+  if (params_->message.destination_id.empty()) {
+    SetError("message_info.destination_id is required");
+    return false;
+  }
+  switch (params_->message.data->GetType()) {
+    case base::Value::TYPE_STRING:
+    case base::Value::TYPE_BINARY:
+      return true;

[imcheng, 2014/04/24 22:13:18]

super nit: I'd prefer to see validation logic return true at the very end:

switch (params_->message.data->GetType()) {
  case base::Value::TYPE_STRING:
  case base::Value::TYPE_BINARY:
    break;
  default:
    SetError("Invalid type of message_info.data");
    return false;
}
return true;


Because that way it will be less error-prone to append other validation logic in
the future.

+    default:
+      break;
+  }
+  SetError("Invalid type of message_info.data");
+  return false;
 }
 
 void CastChannelSendFunction::AsyncWorkStart() {
   CastSocket* socket = GetSocketOrCompleteWithError(
       params_->channel.channel_id);
   if (socket)
     socket->SendMessage(params_->message,
                         base::Bind(&CastChannelSendFunction::OnSend, this));
 }
 
@@ skipping 31 matching lines @@
     SetResultFromError(cast_channel::CHANNEL_ERROR_SOCKET_ERROR);
   } else {
     int channel_id = params_->channel.channel_id;
     SetResultFromSocket(channel_id);
     RemoveSocket(channel_id);
   }
   AsyncWorkCompleted();
 }
 
 }  // namespace extensions