Make downloads.download() respect host permissions

chrome/browser/download/download_extension_api.cc

Index: chrome/browser/download/download_extension_api.cc
diff --git a/chrome/browser/download/download_extension_api.cc b/chrome/browser/download/download_extension_api.cc
index 48006b83ce1ae96bdbcd98f17722a4bffec249a1..58f60ba81da3ef4af9d8e1dfb8f53eea7633945a 100644
--- a/chrome/browser/download/download_extension_api.cc
+++ b/chrome/browser/download/download_extension_api.cc
@@ -381,6 +381,18 @@ bool DownloadsDownloadFunction::ParseArgs() {
     return false;
   }
 
+  // The host permissions should not be why e.g. data, filesystem URLs don't
+  // work.
+  if ((iodata_->url.SchemeIs("http") ||

[Mihai Parparita -not on Chrome, 2012/05/01 20:08:31]

Why do you have these scheme checks? HasHostPermission (despite the name) also
checks schemes.

[benjhayden, 2012/05/01 20:19:12]

HasHostPermission() *disallows* schemes other than these, but download() should
*allow* them. Extensions should be able to "download" data: and filesystem:
URLs.

[Mihai Parparita -not on Chrome, 2012/05/01 20:53:12]

Then I'd rather have those be explicitly listed (i.e. making the check into
!url.SchemeIs("data") && !url.SchemeIs("filesystem") &&
!GetExtension()->HasHostPermission(url)). Otherwise you risk allowing unknown
schemes (e.g. your current code allows downloads from the chrome:// scheme,
which doesn't seem wise).

[benjhayden, 2012/05/02 14:40:18]

Done.

+       iodata_->url.SchemeIs("https") ||
+       iodata_->url.SchemeIs("ftp") ||
+       iodata_->url.SchemeIs("file") ||
+       iodata_->url.SchemeIs("chrome-extension")) &&
+      !GetExtension()->HasHostPermission(iodata_->url)) {
+    error_ = download_extension_errors::kInvalidURLError;
+    return false;
+  }
+
   if (options->HasKey(kFilenameKey)) {
     EXTENSION_FUNCTION_VALIDATE(options->GetString(
         kFilenameKey, &iodata_->filename));