diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift index 1a084de4685c..856bf99b446f 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift @@ -280,7 +280,7 @@ class AddEditBookmarkTableViewController: UITableViewController { switch mode { case .addBookmark(_, _): guard let urlString = bookmarkDetailsView.urlTextField?.text, - let url = URL(string: urlString) ?? urlString.bookmarkletURL + let url = URL(string: urlString) ?? URL.bookmarkletURL(from: urlString) else { return earlyReturn() } @@ -317,7 +317,7 @@ class AddEditBookmarkTableViewController: UITableViewController { } case .editBookmark(let bookmark): guard let urlString = bookmarkDetailsView.urlTextField?.text, - let url = URL(string: urlString) ?? urlString.bookmarkletURL + let url = URL(string: urlString) ?? URL.bookmarkletURL(from: urlString) else { return earlyReturn() } @@ -358,7 +358,7 @@ class AddEditBookmarkTableViewController: UITableViewController { case .editFavorite(let favorite): guard let urlString = bookmarkDetailsView.urlTextField?.text, - let url = URL(string: urlString) ?? urlString.bookmarkletURL + let url = URL(string: urlString) ?? URL.bookmarkletURL(from: urlString) else { return earlyReturn() } diff --git a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkDetailsView.swift b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkDetailsView.swift index b970b88da504..1a4ee8d79834 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkDetailsView.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/BookmarkDetailsView.swift @@ -73,8 +73,8 @@ class BookmarkDetailsView: AddEditHeaderView, BookmarkFormFieldsProtocol { .forEach(contentStackView.addArrangedSubview) var url = url - if url?.isBookmarklet == true { - url = url?.removingPercentEncoding + if let urlString = url, URL.bookmarkletURL(from: urlString) != nil { + url = urlString.removingPercentEncoding } else if let url = url, let favUrl = URL(string: url) { faviconImageView.loadFavicon(siteURL: favUrl, isPrivateBrowsing: isPrivateBrowsing) } @@ -94,7 +94,7 @@ class BookmarkDetailsView: AddEditHeaderView, BookmarkFormFieldsProtocol { // MARK: - Delegate actions @objc func textFieldDidChange(_ textField: UITextField) { - if textField.text?.isBookmarklet == true { + if let text = textField.text, URL.bookmarkletURL(from: text) != nil { delegate?.correctValues(validationPassed: validateCodeFields()) } else { delegate?.correctValues(validationPassed: validateFields()) diff --git a/ios/brave-ios/Sources/Shared/BookmarkValidation.swift b/ios/brave-ios/Sources/Shared/BookmarkValidation.swift index b0d7c645c331..03200df2b9c4 100644 --- a/ios/brave-ios/Sources/Shared/BookmarkValidation.swift +++ b/ios/brave-ios/Sources/Shared/BookmarkValidation.swift @@ -18,8 +18,8 @@ public struct BookmarkValidation { public static func validateBookmarklet(title: String?, url: String?) -> Bool { guard let url = url else { return validateTitle(title) } - if !url.isBookmarklet { return false } - guard let javascriptCode = url.bookmarkletCodeComponent else { + guard let bookmarklet = URL.bookmarkletURL(from: url) else { return false } + guard let javascriptCode = bookmarklet.bookmarkletCodeComponent else { return false } diff --git a/ios/brave-ios/Sources/Shared/Extensions/URLExtensions.swift b/ios/brave-ios/Sources/Shared/Extensions/URLExtensions.swift index 3842e25dcfd2..f7ff8f1dd23f 100644 --- a/ios/brave-ios/Sources/Shared/Extensions/URLExtensions.swift +++ b/ios/brave-ios/Sources/Shared/Extensions/URLExtensions.swift @@ -412,38 +412,39 @@ extension URL { extension URL { public var isBookmarklet: Bool { - return self.absoluteString.isBookmarklet - } - - public var bookmarkletCodeComponent: String? { - return self.absoluteString.bookmarkletCodeComponent - } -} - -extension String { - public var isBookmarklet: Bool { - let url = self.lowercased() - return url.hasPrefix("javascript:") && !url.hasPrefix("javascript:/") + if let scheme = self.scheme { + return scheme == "javascript" + } + return self.absoluteString.hasPrefix("javascript:") } public var bookmarkletCodeComponent: String? { if self.isBookmarklet { - if let result = String(self.dropFirst("javascript:".count)).removingPercentEncoding { - return result.isEmpty ? nil : result + // Chrome, Safari, Desktop all treat anything after `javascript:` including `//` as the component! + // So `javascript:/` has a component of `/` + // `javascript://test` has a component of `//test` + if self.absoluteString.hasPrefix("javascript:") { + if let result = String(self.absoluteString.dropFirst("javascript:".count)) + .removingPercentEncoding + { + return result.isEmpty ? nil : result + } } } return nil } - public var bookmarkletURL: URL? { - if self.isBookmarklet, - let escaped = self.addingPercentEncoding(withAllowedCharacters: .urlAllowed) + public static func bookmarkletURL(from string: String) -> URL? { + if string.hasPrefix("javascript:"), + let escaped = string.addingPercentEncoding(withAllowedCharacters: .urlAllowed) { return URL(string: escaped) } return nil } +} +extension String { public func removeSchemeFromURLString(_ scheme: String?) -> String { guard let scheme = scheme else { return self diff --git a/ios/brave-ios/Tests/BraveSharedTests/NSURLExtensionsTests.swift b/ios/brave-ios/Tests/BraveSharedTests/NSURLExtensionsTests.swift index 0d6802943b4b..9f68c45e11ef 100644 --- a/ios/brave-ios/Tests/BraveSharedTests/NSURLExtensionsTests.swift +++ b/ios/brave-ios/Tests/BraveSharedTests/NSURLExtensionsTests.swift @@ -697,46 +697,52 @@ class NSURLExtensionsTests: XCTestCase { } func testIsBookmarkletURL() { - let goodURLs = [ + let javascriptURLs = [ "javascript:", + "javascript://", "javascript:void(window.close(self))", "javascript:window.open('https://brave.com')", ] - let badURLs = [ - "javascript:/", - "javascript://", - "javascript://something", + let nonJavascriptURLs = [ + "https://brave", + "https://brave.com", + "https://javascript://", + "javascript//", ] - goodURLs.forEach { - XCTAssertNotNil($0.bookmarkletURL) + javascriptURLs.forEach { + XCTAssertNotNil(URL.bookmarkletURL(from: $0)) } - badURLs.forEach { - XCTAssertNil($0.bookmarkletURL) + nonJavascriptURLs.forEach { + XCTAssertNil(URL.bookmarkletURL(from: $0)) } } func testIsBookmarkletURLComponent() { - let goodURLs = [ + let javascriptURLs = [ + "javascript:/", + "javascript://", + "javascript://something", "javascript:void(window.close(self))", "javascript:window.open('https://brave.com')", + "javascript://%0a%0dalert('UXSS')", ] let badURLs = [ "javascript:", - "javascript:/", - "javascript://", - "javascript://something", + "https://test", + "javascript//test", + "javascript/test", ] - goodURLs.forEach { - XCTAssertNotNil($0.bookmarkletCodeComponent) + javascriptURLs.forEach { + XCTAssertNotNil(URL.bookmarkletURL(from: $0)?.bookmarkletCodeComponent) } badURLs.forEach { - XCTAssertNil($0.bookmarkletCodeComponent) + XCTAssertNil(URL.bookmarkletURL(from: $0)?.bookmarkletCodeComponent) } }