Ensure that window.open requests issued by ChromeFrame carry the correct...

Ensure that window.open requests issued by ChromeFrame carry the correct cookies in the outgoing HTTP requests.

To achieve this we no longer issue navigations with the gcf:attach* prefix. We now issue a navigation to the current
page URL with the attach external tab suffix, which indicates that the page is forced into ChromeFrame without making
an actual HTTP request. This ensures that the new IE8 process has access to all HTTP cookies.

We need to patch IInternetProtocol::LockRequest and UnlockRequest to not call the underlying implementations for our
dummy request as this crashes in IE8 in the prebinding code path.

Fixes bug http://b/issue?id=2277519

Added tests for the CanNavigateFullTabMode helper function.

Bug=2277519

Review URL: http://codereview.chromium.org/3051018

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

chrome_frame/chrome_active_document.cc

@@ -65,8 +65,7 @@ ChromeActiveDocument::ChromeActiveDocument()
     : first_navigation_(true),
       is_automation_client_reused_(false),
       popup_allowed_(false),
-      accelerator_table_(NULL),
-      is_new_navigation_(false) {
+      accelerator_table_(NULL) {
   TRACE_EVENT_BEGIN("chromeframe.createactivedocument", this, "");
 
   url_fetcher_.set_frame_busting(false);
@@ -125,6 +124,14 @@ HRESULT ChromeActiveDocument::FinalConstruct() {
     LoadAccelerators(this_module,
                      MAKEINTRESOURCE(IDR_CHROME_FRAME_IE_FULL_TAB));
   DCHECK(accelerator_table_ != NULL);
+
+  HRESULT hr = security_manager_.CreateInstance(CLSID_InternetSecurityManager);
+  if (FAILED(hr)) {
+    NOTREACHED() << __FUNCTION__
+                 << " Failed to create InternetSecurityManager. Error: 0x%x"
+                 << hr;
+  }
+
   return S_OK;
 }
 
@@ -260,18 +267,16 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
                                   mgr ? mgr->url(): std::wstring());
   }
 
-  // The is_new_navigation variable indicates if this a navigation initiated
-  // by typing in a URL for e.g. in the IE address bar, or from Chrome by
-  // a window.open call from javascript, in which case the current IE tab
-  // will attach to an existing ExternalTabContainer instance.
-  bool is_new_navigation = true;
-  bool is_chrome_protocol = false;
-
-  if (!ParseUrl(url, &is_new_navigation, &is_chrome_protocol, &url)) {
+  ChromeFrameUrl cf_url;
+  if (!cf_url.Parse(url)) {
     DLOG(WARNING) << __FUNCTION__ << " Failed to parse url:" << url;
     return E_INVALIDARG;
   }
 
+  if (!CanNavigateInFullTabMode(cf_url, security_manager_)) {
+    return E_INVALIDARG;
+  }
+
   std::string referrer = mgr ? mgr->referrer() : EmptyString();
   // With CTransaction patch we have more robust way to grab the referrer for
   // each top-level-switch-to-CF request by peeking at our sniffing data
@@ -282,16 +287,17 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
       referrer = prot_data->referrer();
   }
 
-  if (!LaunchUrl(url, referrer, is_new_navigation)) {
+  if (!LaunchUrl(cf_url, referrer)) {
     NOTREACHED() << __FUNCTION__ << " Failed to launch url:" << url;
     return E_INVALIDARG;
   }
 
-  if (!is_chrome_protocol)
-    url_fetcher_.SetInfoForUrl(url, moniker_name, bind_context);
+  if (!cf_url.is_chrome_protocol() && !cf_url.attach_to_external_tab())
+    url_fetcher_.SetInfoForUrl(cf_url.url(), moniker_name, bind_context);
 
   THREAD_SAFE_UMA_HISTOGRAM_CUSTOM_COUNTS("ChromeFrame.FullTabLaunchType",
-                                          is_chrome_protocol, 0, 1, 2);
+                                          cf_url.is_chrome_protocol(),
+                                          0, 1, 2);
   return S_OK;
 }
 
@@ -387,16 +393,18 @@ STDMETHODIMP ChromeActiveDocument::LoadHistory(IStream* stream,
   stream->Read(url_bstr.AllocateBytes(url_size), url_size, &bytes_read);
   std::wstring url(url_bstr);
 
-  bool is_new_navigation = true;
-  bool is_chrome_protocol = false;
-
-  if (!ParseUrl(url, &is_new_navigation, &is_chrome_protocol, &url)) {
+  ChromeFrameUrl cf_url;
+  if (!cf_url.Parse(url)) {
     DLOG(WARNING) << __FUNCTION__ << " Failed to parse url:" << url;
     return E_INVALIDARG;
   }
 
+  if (!CanNavigateInFullTabMode(cf_url, security_manager_)) {
+    return E_INVALIDARG;
+  }
+
   const std::string& referrer = EmptyString();
-  if (!LaunchUrl(url, referrer, is_new_navigation)) {
+  if (!LaunchUrl(cf_url, referrer)) {
     NOTREACHED() << __FUNCTION__ << " Failed to launch url:" << url;
     return E_INVALIDARG;
   }
@@ -729,8 +737,7 @@ void ChromeActiveDocument::UpdateNavigationState(
   bool is_internal_navigation = ((new_navigation_info.navigation_index > 0) &&
       (new_navigation_info.navigation_index !=
        navigation_info_.navigation_index)) ||
-       StartsWith(static_cast<BSTR>(url_), kChromeAttachExternalTabPrefix,
-                  false);
+       MatchPatternWide(static_cast<BSTR>(url_), kChromeFrameAttachTabPattern);
 
   if (new_navigation_info.url.is_valid())
     url_.Allocate(UTF8ToWide(new_navigation_info.url.spec()).c_str());
@@ -950,124 +957,30 @@ HRESULT ChromeActiveDocument::IEExec(const GUID* cmd_group_guid,
   return hr;
 }
 
-DWORD ChromeActiveDocument::MapUrlToZone(const wchar_t* url) {
-  DWORD zone = URLZONE_INVALID;
-  if (security_manager_.get() == NULL) {
-    HRESULT hr = CoCreateInstance(
-        CLSID_InternetSecurityManager,
-        NULL,
-        CLSCTX_ALL,
-        IID_IInternetSecurityManager,
-        reinterpret_cast<void**>(security_manager_.Receive()));
-
-    if (FAILED(hr)) {
-      NOTREACHED() << __FUNCTION__
-                   << " Failed to create InternetSecurityManager. Error: 0x%x"
-                   << hr;
-      return zone;
-    }
-  }
-
-  security_manager_->MapUrlToZone(url, &zone, 0);
-  return zone;
-}
-
-bool ChromeActiveDocument::ParseUrl(const std::wstring& url,
-                                    bool* is_new_navigation,
-                                    bool* is_chrome_protocol,
-                                    std::wstring* parsed_url) {
-  if (!is_new_navigation || !is_chrome_protocol|| !parsed_url) {
-    NOTREACHED() << __FUNCTION__ << " Invalid arguments";
-    return false;
-  }
-
-  std::wstring initial_url = url;
-
-  *is_chrome_protocol = StartsWith(initial_url, kChromeProtocolPrefix,
-                                   false);
-
-  *is_new_navigation = true;
-
-  if (*is_chrome_protocol) {
-    initial_url.erase(0, lstrlen(kChromeProtocolPrefix));
-    *is_new_navigation =
-        !StartsWith(initial_url, kChromeAttachExternalTabPrefix, false);
-  }
-
-  if (!IsValidUrlScheme(initial_url, is_privileged_)) {
-    DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: "
-                  << url;
-    return false;
-  }
-
-  if (URLZONE_UNTRUSTED == MapUrlToZone(initial_url.c_str())) {
-    DLOG(WARNING) << __FUNCTION__
-                  << " Disallowing navigation to restricted url: "
-                  << initial_url;
-    return false;
-  }
-
-  if (*is_chrome_protocol) {
-    // Allow chrome protocol (gcf:) if -
-    // - explicitly enabled using registry
-    // - for gcf:attach_external_tab
-    // - for gcf:about and gcf:view-source
-    GURL crack_url(initial_url);
-    bool allow_gcf_protocol = !*is_new_navigation ||
-        GetConfigBool(false, kEnableGCFProtocol) ||
-        crack_url.SchemeIs(chrome::kAboutScheme) ||
-        crack_url.SchemeIs(chrome::kViewSourceScheme);
-    if (!allow_gcf_protocol)
-      return false;
-  }
-
-  *parsed_url = initial_url;
-  return true;
-}
-
-bool ChromeActiveDocument::LaunchUrl(const std::wstring& url,
-                                     const std::string& referrer,
-                                     bool is_new_navigation) {
+bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,
+                                     const std::string& referrer) {
   DCHECK(automation_client_.get() != NULL);
+  DCHECK(!cf_url.url().empty());
 
-  url_.Allocate(url.c_str());
+  url_.Allocate(cf_url.url().c_str());
 
   std::string utf8_url;
-
-  if (!is_new_navigation) {
-    int disposition = 0;
-    uint64 external_tab_cookie = 0;
-
-    if (!ParseAttachExternalTabUrl(url, &external_tab_cookie, &dimensions_,
-                                   &disposition)) {
-      NOTREACHED() << "Failed to parse attach tab url:" << url;
-      return false;
-    }
-
-    if (external_tab_cookie == 0) {
-      NOTREACHED() << "invalid url for attach tab: " << url;
-      return false;
-    }
-
-    is_new_navigation_ = false;
-    automation_client_->AttachExternalTab(external_tab_cookie);
-  } else {
-    is_new_navigation_ = true;
-    // Initiate navigation before launching chrome so that the url will be
-    // cached and sent with launch settings.
-    if (url_.Length()) {
-      WideToUTF8(url_, url_.Length(), &utf8_url);
-      if (!automation_client_->InitiateNavigation(utf8_url,
-                                                  referrer,
-                                                  is_privileged_)) {
-        DLOG(ERROR) << "Invalid URL: " << url;
-        Error(L"Invalid URL");
-        url_.Reset();
-        return false;
-      }
-
-      DLOG(INFO) << "Url is " << url_;
-    }
+  WideToUTF8(url_, url_.Length(), &utf8_url);
+
+  DLOG(INFO) << "Url is " << url_;
+
+  // Initiate navigation before launching chrome so that the url will be
+  // cached and sent with launch settings.
+  if (cf_url.attach_to_external_tab()) {
+    dimensions_ = cf_url.dimensions();
+    automation_client_->AttachExternalTab(cf_url.cookie());
+  } else if (!automation_client_->InitiateNavigation(utf8_url,
+                                                     referrer,
+                                                     is_privileged_)) {
+    DLOG(ERROR) << "Invalid URL: " << url_;
+    Error(L"Invalid URL");
+    url_.Reset();
+    return false;
   }
 
   if (is_automation_client_reused_)

chrome_frame/chrome_active_document.h

@@ -9,10 +9,10 @@
 #include <atlcom.h>
 #include <atlctl.h>
 #include <htiframe.h>
+#include <map>
 #include <mshtmcid.h>
 #include <perhist.h>
-
-#include <map>
+#include <string>
 
 #include "base/scoped_ptr.h"
 #include "base/scoped_comptr_win.h"
@@ -382,20 +382,9 @@ END_EXEC_COMMAND_MAP()
   HRESULT IEExec(const GUID* cmd_group_guid, DWORD command_id,
                  DWORD cmd_exec_opt, VARIANT* in_args, VARIANT* out_args);
 
-  DWORD MapUrlToZone(const wchar_t* url);
-
-  // Parses the URL and returns information whether it is a new navigation and
-  // the actual url after stripping out the cf: prefix if any.
-  // This function also checks if the url scheme is valid for navigation within
-  // chrome and whether it is a restricted URL as per IE settings. In either of
-  // these cases it returns false.
-  bool ParseUrl(const std::wstring& url, bool* is_new_navigation,
-                bool* is_chrome_protocol, std::wstring* parsed_url);
-
   // Initiates navigation to the URL passed in.
   // Returns true on success.
-  bool LaunchUrl(const std::wstring& url, const std::string& referrer,
-                 bool is_new_navigation);
+  bool LaunchUrl(const ChromeFrameUrl& cf_url, const std::string& referrer);
 
   // Handler to set the page font size in Chrome.
   HRESULT SetPageFontSize(const GUID* cmd_group_guid,
@@ -471,8 +460,6 @@ END_EXEC_COMMAND_MAP()
 
   // Dimensions of the window. Used only when opening popups.
   gfx::Rect dimensions_;
-  // Set to true if the document was loaded by a window.open in chrome.
-  bool is_new_navigation_;
  public:
   ScopedComPtr<IOleInPlaceFrame> in_place_frame_;
   OLEINPLACEFRAMEINFO frame_info_;

chrome_frame/chrome_frame_activex_base.h

@@ -480,15 +480,19 @@ END_MSG_MAP()
 
   virtual void OnAttachExternalTab(int tab_handle,
       const IPC::AttachExternalTabParams& params) {
-    std::string url;
-    url = StringPrintf("%lsattach_external_tab&%ls&%d&%d&%d&%d&%d",
-                       kChromeProtocolPrefix,
-                       Uint64ToWString(params.cookie).c_str(),
-                       params.disposition,
-                       params.dimensions.x(),
-                       params.dimensions.y(),
-                       params.dimensions.width(),
-                       params.dimensions.height());
+    std::wstring wide_url = url_;
+    GURL parsed_url(WideToUTF8(wide_url));
+
+    std::string url =
+        StringPrintf("%hs:%hs?attach_external_tab&%I64u&%d&%d&%d&%d&%d",
+                     parsed_url.scheme().c_str(),
+                     parsed_url.host().c_str(),
+                     params.cookie,
+                     params.disposition,
+                     params.dimensions.x(),
+                     params.dimensions.y(),
+                     params.dimensions.width(),
+                     params.dimensions.height());
     HostNavigate(GURL(url), GURL(), params.disposition);
   }
 

chrome_frame/protocol_sink_wrap.cc

@@ -13,6 +13,7 @@
 #include "base/singleton.h"
 #include "base/string_util.h"
 
+#include "chrome_frame/bho.h"
 #include "chrome_frame/bind_context_info.h"
 #include "chrome_frame/function_stub.h"
 #include "chrome_frame/utils.h"
@@ -30,6 +31,8 @@ const wchar_t kUrlMonDllName[] = L"urlmon.dll";
 static const int kInternetProtocolStartIndex = 3;
 static const int kInternetProtocolReadIndex = 9;
 static const int kInternetProtocolStartExIndex = 13;
+static const int kInternetProtocolLockRequestIndex = 11;
+static const int kInternetProtocolUnlockRequestIndex = 12;
 
 
 // IInternetProtocol/Ex patches.
@@ -55,10 +58,18 @@ STDMETHODIMP Hook_Read(InternetProtocol_Read_Fn orig_read,
                        ULONG size,
                        ULONG* size_read);
 
+STDMETHODIMP Hook_LockRequest(InternetProtocol_LockRequest_Fn orig_req,
+                              IInternetProtocol* protocol, DWORD dwOptions);
+
+STDMETHODIMP Hook_UnlockRequest(InternetProtocol_UnlockRequest_Fn orig_req,
+                                IInternetProtocol* protocol);
+
 /////////////////////////////////////////////////////////////////////////////
 BEGIN_VTABLE_PATCHES(CTransaction)
   VTABLE_PATCH_ENTRY(kInternetProtocolStartIndex, Hook_Start)
   VTABLE_PATCH_ENTRY(kInternetProtocolReadIndex, Hook_Read)
+  VTABLE_PATCH_ENTRY(kInternetProtocolLockRequestIndex, Hook_LockRequest)
+  VTABLE_PATCH_ENTRY(kInternetProtocolUnlockRequestIndex, Hook_UnlockRequest)
 END_VTABLE_PATCHES()
 
 BEGIN_VTABLE_PATCHES(CTransaction2)
@@ -502,7 +513,6 @@ void ProtData::SaveReferrer(IInternetProtocolSink* delegate) {
   DCHECK_EQ(CHROME, renderer_type_);
   ScopedComPtr<IWinInetHttpInfo> info;
   info.QueryFrom(delegate);
-  DCHECK(info);
   if (info) {
     char buffer[4096] = {0};
     DWORD len = sizeof(buffer);
@@ -512,6 +522,8 @@ void ProtData::SaveReferrer(IInternetProtocolSink* delegate) {
         buffer, &len, &flags, 0);
     if (hr == S_OK && len > 0)
       referrer_.assign(buffer);
+  } else {
+    DLOG(WARNING) << "Failed to QI for IWinInetHttpInfo";
   }
 }
 
@@ -525,6 +537,38 @@ scoped_refptr<ProtData> ProtData::DataFromProtocol(
   return instance;
 }
 
+// This function looks for the url pattern indicating that this request needs
+// to be forced into chrome frame.
+// This hack is required because window.open requests from Chrome don't have
+// the URL up front. The URL comes in much later when the renderer initiates a
+// top level navigation for the url passed into window.open.
+// The new page must be rendered in ChromeFrame to preserve the opener
+// relationship with its parent even if the new page does not have the chrome
+// meta tag.
+bool HandleAttachToExistingExternalTab(LPCWSTR url,
+                                       IInternetProtocol* protocol,
+                                       IInternetProtocolSink* prot_sink,
+                                       IBindCtx* bind_ctx) {
+  if (MatchPatternWide(url, kChromeFrameAttachTabPattern)) {
+    scoped_refptr<ProtData> prot_data = ProtData::DataFromProtocol(protocol);
+    if (!prot_data) {
+      // Pass NULL as the read function which indicates that always return EOF
+      // without calling the underlying protocol.
+      prot_data = new ProtData(protocol, NULL, url);
+      PutProtData(bind_ctx, prot_data);
+    }
+
+    prot_sink->ReportProgress(BINDSTATUS_MIMETYPEAVAILABLE, kChromeMimeType);
+
+    int data_flags = BSCF_FIRSTDATANOTIFICATION | BSCF_LASTDATANOTIFICATION;
+    prot_sink->ReportData(data_flags, 0, 0);
+
+    prot_sink->ReportResult(S_OK, 0, NULL);
+    return true;
+  }
+  return false;
+}
+
 // IInternetProtocol/Ex hooks.
 STDMETHODIMP Hook_Start(InternetProtocol_Start_Fn orig_start,
     IInternetProtocol* protocol, LPCWSTR url, IInternetProtocolSink* prot_sink,
@@ -542,6 +586,10 @@ STDMETHODIMP Hook_Start(InternetProtocol_Start_Fn orig_start,
     return orig_start(protocol, url, prot_sink, bind_info, flags, reserved);
   }
 
+  if (HandleAttachToExistingExternalTab(url, protocol, prot_sink, bind_ctx)) {
+    return S_OK;
+  }
+
   if (IsCFRequest(bind_ctx)) {
     return orig_start(protocol, url, prot_sink, bind_info, flags, reserved);
   }
@@ -589,6 +637,10 @@ STDMETHODIMP Hook_StartEx(InternetProtocol_StartEx_Fn orig_start_ex,
     return orig_start_ex(protocol, uri, prot_sink, bind_info, flags, reserved);
   }
 
+  if (HandleAttachToExistingExternalTab(url, protocol, prot_sink, bind_ctx)) {
+    return S_OK;
+  }
+
   if (IsCFRequest(bind_ctx)) {
     return orig_start_ex(protocol, uri, prot_sink, bind_info, flags, reserved);
   }
@@ -620,15 +672,51 @@ STDMETHODIMP Hook_StartEx(InternetProtocol_StartEx_Fn orig_start_ex,
 STDMETHODIMP Hook_Read(InternetProtocol_Read_Fn orig_read,
     IInternetProtocol* protocol, void* buffer, ULONG size, ULONG* size_read) {
   DCHECK(orig_read);
+  HRESULT hr = E_FAIL;
+
   scoped_refptr<ProtData> prot_data = ProtData::DataFromProtocol(protocol);
   if (!prot_data) {
-    return orig_read(protocol, buffer, size, size_read);
+    hr = orig_read(protocol, buffer, size, size_read);
+    return hr;
+  }
+
+  if (prot_data->is_attach_external_tab_request()) {
+    // return EOF always.
+    if (size_read)
+      *size_read = 0;
+    return S_FALSE;
   }
 
-  HRESULT hr = prot_data->Read(buffer, size, size_read);
+  hr = prot_data->Read(buffer, size, size_read);
   return hr;
 }
 
+STDMETHODIMP Hook_LockRequest(InternetProtocol_LockRequest_Fn orig_req,
+                              IInternetProtocol* protocol, DWORD options) {
+  DCHECK(orig_req);
+
+  scoped_refptr<ProtData> prot_data = ProtData::DataFromProtocol(protocol);
+  if (prot_data && prot_data->is_attach_external_tab_request()) {
+    prot_data->AddRef();
+    return S_OK;
+  }
+
+  return orig_req(protocol, options);
+}
+
+STDMETHODIMP Hook_UnlockRequest(InternetProtocol_UnlockRequest_Fn orig_req,
+                                IInternetProtocol* protocol) {
+  DCHECK(orig_req);
+
+  scoped_refptr<ProtData> prot_data = ProtData::DataFromProtocol(protocol);
+  if (prot_data && prot_data->is_attach_external_tab_request()) {
+    prot_data->Release();
+    return S_OK;
+  }
+
+  return orig_req(protocol);
+}
+
 // Patching / Hooking code.
 class FakeProtocol : public CComObjectRootEx<CComSingleThreadModel>,
                      public IInternetProtocol {

chrome_frame/protocol_sink_wrap.h

@@ -36,8 +36,10 @@ typedef HRESULT (STDMETHODCALLTYPE* InternetProtocol_StartEx_Fn)(
     IInternetProtocolEx* this_object, IUri* uri,
     IInternetProtocolSink* prot_sink, IInternetBindInfo* bind_info,
     DWORD flags, HANDLE_PTR reserved);
-typedef HRESULT (STDMETHODCALLTYPE* InternetProtocolRoot_Continue_Fn)(
-    IInternetProtocolRoot* me, PROTOCOLDATA* data);
+typedef HRESULT (STDMETHODCALLTYPE* InternetProtocol_LockRequest_Fn)(
+    IInternetProtocol* this_object, DWORD options);
+typedef HRESULT (STDMETHODCALLTYPE* InternetProtocol_UnlockRequest_Fn)(
+    IInternetProtocol* this_object);
 
 
 enum RendererType {
@@ -130,6 +132,10 @@ class ProtData : public base::RefCounted<ProtData> {
     return referrer_;
   }
 
+  bool is_attach_external_tab_request() const {
+    return read_fun_ == NULL;
+  }
+
  private:
   typedef std::map<IInternetProtocol*, ProtData*> ProtocolDataMap;
   static ProtocolDataMap datamap_;

chrome_frame/test/util_unittests.cc

@@ -5,7 +5,9 @@
 #include "base/file_version_info.h"
 #include "base/file_version_info_win.h"
 #include "chrome_frame/utils.h"
+
 #include "testing/gtest/include/gtest/gtest.h"
+#include "testing/gmock/include/gmock/gmock.h"
 
 const wchar_t kChannelName[] = L"-dev";
 const wchar_t kSuffix[] = L"-fix";
@@ -114,27 +116,130 @@ TEST(UtilTests, GetTempInternetFiles) {
 }
 
 TEST(UtilTests, ParseAttachTabUrlTest) {
-  std::wstring url = L"attach_external_tab&10&1&0&0&100&100";
-
-  uint64 cookie = 0;
-  gfx::Rect dimensions;
-  int disposition = 0;
-
-  EXPECT_TRUE(ParseAttachExternalTabUrl(url, &cookie, &dimensions,
-                                        &disposition));
-  EXPECT_EQ(10, cookie);
-  EXPECT_EQ(1, disposition);
-  EXPECT_EQ(0, dimensions.x());
-  EXPECT_EQ(0, dimensions.y());
-  EXPECT_EQ(100, dimensions.width());
-  EXPECT_EQ(100, dimensions.height());
-
-  url = L"http://www.foobar.com?&10&1&0&0&100&100";
-  EXPECT_FALSE(ParseAttachExternalTabUrl(url, &cookie, &dimensions,
-                                         &disposition));
-  url = L"attach_external_tab&10&1";
-  EXPECT_FALSE(ParseAttachExternalTabUrl(url, &cookie, &dimensions,
-                                         &disposition));
+  ChromeFrameUrl cf_url;
+
+  EXPECT_TRUE(cf_url.Parse(L"http://f/?attach_external_tab&10&1&0&0&100&100"));
+
+  EXPECT_TRUE(cf_url.attach_to_external_tab());
+  EXPECT_FALSE(cf_url.is_chrome_protocol());
+
+  EXPECT_EQ(10, cf_url.cookie());
+  EXPECT_EQ(1, cf_url.disposition());
+  EXPECT_EQ(0, cf_url.dimensions().x());
+  EXPECT_EQ(0, cf_url.dimensions().y());
+  EXPECT_EQ(100, cf_url.dimensions().width());
+  EXPECT_EQ(100, cf_url.dimensions().height());
+
+  EXPECT_TRUE(cf_url.Parse(L"http://www.foobar.com?&10&1&0&0&100&100"));
+  EXPECT_FALSE(cf_url.attach_to_external_tab());
+
+  EXPECT_FALSE(cf_url.Parse(L"attach_external_tab&10&1"));
+  EXPECT_TRUE(cf_url.attach_to_external_tab());
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://f/?attach_tab&10&1&0&0&100&100"));
+  EXPECT_FALSE(cf_url.attach_to_external_tab());
+  EXPECT_TRUE(cf_url.is_chrome_protocol());
+}
+
+// Mock for the IInternetSecurityManager interface
+class MockIInternetSecurityManager : public IInternetSecurityManager {
+ public:
+  MOCK_METHOD2_WITH_CALLTYPE(__stdcall, QueryInterface,
+      HRESULT(REFIID iid, void** object));
+
+  MOCK_METHOD0_WITH_CALLTYPE(__stdcall, AddRef, ULONG());
+  MOCK_METHOD0_WITH_CALLTYPE(__stdcall, Release, ULONG());
+
+  MOCK_METHOD1_WITH_CALLTYPE(__stdcall, SetSecuritySite,
+      HRESULT(IInternetSecurityMgrSite* site));
+  MOCK_METHOD1_WITH_CALLTYPE(__stdcall, GetSecuritySite,
+      HRESULT(IInternetSecurityMgrSite** site));
+  MOCK_METHOD3_WITH_CALLTYPE(__stdcall, MapUrlToZone,
+      HRESULT(LPCWSTR url, DWORD* zone, DWORD flags));
+  MOCK_METHOD4_WITH_CALLTYPE(__stdcall, GetSecurityId,
+      HRESULT(LPCWSTR url, BYTE* security_id, DWORD* security_size,
+              DWORD_PTR reserved));
+  MOCK_METHOD8_WITH_CALLTYPE(__stdcall, ProcessUrlAction,
+      HRESULT(LPCWSTR url, DWORD action, BYTE* policy, DWORD cb_policy,
+              BYTE* context, DWORD context_size, DWORD flags,
+              DWORD reserved));
+  MOCK_METHOD7_WITH_CALLTYPE(__stdcall, QueryCustomPolicy,
+      HRESULT(LPCWSTR url, REFGUID guid, BYTE** policy, DWORD* cb_policy,
+              BYTE* context, DWORD cb_context, DWORD reserved));
+  MOCK_METHOD3_WITH_CALLTYPE(__stdcall, SetZoneMapping,
+      HRESULT(DWORD zone, LPCWSTR pattern, DWORD flags));
+  MOCK_METHOD3_WITH_CALLTYPE(__stdcall, GetZoneMappings,
+      HRESULT(DWORD zone, IEnumString** enum_string, DWORD flags));
+};
+
+TEST(UtilTests, CanNavigateFullTabModeTest) {
+  ChromeFrameUrl cf_url;
+
+  MockIInternetSecurityManager mock;
+  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"http://blah"),
+                                                     testing::_, testing::_))
+    .WillRepeatedly(testing::DoAll(
+        testing::SetArgumentPointee<1>(URLZONE_INTERNET),
+        testing::Return(S_OK)));
+
+  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"http://untrusted"),
+                                                     testing::_, testing::_))
+    .WillRepeatedly(testing::DoAll(
+        testing::SetArgumentPointee<1>(URLZONE_UNTRUSTED),
+        testing::Return(S_OK)));
+
+  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"about:"),
+                                                     testing::_, testing::_))
+    .WillRepeatedly(testing::DoAll(
+        testing::SetArgumentPointee<1>(URLZONE_TRUSTED),
+        testing::Return(S_OK)));
+
+  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"view-source:"),
+                                                     testing::_, testing::_))
+    .WillRepeatedly(testing::DoAll(
+        testing::SetArgumentPointee<1>(URLZONE_TRUSTED),
+        testing::Return(S_OK)));
+
+  EXPECT_TRUE(cf_url.Parse(
+      L"http://blah/?attach_external_tab&10&1&0&0&100&100"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(
+      L"http://untrusted/bar.html"));
+  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(
+      L"http://blah/?attach_external_tab&10&1&0&0&100&100"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:about:blank"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:view-source:http://www.foo.com"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:about:version"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:about:bar"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  bool enable_gcf = GetConfigBool(false, kEnableGCFProtocol);
+
+  SetConfigBool(kEnableGCFProtocol, false);
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://blah"));
+  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  SetConfigBool(kEnableGCFProtocol, true);
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://blah"));
+  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://untrusted/bar"));
+  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
+
+  SetConfigBool(kEnableGCFProtocol, enable_gcf);
 }
 
 TEST(UtilTests, ParseVersionTest) {

chrome_frame/utils.cc

@@ -43,6 +43,7 @@ const wchar_t kChromeContentPrefix[] = L"chrome=";
 const wchar_t kChromeProtocolPrefix[] = L"gcf:";
 const wchar_t kChromeMimeType[] = L"application/chromepage";
 const wchar_t kPatchProtocols[] = L"PatchProtocols";
+const wchar_t kChromeFrameAttachTabPattern[] = L"*?attach_external_tab&*";
 
 static const wchar_t kChromeFrameConfigKey[] =
     L"Software\\Google\\ChromeFrame";
@@ -58,7 +59,7 @@ static const wchar_t kChromeFramePersistNPAPIReg[] = L"PersistNPAPIReg";
 const wchar_t kChromeFrameOmahaSuffix[] = L"-cf";
 const wchar_t kDevChannelName[] = L"-dev";
 
-const wchar_t kChromeAttachExternalTabPrefix[] = L"attach_external_tab";
+const wchar_t kChromeAttachExternalTabPrefix[] = L"?attach_external_tab";
 
 // Indicates that we are running in a test environment, where execptions, etc
 // are handled by the chrome test crash server.
@@ -858,9 +859,6 @@ bool IsValidUrlScheme(const std::wstring& url, bool is_privileged) {
        crack_url.SchemeIs(chrome::kExtensionScheme)))
     return true;
 
-  if (StartsWith(url, kChromeAttachExternalTabPrefix, false))
-    return true;
-
   return false;
 }
 
@@ -1238,21 +1236,52 @@ HRESULT ReadStream(IStream* stream, size_t size, std::string* data) {
   return hr;
 }
 
-bool ParseAttachExternalTabUrl(const std::wstring& url, uint64* cookie,
-                               gfx::Rect* dimensions, int* disposition) {
-  if (!StartsWith(url, kChromeAttachExternalTabPrefix, true)) {
-    DLOG(WARNING) << "Invalid url passed in:"
-                  << url.c_str();
-    return false;
+ChromeFrameUrl::ChromeFrameUrl()
+    : is_chrome_protocol_(false),
+      attach_to_external_tab_(false),
+      cookie_(0),
+      disposition_(0) {
+}
+
+bool ChromeFrameUrl::Parse(const std::wstring& url) {
+  bool ret = false;
+  if (url.empty())
+    return ret;
+
+  url_ = url;
+
+  attach_to_external_tab_ = MatchPatternWide(url.c_str(),
+                                             kChromeFrameAttachTabPattern);
+  is_chrome_protocol_ = StartsWith(url, kChromeProtocolPrefix,
+                                   false);
+  DCHECK(!(attach_to_external_tab_ && is_chrome_protocol_));
+  if (is_chrome_protocol_) {
+    url_.erase(0, lstrlen(kChromeProtocolPrefix));
   }
 
-  if (!cookie || !dimensions || !disposition)
+  if (attach_to_external_tab_) {
+    ret = ParseAttachExternalTabUrl();
+  } else {
+    ret = true;
+  }
+  return ret;
+}
+
+bool ChromeFrameUrl::ParseAttachExternalTabUrl() {
+  size_t attach_external_tab_start_pos =
+      url_.find(kChromeAttachExternalTabPrefix);
+  if (attach_external_tab_start_pos == std::wstring::npos) {
+    NOTREACHED() << "Invalid url:" << url_;
     return false;
+  }
+
+  std::wstring url =
+      url_.substr(attach_external_tab_start_pos,
+                  url_.length() - attach_external_tab_start_pos);
 
   WStringTokenizer tokenizer(url, L"&");
   // Skip over kChromeAttachExternalTabPrefix
   tokenizer.GetNext();
-
   // Read the following items in order.
   // 1. cookie
   // 2. disposition
@@ -1262,41 +1291,81 @@ bool ParseAttachExternalTabUrl(const std::wstring& url, uint64* cookie,
   // 6. dimension.height.
   if (tokenizer.GetNext()) {
     wchar_t* end_ptr = 0;
-    *cookie = _wcstoui64(tokenizer.token().c_str(), &end_ptr, 10);
+    cookie_ = _wcstoui64(tokenizer.token().c_str(), &end_ptr, 10);
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    *disposition = _wtoi(tokenizer.token().c_str());
+    disposition_ = _wtoi(tokenizer.token().c_str());
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions->set_x(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_x(_wtoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions->set_y(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_y(_wtoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions->set_width(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_width(_wtoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions->set_height(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_height(_wtoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
-
   return true;
 }
 
+bool CanNavigateInFullTabMode(const ChromeFrameUrl& cf_url,
+                              IInternetSecurityManager* security_manager) {
+  bool is_privileged = false;
+
+  if (!IsValidUrlScheme(cf_url.url(), is_privileged)) {
+    DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: "
+                  << cf_url.url();
+    return false;
+  }
+
+  if (security_manager) {
+    DWORD zone = URLZONE_INVALID;
+    security_manager->MapUrlToZone(cf_url.url().c_str(), &zone, 0);
+    if (zone == URLZONE_UNTRUSTED) {
+      DLOG(WARNING) << __FUNCTION__
+                    << " Disallowing navigation to restricted url: "
+                    << cf_url.url();
+      return false;
+    }
+  }
+
+  if (cf_url.is_chrome_protocol()) {
+    // Allow chrome protocol (gcf:) if -
+    // - explicitly enabled using registry
+    // - for gcf:attach_external_tab
+    // - for gcf:about and gcf:view-source
+    GURL crack_url(cf_url.url());
+    bool allow_gcf_protocol =
+        GetConfigBool(false, kEnableGCFProtocol) ||
+        crack_url.SchemeIs(chrome::kAboutScheme) ||
+        crack_url.SchemeIs(chrome::kViewSourceScheme);
+    if (!allow_gcf_protocol) {
+      DLOG(WARNING) << __FUNCTION__
+                    << " Disallowing navigation to gcf url: "
+                    << cf_url.url();
+      return false;
+    }
+  }
+
+  return true;
+}

chrome_frame/utils.h

@@ -30,6 +30,7 @@ extern const wchar_t kChromeFrameUnpinnedMode[];
 extern const wchar_t kEnableGCFProtocol[];
 extern const wchar_t kEnableBuggyBhoIntercept[];
 extern const wchar_t kChromeMimeType[];
+extern const wchar_t kChromeFrameAttachTabPattern[];
 
 typedef enum ProtocolPatchMethod {
   PATCH_METHOD_IBROWSER = 0,
@@ -472,13 +473,58 @@ std::string Bscf2Str(DWORD flags);
 // Reads data from a stream into a string.
 HRESULT ReadStream(IStream* stream, size_t size, std::string* data);
 
-// Parses the attach external tab url, which comes in from Chrome in the course
-// of a window.open operation. The format of this URL is as below:-
-// gcf:attach_external_tab&n1&n2&x&y&width&height
-// n1 -> cookie, n2 -> disposition, x, y, width, height -> dimensions of the
-// window.
-// Returns true on success.
-bool ParseAttachExternalTabUrl(const std::wstring& url, uint64* cookie,
-                               gfx::Rect* dimensions, int* disposition);
+// Parses urls targetted at ChromeFrame. This class maintains state like
+// whether a url is prefixed with the gcf: prefix, whether it is being
+// attached to an existing external tab, etc.
+class ChromeFrameUrl {
+ public:
+  ChromeFrameUrl();
+
+  // Parses the url passed in. Returns true on success.
+  bool Parse(const std::wstring& url);
+
+  bool is_chrome_protocol() const {
+    return is_chrome_protocol_;
+  }
+
+  bool attach_to_external_tab() const {
+    return attach_to_external_tab_;
+  }
+
+  uint64 cookie() const {
+    return cookie_;
+  }
+
+  int disposition() const {
+    return disposition_;
+  }
+
+  const gfx::Rect& dimensions() const {
+    return dimensions_;
+  }
+
+  const std::wstring& url() const {
+    return url_;
+  }
+
+ private:
+  // If we are attaching to an existing external tab, this function parses the
+  // suffix portion of the URL which contains the attach_external_tab prefix.
+  bool ParseAttachExternalTabUrl();
+
+  bool attach_to_external_tab_;
+  bool is_chrome_protocol_;
+  std::wstring url_;
+  uint64 cookie_;
+  gfx::Rect dimensions_;
+  int disposition_;
+};
+
+// Returns true if we can navigate to this URL.
+// This function checks if the url scheme is valid for navigation within
+// chrome and whether it is a restricted URL as per IE settings. In either of
+// these cases it returns false.
+bool CanNavigateInFullTabMode(const ChromeFrameUrl& cf_url,
+                              IInternetSecurityManager* security_manager);
 
 #endif  // CHROME_FRAME_UTILS_H_