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

Cleanup namespace usage in cast_channel_api.h.

TESTED=Browser test.  Manually with Cast extension
BUG=331165

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266804

Review URL: https://codereview.chromium.org/255443002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284200 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/api/cast_channel/cast_channel_api.h"
 
 #include <limits>
+#include <string>
 
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
@@ -60,6 +61,21 @@ void FillChannelInfo(const CastSocket& socket, ChannelInfo* channel_info) {
   channel_info->error_state = socket.error_state();
 }
 
+bool IsValidConnectInfoPort(const ConnectInfo& connect_info) {
+  return connect_info.port > 0 && connect_info.port <
+    std::numeric_limits<unsigned short>::max();
+}
+
+bool IsValidConnectInfoAuth(const ConnectInfo& connect_info) {
+  return connect_info.auth == cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED ||
+    connect_info.auth == cast_channel::CHANNEL_AUTH_TYPE_SSL;
+}
+
+bool IsValidConnectInfoIpAddress(const ConnectInfo& connect_info) {
+  net::IPAddressNumber ip_address;
+  return net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address);
+}
+
 }  // namespace
 
 CastChannelAPI::CastChannelAPI(content::BrowserContext* context)
@@ -244,22 +260,12 @@ 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) {
   net::IPAddressNumber ip_address;
-  if (!net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address)) {
-    return NULL;
-  }
-  if (connect_info.port < 0 || connect_info.port >
-      std::numeric_limits<unsigned short>::max()) {
-    return NULL;
-  }
-  if (connect_info.auth != cast_channel::CHANNEL_AUTH_TYPE_SSL_VERIFIED &&
-      connect_info.auth != cast_channel::CHANNEL_AUTH_TYPE_SSL) {
-    return NULL;
-  }
+  CHECK(net::ParseIPLiteralToNumber(connect_info.ip_address, &ip_address));
   return new net::IPEndPoint(ip_address, connect_info.port);
 }
 
@@ -280,20 +286,35 @@ bool CastChannelOpenFunction::Prepare() {
       connect_info_.reset(new ConnectInfo);
       if (!ParseChannelUrl(GURL(cast_url), connect_info_.get())) {
         connect_info_.reset();
+        SetError("Invalid connect_info (invalid Cast URL " + cast_url + ")");
       }
       break;
     case base::Value::TYPE_DICTIONARY:
       connect_info_ = ConnectInfo::FromValue(*(params_->connect_info));
+      if (!connect_info_.get()) {
+        SetError("connect_info.auth is required");
+      }
       break;
     default:
+      SetError("Invalid connect_info (unknown type)");
       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()) {
+    return false;
+  }
+  if (!IsValidConnectInfoPort(*connect_info_)) {
+    SetError("Invalid connect_info (invalid port)");
+  } else if (!IsValidConnectInfoAuth(*connect_info_)) {
+    SetError("Invalid connect_info (invalid auth)");
+  } else if (!IsValidConnectInfoIpAddress(*connect_info_)) {
+    SetError("Invalid connect_info (invalid IP address)");
   }
-  return false;
+  if (!GetError().empty()) {
+    return false;
+  }
+  channel_auth_ = connect_info_->auth;
+  ip_endpoint_.reset(ParseConnectInfo(*connect_info_));
+  return true;
 }
 
 void CastChannelOpenFunction::AsyncWorkStart() {
@@ -319,6 +340,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;
 }
 

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

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_CAST_CHANNEL_CAST_CHANNEL_API_H_
 #define CHROME_BROWSER_EXTENSIONS_API_CAST_CHANNEL_CAST_CHANNEL_API_H_
 
+#include <string>
+
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/threading/thread_checker.h"
@@ -31,7 +33,6 @@ namespace cast_channel = api::cast_channel;
 
 class CastChannelAPI : public BrowserContextKeyedAPI,
                        public cast_channel::CastSocket::Delegate {
-
  public:
   explicit CastChannelAPI(content::BrowserContext* context);
 
@@ -137,12 +138,12 @@ class CastChannelOpenFunction : public CastChannelAsyncApiFunction {
   // corresponding details, and returns true. Returns false if |url| is not a
   // valid Cast URL.
   static bool ParseChannelUrl(const GURL& url,
-                              api::cast_channel::ConnectInfo* connect_info);
+                              cast_channel::ConnectInfo* connect_info);
 
   // Validates that |connect_info| represents a valid IP end point and returns a
   // new IPEndPoint if so.  Otherwise returns NULL.
   static net::IPEndPoint* ParseConnectInfo(
-      const api::cast_channel::ConnectInfo& connect_info);
+      const cast_channel::ConnectInfo& connect_info);
 
   void OnOpen(int result);
 
@@ -150,9 +151,9 @@ class CastChannelOpenFunction : public CastChannelAsyncApiFunction {
   // The id of the newly opened socket.
   int new_channel_id_;
   CastChannelAPI* api_;
-  scoped_ptr<api::cast_channel::ConnectInfo> connect_info_;
+  scoped_ptr<cast_channel::ConnectInfo> connect_info_;
   scoped_ptr<net::IPEndPoint> ip_endpoint_;
-  api::cast_channel::ChannelAuthType channel_auth_;
+  cast_channel::ChannelAuthType channel_auth_;
 
   FRIEND_TEST_ALL_PREFIXES(CastChannelOpenFunctionTest, TestParseChannelUrl);
   FRIEND_TEST_ALL_PREFIXES(CastChannelOpenFunctionTest, TestParseConnectInfo);

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

@@ -46,7 +46,7 @@ TEST(CastChannelOpenFunctionTest, TestParseChannelUrl) {
   EXPECT_FALSE(ccof::ParseChannelUrl(GURL("cast://192.0.0.1:"), &connect_info));
 }
 
-// Tests validation of ConnectInfo.
+// Tests parsing of ConnectInfo.
 TEST(CastChannelOpenFunctionTest, TestParseConnectInfo) {
   typedef CastChannelOpenFunction ccof;
   scoped_ptr<net::IPEndPoint> ip_endpoint;
@@ -60,30 +60,6 @@ TEST(CastChannelOpenFunctionTest, TestParseConnectInfo) {
   ip_endpoint.reset(ccof::ParseConnectInfo(connect_info));
   EXPECT_TRUE(ip_endpoint.get() != NULL);
   EXPECT_EQ(ip_endpoint->ToString(), "192.0.0.1:8009");
-
-  // Invalid IP
-  ConnectInfo invalid_ip_connect_info;
-  invalid_ip_connect_info.ip_address = "blargh";
-  invalid_ip_connect_info.port = 8009;
-  invalid_ip_connect_info.auth = CHANNEL_AUTH_TYPE_SSL;
-  ip_endpoint.reset(ccof::ParseConnectInfo(invalid_ip_connect_info));
-  EXPECT_TRUE(ip_endpoint.get() == NULL);
-
-  // Invalid port
-  ConnectInfo invalid_port_connect_info;
-  invalid_port_connect_info.ip_address = "192.0.0.1";
-  invalid_port_connect_info.port = -1;
-  invalid_port_connect_info.auth = CHANNEL_AUTH_TYPE_SSL;
-  ip_endpoint.reset(ccof::ParseConnectInfo(invalid_port_connect_info));
-  EXPECT_TRUE(ip_endpoint.get() == NULL);
-
-  // Invalid auth
-  ConnectInfo invalid_auth_connect_info;
-  invalid_auth_connect_info.ip_address = "192.0.0.1";
-  invalid_auth_connect_info.port = 8009;
-  invalid_auth_connect_info.auth = CHANNEL_AUTH_TYPE_NONE;
-  ip_endpoint.reset(ccof::ParseConnectInfo(invalid_auth_connect_info));
-  EXPECT_TRUE(ip_endpoint.get() == NULL);
 }
 
 }  // namespace cast_channel

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

@@ -8,7 +8,9 @@
 #include "chrome/browser/extensions/api/cast_channel/cast_channel_api.h"
 #include "chrome/browser/extensions/api/cast_channel/cast_socket.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/common/extensions/api/cast_channel.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/common/switches.h"
@@ -23,6 +25,9 @@ using cast_channel::CastSocket;
 using cast_channel::ChannelError;
 using cast_channel::MessageInfo;
 using cast_channel::ReadyState;
+using extensions::Extension;
+
+namespace utils = extension_function_test_utils;
 
 using ::testing::_;
 using ::testing::A;
@@ -130,6 +135,22 @@ class CastChannelAPITest : public ExtensionApiTest {
     api->OnMessage(cast_socket, message_info);
   }
 
+  extensions::CastChannelOpenFunction* CreateOpenFunction(
+        scoped_refptr<Extension> extension) {
+    extensions::CastChannelOpenFunction* cast_channel_open_function =
+      new extensions::CastChannelOpenFunction;
+    cast_channel_open_function->set_extension(extension.get());
+    return cast_channel_open_function;
+  }
+
+  extensions::CastChannelSendFunction* CreateSendFunction(
+        scoped_refptr<Extension> extension) {
+    extensions::CastChannelSendFunction* cast_channel_send_function =
+      new extensions::CastChannelSendFunction;
+    cast_channel_send_function->set_extension(extension.get());
+    return cast_channel_send_function;
+  }
+
   MockCastSocket* mock_cast_socket_;
   net::CapturingNetLog capturing_net_log_;
 };
@@ -259,3 +280,103 @@ IN_PROC_BROWSER_TEST_F(CastChannelAPITest, MAYBE_TestOpenError) {
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
+IN_PROC_BROWSER_TEST_F(CastChannelAPITest, TestOpenInvalidConnectInfo) {
+  scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension());
+  scoped_refptr<extensions::CastChannelOpenFunction> cast_channel_open_function;
+
+  // Invalid URL
+  // TODO(mfoltz): Remove this test case when fixing crbug.com/331905
+  cast_channel_open_function = CreateOpenFunction(empty_extension);
+  std::string error(utils::RunFunctionAndReturnError(
+      cast_channel_open_function.get(), "[\"blargh\"]", browser()));
+  EXPECT_EQ(error, "Invalid connect_info (invalid Cast URL blargh)");
+
+  // Wrong type
+  // TODO(mfoltz): Remove this test case when fixing crbug.com/331905
+  cast_channel_open_function = CreateOpenFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_open_function.get(),
+      "[123]", browser());
+  EXPECT_EQ(error, "Invalid connect_info (unknown type)");
+
+  // Invalid IP address
+  cast_channel_open_function = CreateOpenFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_open_function.get(),
+      "[{\"ipAddress\": \"invalid_ip\", \"port\": 8009, \"auth\": \"ssl\"}]",
+      browser());
+  EXPECT_EQ(error, "Invalid connect_info (invalid IP address)");
+
+  // Invalid port
+  cast_channel_open_function = CreateOpenFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_open_function.get(),
+      "[{\"ipAddress\": \"127.0.0.1\", \"port\": -200, \"auth\": \"ssl\"}]",
+      browser());
+  EXPECT_EQ(error, "Invalid connect_info (invalid port)");
+
+  // Auth not set
+  cast_channel_open_function = CreateOpenFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_open_function.get(),
+      "[{\"ipAddress\": \"127.0.0.1\", \"port\": 8009}]",
+      browser());
+  EXPECT_EQ(error, "connect_info.auth is required");
+}
+
+IN_PROC_BROWSER_TEST_F(CastChannelAPITest, TestSendInvalidMessageInfo) {
+  scoped_refptr<Extension> empty_extension(utils::CreateEmptyExtension());
+  scoped_refptr<extensions::CastChannelSendFunction> cast_channel_send_function;
+
+  // Numbers are not supported
+  cast_channel_send_function = CreateSendFunction(empty_extension);
+  std::string error(utils::RunFunctionAndReturnError(
+      cast_channel_send_function.get(),
+      "[{\"channelId\": 1, \"url\": \"cast://127.0.0.1:8009\", "
+      "\"connectInfo\": "
+      "{\"ipAddress\": \"127.0.0.1\", \"port\": 8009, "
+      "\"auth\": \"ssl\"}, \"readyState\": \"open\"}, "
+      "{\"namespace_\": \"foo\", \"sourceId\": \"src\", "
+      "\"destinationId\": \"dest\", \"data\": 1235}]",
+      browser()));
+  EXPECT_EQ(error, "Invalid type of message_info.data");
+
+  // Missing namespace_
+  cast_channel_send_function = CreateSendFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_send_function.get(),
+      "[{\"channelId\": 1, \"url\": \"cast://127.0.0.1:8009\", "
+      "\"connectInfo\": "
+      "{\"ipAddress\": \"127.0.0.1\", \"port\": 8009, "
+      "\"auth\": \"ssl\"}, \"readyState\": \"open\"}, "
+      "{\"namespace_\": \"\", \"sourceId\": \"src\", "
+      "\"destinationId\": \"dest\", \"data\": \"data\"}]",
+      browser());
+  EXPECT_EQ(error, "message_info.namespace_ is required");
+
+  // Missing source_id
+  cast_channel_send_function = CreateSendFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_send_function.get(),
+      "[{\"channelId\": 1, \"url\": \"cast://127.0.0.1:8009\", "
+      "\"connectInfo\": "
+      "{\"ipAddress\": \"127.0.0.1\", \"port\": 8009, "
+      "\"auth\": \"ssl\"}, \"readyState\": \"open\"}, "
+      "{\"namespace_\": \"foo\", \"sourceId\": \"\", "
+      "\"destinationId\": \"dest\", \"data\": \"data\"}]",
+      browser());
+  EXPECT_EQ(error, "message_info.source_id is required");
+
+  // Missing destination_id
+  cast_channel_send_function = CreateSendFunction(empty_extension);
+  error = utils::RunFunctionAndReturnError(
+      cast_channel_send_function.get(),
+      "[{\"channelId\": 1, \"url\": \"cast://127.0.0.1:8009\", "
+      "\"connectInfo\": "
+      "{\"ipAddress\": \"127.0.0.1\", \"port\": 8009, "
+      "\"auth\": \"ssl\"}, \"readyState\": \"open\"}, "
+      "{\"namespace_\": \"foo\", \"sourceId\": \"src\", "
+      "\"destinationId\": \"\", \"data\": \"data\"}]",
+      browser());
+  EXPECT_EQ(error, "message_info.destination_id is required");
+}