Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #8647: Fix crash when loading invalid URLs (#8648)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brandon-T authored and soner-yuksel committed Jan 15, 2024
1 parent 3c9c257 commit 823a7ba
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ extension Tab: BraveWalletProviderDelegate {
// assigned for a specific origin, the provider(s) will check origin
// of all open Tab's to see if that provider needs(s) updated too.
// We can get the url from the SessionTab, and return it's origin.
if let sessionTabOrigin = SessionTab.from(tabId: id)?.url.origin {
if let sessionTabOrigin = SessionTab.from(tabId: id)?.url?.origin {
return sessionTabOrigin
}
assert(false, "We should have a valid origin to get to this point")
Expand Down
2 changes: 1 addition & 1 deletion Sources/Brave/Frontend/Browser/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ class Tab: NSObject {
} else {
if let tabUrl = url, tabUrl.isWebPage() {
return tabUrl
} else if let fetchedTab = SessionTab.from(tabId: id), fetchedTab.url.isWebPage() {
} else if let fetchedTab = SessionTab.from(tabId: id), fetchedTab.url?.isWebPage() == true {
return url
}
}
Expand Down
86 changes: 58 additions & 28 deletions Sources/Brave/Frontend/Browser/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ class TabManager: NSObject {

/// Internal url to access the new tab page.
static let ntpInteralURL = URL(string: "\(InternalURL.baseUrl)/\(AboutHomeHandler.path)#panel=0")!

/// When a URL is invalid and can't be restored or loaded, we display about:blank#blocked (same as on Desktop)
static let aboutBlankBlockedURL = URL(string: "about:blank")!

func addDelegate(_ delegate: TabManagerDelegate) {
assert(Thread.isMainThread)
Expand Down Expand Up @@ -513,9 +516,15 @@ class TabManager: NSObject {
@MainActor func configureTab(_ tab: Tab, request: URLRequest?, afterTab parent: Tab? = nil, flushToDisk: Bool, zombie: Bool, isPopup: Bool = false) {
assert(Thread.isMainThread)

var request = request
let isPrivate = tab.type == .private
let isPersistentTab = !isPrivate || (isPrivate && !Preferences.Privacy.privateBrowsingOnly.value && Preferences.Privacy.persistentPrivateBrowsing.value)

// WebKit can sometimes return a URL that isn't valid at all!
if let requestURL = request?.url, NSURL(idnString: requestURL.absoluteString) == nil {
request?.url = TabManager.aboutBlankBlockedURL
}

if isPersistentTab {
SessionTab.createIfNeeded(windowId: windowId,
tabId: tab.id,
Expand Down Expand Up @@ -901,30 +910,46 @@ class TabManager: NSObject {

var tabToSelect: Tab?
for savedTab in savedTabs {
let tabURL = savedTab.url
// Provide an empty request to prevent a new tab from loading the home screen
let request = InternalURL.isValid(url: tabURL) ?
PrivilegedRequest(url: tabURL) as URLRequest :
URLRequest(url: tabURL)
if let tabURL = savedTab.url {
// Provide an empty request to prevent a new tab from loading the home screen
let request = InternalURL.isValid(url: tabURL) ?
PrivilegedRequest(url: tabURL) as URLRequest :
URLRequest(url: tabURL)

let tab = addTab(request,
flushToDisk: false,
zombie: true,
id: savedTab.tabId,
isPrivate: savedTab.isPrivate)

tab.lastTitle = savedTab.title
tab.favicon = FaviconFetcher.getIconFromCache(for: tabURL) ?? Favicon.default
tab.setScreenshot(savedTab.screenshot)

Task { @MainActor in
tab.favicon = try await FaviconFetcher.loadIcon(url: tabURL, kind: .smallIcon, persistent: !tab.isPrivate)
let tab = addTab(request,
flushToDisk: false,
zombie: true,
id: savedTab.tabId,
isPrivate: savedTab.isPrivate)

tab.lastTitle = savedTab.title
tab.favicon = FaviconFetcher.getIconFromCache(for: tabURL) ?? Favicon.default
tab.setScreenshot(savedTab.screenshot)
}

// Do not select the private tab since we always restore to regular mode!
if savedTab.isSelected && !savedTab.isPrivate {
tabToSelect = tab

Task { @MainActor in
tab.favicon = try await FaviconFetcher.loadIcon(url: tabURL, kind: .smallIcon, persistent: !tab.isPrivate)
tab.setScreenshot(savedTab.screenshot)
}

// Do not select the private tab since we always restore to regular mode!
if savedTab.isSelected && !savedTab.isPrivate {
tabToSelect = tab
}
} else {
let tab = addTab(nil,
flushToDisk: false,
zombie: true,
id: savedTab.tabId,
isPrivate: savedTab.isPrivate)

tab.lastTitle = savedTab.title
tab.favicon = Favicon.default
tab.setScreenshot(savedTab.screenshot)

// Do not select the private tab since we always restore to regular mode!
if savedTab.isSelected && !savedTab.isPrivate {
tabToSelect = tab
}
}
}

Expand Down Expand Up @@ -969,12 +994,17 @@ class TabManager: NSObject {
if sessionTab.interactionState.isEmpty {
tab.navigationDelegate = navDelegate

let request = InternalURL.isValid(url: sessionTab.url) ?
PrivilegedRequest(url: sessionTab.url) as URLRequest :
URLRequest(url: sessionTab.url)

tab.restore(webView,
requestRestorationData: (sessionTab.url.absoluteDisplayString, request))
if let tabURL = sessionTab.url {
let request = InternalURL.isValid(url: tabURL) ?
PrivilegedRequest(url: tabURL) as URLRequest :
URLRequest(url: tabURL)

tab.restore(webView,
requestRestorationData: (tabURL.absoluteDisplayString, request))
} else {
tab.restore(webView,
requestRestorationData: (TabManager.aboutBlankBlockedURL.absoluteDisplayString, URLRequest(url: TabManager.aboutBlankBlockedURL)))
}
return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,10 @@ class AddEditBookmarkTableViewController: UITableViewController {
if let url = tab.url, url.isWebPage(), !(InternalURL(url)?.isAboutHomeURL ?? false) {
bookmarkManager.add(url: url, title: tab.title, parentFolder: parentFolder)
}
} else if let fetchedTab = SessionTab.from(tabId: tab.id) {
if fetchedTab.url.isWebPage(), !(InternalURL(fetchedTab.url)?.isAboutHomeURL ?? false) {
} else if let fetchedTab = SessionTab.from(tabId: tab.id), let tabURL = fetchedTab.url {
if tabURL.isWebPage(), !(InternalURL(tabURL)?.isAboutHomeURL ?? false) {
bookmarkManager.add(
url: fetchedTab.url,
url: tabURL,
title: fetchedTab.title,
parentFolder: parentFolder)
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/Brave/Migration/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ public class Migration {
// Restore private tabs if persistency is enabled
if Preferences.Privacy.persistentPrivateBrowsing.value {
zombieTabs.filter({ $0.isPrivate }).forEach {
if !activeURLs.contains($0.url) {
if let url = $0.url, !activeURLs.contains(url) {
SessionTab.move(tab: $0.tabId, toWindow: activeWindow.windowId)
}
}
}

// Restore regular tabs
zombieTabs.filter({ !$0.isPrivate }).forEach {
if !activeURLs.contains($0.url) {
if let url = $0.url, !activeURLs.contains(url) {
SessionTab.move(tab: $0.tabId, toWindow: activeWindow.windowId)
}
}
Expand Down
Loading

0 comments on commit 823a7ba

Please sign in to comment.