Validate debuggee.targetId before use in chrome.debugger

chrome/browser/extensions/api/debugger/debugger_api.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.
 
 // Implements the Chrome Extensions Debugger API.
 
 #include "chrome/browser/extensions/api/debugger/debugger_api.h"
 
 #include <map>
 #include <set>
@@ skipping 511 matching lines @@
                                            extension_host->GetURL(),
                                            extension(),
                                            &error_)) {
         return false;
       }
       agent_host_ =
           DevToolsAgentHost::GetOrCreateFor(extension_host->host_contents());
     }
   } else if (debuggee_.target_id) {
     agent_host_ = DevToolsAgentHost::GetForId(*debuggee_.target_id);
+    if (agent_host_.get()) {
+      if (PermissionsData::IsRestrictedUrl(agent_host_->GetURL(),

[Devlin, 2015/02/09 23:11:22]

I'm a little worried about race conditions here, if, for instance, the debugger
is attached before the url is fully resolved. Not sure how much we can do about
it, but a sanity check that url is_valid() would probably be better than
nothing.

[robwu, 2015/02/10 12:56:31]

Isn't the IsValidSchemeForExtensions check in PermissionsData::IsRestrictedUrl
sufficient?

[Devlin, 2015/02/10 17:14:42]

Yeah, that'd probably work.  Nevermind.

+                                           agent_host_->GetURL(),
+                                           extension(),
+                                           &error_)) {
+        agent_host_ = NULL;

[Devlin, 2015/02/09 23:11:22]

nit: prefer nullptr now.

[robwu, 2015/02/10 12:56:31]

Done, but the rest of the file still uses NULL (should I change back to NULL,
replace the rest with nullptr, or leave it as-is?).

[Devlin, 2015/02/10 17:14:42]

The rule for nullptr is pretty much just "prefer nullptr".  So it's worth using
in new code, or when refactoring old code, but probably not worth contaminating
the git blame history for the whole file (which is why we haven't just done a
repo-wide find-and-replace).

+        return false;
+      }
+    }
   } else {
     error_ = keys::kInvalidTargetError;
     return false;
   }
 
   if (!agent_host_.get()) {
     FormatErrorMessage(keys::kNoTargetError);
     return false;
   }
   return true;
@@ skipping 192 matching lines @@
     const std::vector<DevToolsTargetImpl*>& target_list) {
   scoped_ptr<base::ListValue> result(new base::ListValue());
   for (size_t i = 0; i < target_list.size(); ++i)
     result->Append(SerializeTarget(*target_list[i]));
   STLDeleteContainerPointers(target_list.begin(), target_list.end());
   SetResult(result.release());
   SendResponse(true);
 }
 
 }  // namespace extensions