From 2198b402423426781e5285bd87777cf6f7d5d935 Mon Sep 17 00:00:00 2001 From: Nishant Bhasin Date: Fri, 11 Oct 2024 18:18:33 -0400 Subject: [PATCH 1/2] Bugfix FXIOS-9483 Bugzilla 1905749 --- ...owserViewController+WebViewDelegates.swift | 23 +++++++++- .../WebView/WebViewNavigationHandler.swift | 46 +++++++++++-------- .../Legacy/LegacyTabManager.swift | 7 ++- 3 files changed, 55 insertions(+), 21 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift index 94e3669c317e..350dd7b1b5ba 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift @@ -17,6 +17,13 @@ extension BrowserViewController: WKUIDelegate { for navigationAction: WKNavigationAction, windowFeatures: WKWindowFeatures ) -> WKWebView? { + print("NB -- 1 parent tab webview url \(String(describing: tabManager[webView]?.webView?.url))") + print("NB -- 1.1 parent tab webview title \(String(describing: tabManager[webView]?.webView?.title))") + print("NB -- 2 webView url \(String(describing: webView.url))") + print("NB -- 2.1 webView title \(String(describing: webView.title))") + print("NB -- 2.2 webView tag \(webView.tag)") + print("NB -- 3 navigationAction request \(navigationAction.request)") + print("NB -- 3.1 navigationAction request url \(String(describing: navigationAction.request.url))") guard let parentTab = tabManager[webView] else { return nil } guard !navigationAction.isInternalUnprivileged, shouldRequestBeOpenedAsPopup(navigationAction.request) @@ -35,6 +42,18 @@ extension BrowserViewController: WKUIDelegate { return nil } + let navigationUrl = navigationAction.request.url + let navigationUrlString = navigationUrl?.absoluteString ?? "" + + // Check for "data" scheme using WebViewNavigationHandlerImplementation + let navigationHandler = WebViewNavigationHandlerImplementation { _ in } + var shouldAllowDataScheme = true + if navigationHandler.shouldFilterDataScheme(url: navigationUrl) { + shouldAllowDataScheme = navigationHandler.shouldAllowDataScheme(for: navigationUrl) + } + + guard shouldAllowDataScheme else { return nil } + // If the page uses `window.open()` or `[target="_blank"]`, open the page in a new tab. // IMPORTANT!!: WebKit will perform the `URLRequest` automatically!! Attempting to do // the request here manually leads to incorrect results!! @@ -44,7 +63,7 @@ extension BrowserViewController: WKUIDelegate { configuration: configuration ) - if navigationAction.request.url == nil { + if navigationUrl == nil || navigationUrlString.isEmpty { newTab.url = URL(string: "about:blank") } @@ -512,6 +531,8 @@ extension BrowserViewController: WKNavigationDelegate { } let navigationHandler = WebViewNavigationHandlerImplementation(decisionHandler: decisionHandler) + print("NB -- 0 Data scheme \(url)") + print("NB -- 0 Data shouldFilterDataScheme \(navigationHandler.shouldFilterDataScheme(url: url))") if navigationHandler.shouldFilterDataScheme(url: url) { navigationHandler.filterDataScheme(url: url, navigationAction: navigationAction) return diff --git a/firefox-ios/Client/Frontend/Browser/WebView/WebViewNavigationHandler.swift b/firefox-ios/Client/Frontend/Browser/WebView/WebViewNavigationHandler.swift index 786ff92c4dae..0c5ef7af6dc9 100644 --- a/firefox-ios/Client/Frontend/Browser/WebView/WebViewNavigationHandler.swift +++ b/firefox-ios/Client/Frontend/Browser/WebView/WebViewNavigationHandler.swift @@ -11,7 +11,7 @@ protocol WebViewNavigationHandler { /// Whether we should filter that URL for data scheme or not /// - Returns: True when the URL needs to be filtered for the data scheme - func shouldFilterDataScheme(url: URL) -> Bool + func shouldFilterDataScheme(url: URL?) -> Bool /// Filter top-level data scheme has defined in: /// https://blog.mozilla.org/security/2017/11/27/blocking-top-level-navigations-data-urls-firefox-59/ @@ -38,7 +38,8 @@ struct WebViewNavigationHandlerImplementation: WebViewNavigationHandler { self.decisionHandler = decisionHandler } - func shouldFilterDataScheme(url: URL) -> Bool { + func shouldFilterDataScheme(url: URL?) -> Bool { + guard let url else { return false } return url.scheme == WebViewNavigationHandlerImplementation.Scheme.data.rawValue } @@ -51,32 +52,39 @@ struct WebViewNavigationHandlerImplementation: WebViewNavigationHandler { return } - let url = url.absoluteString.lowercased() - // Allow certain image types - if url.hasPrefix("data:image/") && !url.hasPrefix("data:image/svg+xml") { + if shouldAllowDataScheme(for: url) { decisionHandler(.allow) - return + } else { + decisionHandler(.cancel) + } + } + + func shouldAllowDataScheme(for url: URL?) -> Bool { + guard let url else { return false } + let urlString = url.absoluteString.lowercased() + + // Allow certain image types + if urlString.hasPrefix("data:image/") && !urlString.hasPrefix("data:image/svg+xml") { + return true } // Allow video, and certain application types - if url.hasPrefix("data:video/") - || url.hasPrefix("data:application/pdf") - || url.hasPrefix("data:application/json") { - decisionHandler(.allow) - return + if urlString.hasPrefix("data:video/") + || urlString.hasPrefix("data:application/pdf") + || urlString.hasPrefix("data:application/json") { + return true } // Allow plain text types. - // Note the format of data URLs is `data:[][;base64],` + // Note the format of data URLs is `data:[][;base64],` // with empty indicating plain text. - if url.hasPrefix("data:;base64,") - || url.hasPrefix("data:,") - || url.hasPrefix("data:text/plain,") - || url.hasPrefix("data:text/plain;") { - decisionHandler(.allow) - return + if urlString.hasPrefix("data:;base64,") + || urlString.hasPrefix("data:,") + || urlString.hasPrefix("data:text/plain,") + || urlString.hasPrefix("data:text/plain;") { + return true } - decisionHandler(.cancel) + return false } } diff --git a/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift b/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift index 75bbc801e67e..4cae17f0dc07 100644 --- a/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift +++ b/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift @@ -399,6 +399,8 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler zombie: Bool, isPrivate: Bool = false ) -> Tab { + print("NB -- 4 Add tab request request \(String(describing: request))") + print("NB -- 4.1 Add tab request url \(String(describing: request?.url))") let tab = Tab(profile: profile, isPrivate: isPrivate, windowUUID: windowUUID) configureTab(tab, request: request, afterTab: afterTab, flushToDisk: flushToDisk, zombie: zombie) return tab @@ -408,7 +410,10 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler let popup = Tab(profile: profile, isPrivate: parentTab.isPrivate, windowUUID: windowUUID) - + print("NB -- 5 addPopupForParentTab popup webView title \(String(describing: popup.webView?.title))") + print("NB -- 5.1 addPopupForParentTab popup webView url \(String(describing: popup.webView?.url))") + print("NB -- 5.2 addPopupForParentTab parent webView title \(String(describing: parentTab.webView?.title))") + print("NB -- 5.3 addPopupForParentTab parent webView url \(String(describing: parentTab.webView?.url))") // Configure the tab for the child popup webview. In this scenario we need to be sure to pass along // the specific `configuration` that we are given by the WKUIDelegate callback, since if we do not // use this configuration WebKit will throw an exception. From 82f313e735e95c6256c82d34d1f7f65c9edbe3cf Mon Sep 17 00:00:00 2001 From: Nishant Bhasin Date: Fri, 11 Oct 2024 18:19:57 -0400 Subject: [PATCH 2/2] Cleanup --- .../BrowserViewController+WebViewDelegates.swift | 9 --------- .../Client/TabManagement/Legacy/LegacyTabManager.swift | 6 ------ 2 files changed, 15 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift index 350dd7b1b5ba..070b00dbb148 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+WebViewDelegates.swift @@ -17,13 +17,6 @@ extension BrowserViewController: WKUIDelegate { for navigationAction: WKNavigationAction, windowFeatures: WKWindowFeatures ) -> WKWebView? { - print("NB -- 1 parent tab webview url \(String(describing: tabManager[webView]?.webView?.url))") - print("NB -- 1.1 parent tab webview title \(String(describing: tabManager[webView]?.webView?.title))") - print("NB -- 2 webView url \(String(describing: webView.url))") - print("NB -- 2.1 webView title \(String(describing: webView.title))") - print("NB -- 2.2 webView tag \(webView.tag)") - print("NB -- 3 navigationAction request \(navigationAction.request)") - print("NB -- 3.1 navigationAction request url \(String(describing: navigationAction.request.url))") guard let parentTab = tabManager[webView] else { return nil } guard !navigationAction.isInternalUnprivileged, shouldRequestBeOpenedAsPopup(navigationAction.request) @@ -531,8 +524,6 @@ extension BrowserViewController: WKNavigationDelegate { } let navigationHandler = WebViewNavigationHandlerImplementation(decisionHandler: decisionHandler) - print("NB -- 0 Data scheme \(url)") - print("NB -- 0 Data shouldFilterDataScheme \(navigationHandler.shouldFilterDataScheme(url: url))") if navigationHandler.shouldFilterDataScheme(url: url) { navigationHandler.filterDataScheme(url: url, navigationAction: navigationAction) return diff --git a/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift b/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift index 4cae17f0dc07..8849db4799bf 100644 --- a/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift +++ b/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift @@ -399,8 +399,6 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler zombie: Bool, isPrivate: Bool = false ) -> Tab { - print("NB -- 4 Add tab request request \(String(describing: request))") - print("NB -- 4.1 Add tab request url \(String(describing: request?.url))") let tab = Tab(profile: profile, isPrivate: isPrivate, windowUUID: windowUUID) configureTab(tab, request: request, afterTab: afterTab, flushToDisk: flushToDisk, zombie: zombie) return tab @@ -410,10 +408,6 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler let popup = Tab(profile: profile, isPrivate: parentTab.isPrivate, windowUUID: windowUUID) - print("NB -- 5 addPopupForParentTab popup webView title \(String(describing: popup.webView?.title))") - print("NB -- 5.1 addPopupForParentTab popup webView url \(String(describing: popup.webView?.url))") - print("NB -- 5.2 addPopupForParentTab parent webView title \(String(describing: parentTab.webView?.title))") - print("NB -- 5.3 addPopupForParentTab parent webView url \(String(describing: parentTab.webView?.url))") // Configure the tab for the child popup webview. In this scenario we need to be sure to pass along // the specific `configuration` that we are given by the WKUIDelegate callback, since if we do not // use this configuration WebKit will throw an exception.