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

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

Index: chrome/browser/extensions/api/cast_channel/cast_channel_api.cc
diff --git a/chrome/browser/extensions/api/cast_channel/cast_channel_api.cc b/chrome/browser/extensions/api/cast_channel/cast_channel_api.cc
index d1a84e4a31261a8f61178ce6c93e854619311df5..90f159dd4c8c9d2dff60dda656a0387418538e48 100644
--- a/chrome/browser/extensions/api/cast_channel/cast_channel_api.cc
+++ b/chrome/browser/extensions/api/cast_channel/cast_channel_api.cc
@@ -244,7 +244,7 @@ bool CastChannelOpenFunction::ParseChannelUrl(const GURL& url,
     cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED :
     cast_channel::CHANNEL_AUTH_TYPE_SSL;
   return true;
-};
+}
 
 net::IPEndPoint* CastChannelOpenFunction::ParseConnectInfo(
     const ConnectInfo& connect_info) {
@@ -280,6 +280,8 @@ bool CastChannelOpenFunction::Prepare() {
       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:
@@ -288,12 +290,17 @@ bool CastChannelOpenFunction::Prepare() {
     default:
       break;
   }
-  if (connect_info_.get()) {
-    channel_auth_ = connect_info_->auth;
-    ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
-    return ip_endpoint_.get() != NULL;
+  if (!connect_info_.get()) {
+    SetError("Invalid connect_info");
+    return false;
+  }
+  channel_auth_ = connect_info_->auth;
+  ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
+  if (!ip_endpoint_.get()) {
+    SetError("Invalid connect_info");

[Wez, 2014/07/16 22:42:33]

nit: May be helpful to be able to distinguish this error (bad ConnectInfo
parameters) from the previous error (malformed ConnectInfo)?

It looks like ConnectInfo::FromValue() mostly just parses the fields, except for
the auth-type, which it enforces cannot be NONE; ParseConnectInfo() enforces
values on the fields (including auth-type) and then returns an IPEndpoint for
it. Can we break the latter into IsValidConnectInfo() and
IpEndpointFromConnectInfo(), and remove the auth-type check from FromValue, to
keep parsing, verifying and fetching fields well encapsulated?

[mark a. foltz, 2014/07/17 19:26:51]

Done.
Done, at the expense of parsing the IP address twice.

+    return false;
   }
-  return false;
+  return true;
 }
 
 void CastChannelOpenFunction::AsyncWorkStart() {
@@ -319,6 +326,26 @@ CastChannelSendFunction::~CastChannelSendFunction() { }
 bool CastChannelSendFunction::Prepare() {
   params_ = Send::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
+  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:
+      break;
+    default:
+      SetError("Invalid type of message_info.data");
+      return false;
+  }
   return true;
 }