Support socket endpoint permissions for AppsV2 Socket API.

chrome/browser/extensions/api/socket/socket_api.cc

Index: chrome/browser/extensions/api/socket/socket_api.cc
diff --git a/chrome/browser/extensions/api/socket/socket_api.cc b/chrome/browser/extensions/api/socket/socket_api.cc
index 951f60005d772158d1fd1507a95b2453c27103ae..3a25de763ffd5b43c8f08fc3416543d909f01585 100644
--- a/chrome/browser/extensions/api/socket/socket_api.cc
+++ b/chrome/browser/extensions/api/socket/socket_api.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/api/socket/socket_api.h"
 
 #include "base/bind.h"
+#include "chrome/common/extensions/permissions/socket_permission.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/dns/host_resolver_wrapper.h"
 #include "chrome/browser/extensions/api/socket/socket.h"
@@ -28,10 +29,12 @@ const char kResultCodeKey[] = "resultCode";
 const char kSocketIdKey[] = "socketId";
 const char kTCPOption[] = "tcp";
 const char kUDPOption[] = "udp";
+const char kUnknown[] = "unknown";
 
 const char kSocketNotFoundError[] = "Socket not found";
 const char kSocketTypeInvalidError[] = "Socket type is not supported";
 const char kDnsLookupFailedError[] = "DNS resolution failed";
+const char kPermissionError[] = "Caller does not have permission";
 
 SocketAsyncApiFunction::SocketAsyncApiFunction()
     : manager_(NULL) {
@@ -163,6 +166,36 @@ bool SocketConnectFunction::Prepare() {
 }
 
 void SocketConnectFunction::AsyncWorkStart() {
+  socket_ = manager_->Get(socket_id_);
+  if (!socket_) {
+    error_ = kSocketNotFoundError;
+    SetResult(Value::CreateIntegerValue(-1));
+    AsyncWorkCompleted();
+    return;
+  }
+
+  SocketPermissionData::OperationType type;

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

Can you call this variable operation_type? Otherwise the name is too generic,
and risks getting confused with the socket type.

[Peng, 2012/08/13 16:26:10]

Done.

+  switch (socket_->socket_type()) {
+    case Socket::TYPE_TCP:
+      type = SocketPermissionData::TCP_CONNECT;
+      break;
+    case Socket::TYPE_UDP:
+      type = SocketPermissionData::UDP_SEND_TO;
+      break;
+    default:
+      type = SocketPermissionData::NONE;

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

NOTREACHED() may be more appropriate here (I think we always want to map to a
operation type).

[Peng, 2012/08/13 16:26:10]

Done.

+      break;
+  }
+
+  SocketPermission::CheckParam param(type, hostname_, port_);
+  if (!GetExtension()->CheckAPIPermissionWithDetail(APIPermission::kSocket,
+        &param)) {
+    error_ = kPermissionError;
+    SetResult(Value::CreateIntegerValue(-1));
+    AsyncWorkCompleted();
+    return;
+  }
+
   StartDnsLookup(hostname_);
 }
 
@@ -176,15 +209,8 @@ void SocketConnectFunction::AfterDnsLookup(int lookup_result) {
 }
 
 void SocketConnectFunction::StartConnect() {
-  Socket* socket = manager_->Get(socket_id_);
-  if (!socket) {
-    error_ = kSocketNotFoundError;
-    OnConnect(-1);
-    return;
-  }
-
-  socket->Connect(resolved_address_, port_,
-                  base::Bind(&SocketConnectFunction::OnConnect, this));
+  socket_->Connect(resolved_address_, port_,
+                   base::Bind(&SocketConnectFunction::OnConnect, this));
 }
 
 void SocketConnectFunction::OnConnect(int result) {
@@ -216,10 +242,17 @@ bool SocketBindFunction::Prepare() {
 void SocketBindFunction::Work() {
   int result = -1;
   Socket* socket = manager_->Get(socket_id_);
-  if (socket)
-    result = socket->Bind(address_, port_);
-  else
+  SocketPermission::CheckParam param(
+      SocketPermissionData::UDP_BIND, address_, port_);

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

TCP sockets also implement the Bind method. Can you either check at the start of
the function that the socket type is UDP, or add support for a TCP_BIND
operation?

[Peng, 2012/08/13 16:26:10]

Done.

+  if (socket) {
+    if (GetExtension()->CheckAPIPermissionWithDetail(APIPermission::kSocket,
+          &param))
+      result = socket->Bind(address_, port_);
+    else
+      error_ = kPermissionError;
+  } else {
     error_ = kSocketNotFoundError;
+  }
 
   SetResult(Value::CreateIntegerValue(result));
 }
@@ -373,7 +406,26 @@ bool SocketSendToFunction::Prepare() {
 }
 
 void SocketSendToFunction::AsyncWorkStart() {
-  StartDnsLookup(hostname_);
+  do {
+    socket_ = manager_->Get(socket_id_);
+    if (!socket_) {
+      error_ = kSocketNotFoundError;
+      break;
+    }
+
+    SocketPermission::CheckParam param(SocketPermissionData::UDP_SEND_TO,

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

Same thing here.

[Peng, 2012/08/13 16:26:10]

Done.

+        hostname_, port_);
+    if (!GetExtension()->CheckAPIPermissionWithDetail(APIPermission::kSocket,
+          &param)) {
+      error_ = kPermissionError;
+      break;
+    }
+
+    StartDnsLookup(hostname_);
+    return;
+  } while (false);

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

I found the break/while(false) pattern hard to read (it's effectively a goto).
Can you use the same early return flow as in
SocketConnectFunction::AsyncWorkStart(), even if it's a bit more repetitive?

[Peng, 2012/08/13 16:26:10]

Done.

[Peng, 2012/08/13 16:26:10]

Done.

+  SetResult(Value::CreateIntegerValue(-1));
+  AsyncWorkCompleted();
 }
 
 void SocketSendToFunction::AfterDnsLookup(int lookup_result) {
@@ -386,15 +438,8 @@ void SocketSendToFunction::AfterDnsLookup(int lookup_result) {
 }
 
 void SocketSendToFunction::StartSendTo() {
-  Socket* socket = manager_->Get(socket_id_);
-  if (!socket) {
-    error_ = kSocketNotFoundError;
-    OnCompleted(-1);
-    return;
-  }
-
-  socket->SendTo(io_buffer_, io_buffer_size_, resolved_address_, port_,
-                 base::Bind(&SocketSendToFunction::OnCompleted, this));
+  socket_->SendTo(io_buffer_, io_buffer_size_, resolved_address_, port_,
+                  base::Bind(&SocketSendToFunction::OnCompleted, this));
 }
 
 void SocketSendToFunction::OnCompleted(int bytes_written) {
@@ -470,7 +515,17 @@ void SocketGetInfoFunction::Work() {
   if (socket) {
     // This represents what we know about the socket, and does not call through
     // to the system.
-    info.socket_type = (socket->IsTCPSocket() ? kTCPOption : kUDPOption);
+    switch (socket->socket_type()) {
+      case Socket::TYPE_TCP:
+        info.socket_type = kTCPOption;
+        break;
+      case Socket::TYPE_UDP:
+        info.socket_type = kUDPOption;
+        break;
+      default:
+        info.socket_type = kUnknown;

[Mihai Parparita -not on Chrome, 2012/08/10 00:16:16]

Also use NOTREACHED here.

[Peng, 2012/08/13 16:26:10]

Done.

+        break;
+    }
     info.connected = socket->IsConnected();
 
     // Grab the peer address as known by the OS. This and the call below will