Fix #8253: Don't navigate to javascript:// URLs from the omnibox (#8456)

brave · Nov 21, 2023 · 05d8a61 · 05d8a61
1 parent ae18fa6
commit 05d8a61
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 62 deletions.
diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController.swift b/Sources/Brave/Frontend/Browser/BrowserViewController.swift
@@ -1597,7 +1597,15 @@ public class BrowserViewController: UIViewController {
     UIApplication.shared.shortcutItems = Preferences.Privacy.privateBrowsingOnly.value ? [privateTabItem, scanQRCodeItem] : [newTabItem, privateTabItem, scanQRCodeItem]
   }
 
-  func finishEditingAndSubmit(_ url: URL) {
+  /// The method that executes the url and make changes in UI to reset the toolbars
+  /// for urls coming from various sources
+  /// If url is bookmarklet check if it is coming from user defined source to decide whether to execute
+  /// using isUserDefinedURLNavigation
+  /// - Parameters:
+  ///   - url: The url submitted
+  ///   - isUserDefinedURLNavigation: Boolean for  determining if url navigation is done from user defined spot
+  ///     user defined spot like Favourites or Bookmarks
+  func finishEditingAndSubmit(_ url: URL, isUserDefinedURLNavigation: Bool = false) {
     if url.isBookmarklet {
       topToolbar.leaveOverlayMode()
 
@@ -1609,7 +1617,7 @@ public class BrowserViewController: UIViewController {
       // Disable any sort of privileged execution contexts
       // IE: The user must explicitly tap a bookmark they have saved.
       // Block all other contexts such as redirects, downloads, embed, linked, etc..
-      if let webView = tab.webView, let code = url.bookmarkletCodeComponent {
+      if isUserDefinedURLNavigation, let webView = tab.webView, let code = url.bookmarkletCodeComponent {
         webView.evaluateSafeJavaScript(
           functionName: code,
           contentWorld: .bookmarkletSandbox,
@@ -2967,15 +2975,15 @@ extension BrowserViewController: ToolbarUrlActionsDelegate {
   func openInNewTab(_ url: URL, isPrivate: Bool) {
     topToolbar.leaveOverlayMode()
 
-    select(url, action: .openInNewTab(isPrivate: isPrivate))
+    select(url, action: .openInNewTab(isPrivate: isPrivate), isUserDefinedURLNavigation: false)
   }
 
   func copy(_ url: URL) {
-    select(url, action: .copy)
+    select(url, action: .copy, isUserDefinedURLNavigation: false)
   }
 
   func share(_ url: URL) {
-    select(url, action: .share)
+    select(url, action: .share, isUserDefinedURLNavigation: false)
   }
 
   func batchOpen(_ urls: [URL]) {
@@ -2992,14 +3000,14 @@ extension BrowserViewController: ToolbarUrlActionsDelegate {
   }
 #endif
 
-  func select(url: URL) {
-    select(url, action: .openInCurrentTab)
+  func select(url: URL, isUserDefinedURLNavigation: Bool) {
+    select(url, action: .openInCurrentTab, isUserDefinedURLNavigation: isUserDefinedURLNavigation)
   }
 
-  private func select(_ url: URL, action: ToolbarURLAction) {
+  private func select(_ url: URL, action: ToolbarURLAction, isUserDefinedURLNavigation: Bool) {
     switch action {
     case .openInCurrentTab:
-      finishEditingAndSubmit(url)
+      finishEditingAndSubmit(url, isUserDefinedURLNavigation: isUserDefinedURLNavigation)
       updateURLBarWalletButton()
     case .openInNewTab(let isPrivate):
       let tab = tabManager.addTab(PrivilegedRequest(url: url) as URLRequest, afterTab: tabManager.selectedTab, isPrivate: isPrivate)
@@ -3048,11 +3056,7 @@ extension BrowserViewController: ToolbarUrlActionsDelegate {
 
 extension BrowserViewController: NewTabPageDelegate {
   func navigateToInput(_ input: String, inNewTab: Bool, switchingToPrivateMode: Bool) {
-    let isPrivate = privateBrowsingManager.isPrivateBrowsing || switchingToPrivateMode
-    if inNewTab {
-      tabManager.addTabAndSelect(isPrivate: isPrivate)
-    }
-    processAddressBar(text: input)
+    handleURLInput(input, inNewTab: inNewTab, switchingToPrivateMode: switchingToPrivateMode, isFavourite: false)
   }
 
   func handleFavoriteAction(favorite: Favorite, action: BookmarksAction) {
@@ -3062,18 +3066,20 @@ extension BrowserViewController: NewTabPageDelegate {
       if switchingToPrivateMode, Preferences.Privacy.privateBrowsingLock.value {
         self.askForLocalAuthentication { [weak self] success, error in
           if success {
-            self?.navigateToInput(
+            self?.handleURLInput(
               url,
               inNewTab: inNewTab,
-              switchingToPrivateMode: switchingToPrivateMode
+              switchingToPrivateMode: switchingToPrivateMode,
+              isFavourite: true
             )
           }
         }
       } else {
-        navigateToInput(
+        handleURLInput(
           url,
           inNewTab: inNewTab,
-          switchingToPrivateMode: switchingToPrivateMode
+          switchingToPrivateMode: switchingToPrivateMode,
+          isFavourite: true
         )
       }
     case .edited:
@@ -3096,6 +3102,18 @@ extension BrowserViewController: NewTabPageDelegate {
       self.present(editPopup, animated: true)
     }
   }
+
+  /// Handling url input action and passing down if input is launched from favourites
+  private func handleURLInput(_ input: String, inNewTab: Bool, switchingToPrivateMode: Bool, isFavourite: Bool ) {
+    let isPrivate = privateBrowsingManager.isPrivateBrowsing || switchingToPrivateMode
+    if inNewTab {
+      tabManager.addTabAndSelect(isPrivate: isPrivate)
+    }
+
+    // Used to determine url navigation coming from a bookmark
+    // And handle it differently under finishEditingAndSubmit for bookmarklets    
+    processAddressBar(text: input, isUserDefinedURLNavigation: isFavourite)
+  }
 
   func focusURLBar() {
     topToolbar.tabLocationViewDidTapLocation(topToolbar.locationView)

diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+BraveTalk.swift b/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+BraveTalk.swift
@@ -26,7 +26,7 @@ extension BrowserViewController {
           var components = URLComponents()
           components.host = currentHost
           components.scheme = url.scheme
-          self.select(url: components.url!)
+          self.select(url: components.url!, isUserDefinedURLNavigation: false)
         }
       }
     )

diff --git a/.../Brave/Frontend/Browser/BrowserViewController/BrowserViewController+ToolbarDelegate.swift b/.../Brave/Frontend/Browser/BrowserViewController/BrowserViewController+ToolbarDelegate.swift
@@ -285,10 +285,10 @@ extension BrowserViewController: TopToolbarDelegate {
     processAddressBar(text: text)
   }
 
-  func processAddressBar(text: String, isBraveSearchPromotion: Bool = false) {
+  func processAddressBar(text: String, isBraveSearchPromotion: Bool = false, isUserDefinedURLNavigation: Bool = false) {
     processAddressBarTask?.cancel()
     processAddressBarTask = Task { @MainActor in
-      if !isBraveSearchPromotion, await submitValidURL(text) {
+      if !isBraveSearchPromotion, await submitValidURL(text, isUserDefinedURLNavigation: isUserDefinedURLNavigation) {
         return
       } else {
         // We couldn't build a URL, so pass it on to the search engine.
@@ -301,41 +301,7 @@ extension BrowserViewController: TopToolbarDelegate {
     }
   }
 
-  @discardableResult
-  func handleIPFSSchemeURL(_ url: URL) -> Bool {
-    guard !privateBrowsingManager.isPrivateBrowsing else {
-      topToolbar.leaveOverlayMode()
-      if let errorPageHelper = tabManager.selectedTab?.getContentScript(name: ErrorPageHelper.scriptName) as? ErrorPageHelper, let webView = tabManager.selectedTab?.webView {
-        errorPageHelper.loadPage(IPFSErrorPageHandler.privateModeError, forUrl: url, inWebView: webView)
-      }
-      return true
-    }
-
-    guard let ipfsPref = Preferences.Wallet.Web3IPFSOption(rawValue: Preferences.Wallet.resolveIPFSResources.value) else {
-      return false
-    }
-
-    switch ipfsPref {
-    case .ask:
-      showIPFSInterstitialPage(originalURL: url)
-      return true
-    case .enabled:
-      if let resolvedUrl = braveCore.ipfsAPI.resolveGatewayUrl(for: url) {
-        finishEditingAndSubmit(resolvedUrl)
-        return true
-      }
-    case .disabled:
-      topToolbar.leaveOverlayMode()
-      if let errorPageHelper = tabManager.selectedTab?.getContentScript(name: ErrorPageHelper.scriptName) as? ErrorPageHelper, let webView = tabManager.selectedTab?.webView {
-        errorPageHelper.loadPage(IPFSErrorPageHandler.privateModeError, forUrl: url, inWebView: webView)
-      }
-      return true
-    }
-
-    return false
-  }
-
-  @MainActor func submitValidURL(_ text: String) async -> Bool {
+  @MainActor private func submitValidURL(_ text: String, isUserDefinedURLNavigation: Bool) async -> Bool {
     if let url = URL(string: text), url.isIPFSScheme {
       return handleIPFSSchemeURL(url)
     } else if let fixupURL = URIFixup.getURL(text) {
@@ -367,14 +333,50 @@ extension BrowserViewController: TopToolbarDelegate {
         }
 
         // The user entered a URL, so use it.
-        finishEditingAndSubmit(fixupURL)
+        // Determine if url navigation is done from favourites or bookmarks
+        // To handle bookmarklets properly
+        finishEditingAndSubmit(fixupURL, isUserDefinedURLNavigation: isUserDefinedURLNavigation)
         return true
       }
     }
 
     return false
   }
 
+  @discardableResult
+  func handleIPFSSchemeURL(_ url: URL) -> Bool {
+    guard !privateBrowsingManager.isPrivateBrowsing else {
+      topToolbar.leaveOverlayMode()
+      if let errorPageHelper = tabManager.selectedTab?.getContentScript(name: ErrorPageHelper.scriptName) as? ErrorPageHelper, let webView = tabManager.selectedTab?.webView {
+        errorPageHelper.loadPage(IPFSErrorPageHandler.privateModeError, forUrl: url, inWebView: webView)
+      }
+      return true
+    }
+
+    guard let ipfsPref = Preferences.Wallet.Web3IPFSOption(rawValue: Preferences.Wallet.resolveIPFSResources.value) else {
+      return false
+    }
+
+    switch ipfsPref {
+    case .ask:
+      showIPFSInterstitialPage(originalURL: url)
+      return true
+    case .enabled:
+      if let resolvedUrl = braveCore.ipfsAPI.resolveGatewayUrl(for: url) {
+        finishEditingAndSubmit(resolvedUrl)
+        return true
+      }
+    case .disabled:
+      topToolbar.leaveOverlayMode()
+      if let errorPageHelper = tabManager.selectedTab?.getContentScript(name: ErrorPageHelper.scriptName) as? ErrorPageHelper, let webView = tabManager.selectedTab?.webView {
+        errorPageHelper.loadPage(IPFSErrorPageHandler.privateModeError, forUrl: url, inWebView: webView)
+      }
+      return true
+    }
+
+    return false
+  }
+
   func submitSearchText(_ text: String, isBraveSearchPromotion: Bool = false) {
     var engine = profile.searchEngines.defaultEngine(forType: privateBrowsingManager.isPrivateBrowsing ? .privateMode : .standard)
 

diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+Wallet.swift b/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+Wallet.swift
@@ -135,7 +135,7 @@ extension BrowserViewController: BraveWalletDelegate {
       self.dismiss(animated: true)
     }
     if let url = tabManager.selectedTab?.url, InternalURL.isValid(url: url) {
-      select(url: destinationURL)
+      select(url: destinationURL, isUserDefinedURLNavigation: false)
     } else {
       _ = tabManager.addTabAndSelect(
         URLRequest(url: destinationURL),

diff --git a/...rave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarksViewController.swift b/...rave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarksViewController.swift
@@ -481,7 +481,7 @@ class BookmarksViewController: SiteTableViewController, ToolbarUrlActionsProtoco
               ActivityShortcutManager.shared.donateCustomIntent(for: .openBookmarks, with: url.absoluteString)
             }
 
-            self.toolbarUrlActionsDelegate?.select(url: url)
+            self.toolbarUrlActionsDelegate?.select(url: url, isUserDefinedURLNavigation: true)
           }
 
           if presentingViewController is MenuViewController {

diff --git a/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/HistoryViewController.swift b/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/HistoryViewController.swift
@@ -285,7 +285,7 @@ class HistoryViewController: SiteTableViewController, ToolbarUrlActionsProtocol
       }
 
       dismiss(animated: true) {
-        self.toolbarUrlActionsDelegate?.select(url: url)
+        self.toolbarUrlActionsDelegate?.select(url: url, isUserDefinedURLNavigation: false)
       }
     }
 

diff --git a/Sources/Brave/Frontend/Browser/Toolbars/ToolbarUrlActionsDelegate.swift b/Sources/Brave/Frontend/Browser/Toolbars/ToolbarUrlActionsDelegate.swift
@@ -10,5 +10,5 @@ protocol ToolbarUrlActionsDelegate: AnyObject {
   func copy(_ url: URL)
   func share(_ url: URL)
   func batchOpen(_ urls: [URL])
-  func select(url: URL)
+  func select(url: URL, isUserDefinedURLNavigation: Bool)
 }
diff --git a/Sources/Brave/Shortcuts/ActivityShortcutManager.swift b/Sources/Brave/Shortcuts/ActivityShortcutManager.swift
@@ -192,7 +192,7 @@ public class ActivityShortcutManager: NSObject {
       } else {
         let controller = NewsSettingsViewController(dataSource: bvc.feedDataSource, openURL: { url in
           bvc.dismiss(animated: true)
-          bvc.select(url: url)
+          bvc.select(url: url, isUserDefinedURLNavigation: false)
         })
         controller.viewDidDisappear = {
           if Preferences.Review.braveNewsCriteriaPassed.value {