From dd57a1673ecea75f6884bf41e8147d94ca995185 Mon Sep 17 00:00:00 2001 From: Soner YUKSEL Date: Wed, 12 Jul 2023 14:31:00 -0400 Subject: [PATCH] Fix #7702: OpenSearch issues - [hackerone 2057565] (#7721) --- .../BrowserViewController+OpenSearch.swift | 28 +++++++++- .../Frontend/Browser/Search/OpenSearch.swift | 2 +- .../Search/SearchSuggestionDataSource.swift | 6 ++ .../Browser/ThirdPartySearchAlerts.swift | 56 +++++++++++++++---- Sources/BraveStrings/BraveStrings.swift | 24 ++++++++ 5 files changed, 101 insertions(+), 15 deletions(-) diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController/OpenSearch/BrowserViewController+OpenSearch.swift b/Sources/Brave/Frontend/Browser/BrowserViewController/OpenSearch/BrowserViewController+OpenSearch.swift index c03056e2c35..e5f4f05a0bf 100644 --- a/Sources/Brave/Frontend/Browser/BrowserViewController/OpenSearch/BrowserViewController+OpenSearch.swift +++ b/Sources/Brave/Frontend/Browser/BrowserViewController/OpenSearch/BrowserViewController+OpenSearch.swift @@ -183,7 +183,30 @@ extension BrowserViewController { } private func addSearchEngine(_ engine: OpenSearchEngine) { - let alert = ThirdPartySearchAlerts.addThirdPartySearchEngine(engine) { alertAction in + var customEngineAlert: UIAlertController + + // Checking existance of search engine with same name + if let existingEngine = profile.searchEngines.orderedEngines.first(where: { $0.shortName.lowercased() == engine.shortName.lowercased() }) { + customEngineAlert = ThirdPartySearchAlerts.engineAlreadyExists(existingEngine) + present(customEngineAlert, animated: true) + return + } + + // Checking Search Template is a secure URL + if let searchTemplateURL = URL(string: engine.searchTemplate), !searchTemplateURL.isSecureWebPage() { + customEngineAlert = ThirdPartySearchAlerts.insecureSearchTemplateURL(engine) + present(customEngineAlert, animated: true) + return + } + + // Checking Suggest Template is a secure URL + if let suggestTemplate = engine.suggestTemplate, let suggestTemplateURL = URL(string: suggestTemplate), !suggestTemplateURL.isSecureWebPage() { + customEngineAlert = ThirdPartySearchAlerts.insecureSearchTemplateURL(engine) + present(customEngineAlert, animated: true) + return + } + + customEngineAlert = ThirdPartySearchAlerts.addThirdPartySearchEngine(engine) { alertAction in if alertAction.style == .cancel { return } @@ -203,7 +226,6 @@ extension BrowserViewController { } } } - - self.present(alert, animated: true, completion: {}) + present(customEngineAlert, animated: true) } } diff --git a/Sources/Brave/Frontend/Browser/Search/OpenSearch.swift b/Sources/Brave/Frontend/Browser/Search/OpenSearch.swift index 8c8a7ff31f1..ea4aca19075 100644 --- a/Sources/Brave/Frontend/Browser/Search/OpenSearch.swift +++ b/Sources/Brave/Frontend/Browser/Search/OpenSearch.swift @@ -55,7 +55,7 @@ class OpenSearchEngine: NSObject, NSSecureCoding { let image: UIImage let isCustomEngine: Bool let searchTemplate: String - fileprivate let suggestTemplate: String? + let suggestTemplate: String? fileprivate let SearchTermComponent = "{searchTerms}" fileprivate let LocaleTermComponent = "{moz:locale}" diff --git a/Sources/Brave/Frontend/Browser/Search/SearchSuggestionDataSource.swift b/Sources/Brave/Frontend/Browser/Search/SearchSuggestionDataSource.swift index 90e9101d02f..146ec5160bb 100644 --- a/Sources/Brave/Frontend/Browser/Search/SearchSuggestionDataSource.swift +++ b/Sources/Brave/Frontend/Browser/Search/SearchSuggestionDataSource.swift @@ -108,6 +108,12 @@ class SearchSuggestionDataSource { } func querySuggestClient() { + // Do not query suggestions if user is not opt_ed in + if !Preferences.Search.shouldShowSuggestionsOptIn.value { + Logger.module.info("Suggestions are not enabled") + return + } + cancelPendingSuggestionsRequests() let localSearchQuery = searchQuery.lowercased() diff --git a/Sources/Brave/Frontend/Browser/ThirdPartySearchAlerts.swift b/Sources/Brave/Frontend/Browser/ThirdPartySearchAlerts.swift index a9357e8c620..4c1df311ced 100644 --- a/Sources/Brave/Frontend/Browser/ThirdPartySearchAlerts.swift +++ b/Sources/Brave/Frontend/Browser/ThirdPartySearchAlerts.swift @@ -16,19 +16,20 @@ class ThirdPartySearchAlerts: UIAlertController { } /** - Builds the Alert view that asks if the users wants to add a third party search engine. + Builds the Alert view that asks if the users wants to add a third party search engine. - - parameter engine: To add engine details to alert - - - parameter completion: Okay option handler. - - - returns: UIAlertController for asking the user to add a search engine - **/ + - parameter engine: To add engine details to alert + - parameter completion: Okay option handler. + - returns: UIAlertController for asking the user to add a search engine + **/ static func addThirdPartySearchEngine(_ engine: OpenSearchEngine, completion: @escaping (UIAlertAction) -> Void) -> UIAlertController { let alertMessage = """ \n\(engine.displayName) + \n\(Strings.CustomSearchEngine.searchTemplateTitle) \(engine.searchTemplate) + \n\(Strings.CustomSearchEngine.suggestionTemplateTitle) + \(engine.suggestTemplate ?? "N/A") \n\(Strings.CustomSearchEngine.thirdPartySearchEngineAddAlertDescription) """ let alert = ThirdPartySearchAlerts( @@ -54,12 +55,45 @@ class ThirdPartySearchAlerts: UIAlertController { return alert } + + static func insecureSearchTemplateURL(_ engine: OpenSearchEngine) -> UIAlertController { + let alertMessage = """ + \n\(Strings.CustomSearchEngine.insecureSearchTemplateURLErrorDescription)" + \(engine.displayName) + \n\(Strings.CustomSearchEngine.searchTemplateTitle) + \(engine.searchTemplate) + """ + return searchAlertWithOK( + title: Strings.CustomSearchEngine.customSearchEngineAddErrorTitle, + message: alertMessage) + } + + static func engineAlreadyExists(_ engine: OpenSearchEngine) -> UIAlertController { + let alertMessage = """ + \n\(engine.displayName) + \n\(Strings.CustomSearchEngine.engineExistsAlertDescription) + """ + return searchAlertWithOK( + title: Strings.CustomSearchEngine.customSearchEngineAddErrorTitle, + message: alertMessage) + } + + static func insecureSuggestionTemplateURL(_ engine: OpenSearchEngine) -> UIAlertController { + let alertMessage = """ + \n\(Strings.CustomSearchEngine.insecureSuggestionTemplateURLErrorDescription) + \(engine.displayName) + \n\(Strings.CustomSearchEngine.suggestionTemplateTitle) + \(engine.suggestTemplate ?? "") + """ + return searchAlertWithOK( + title: Strings.CustomSearchEngine.customSearchEngineAddErrorTitle, + message: alertMessage) + } /** - Builds the Alert view that shows the user an error in case a search engine could not be added. - - - returns: UIAlertController with an error dialog - **/ + Builds the Alert view that shows the user an error in case a search engine could not be added. + - returns: UIAlertController with an error dialog + **/ static func failedToAddThirdPartySearch() -> UIAlertController { return searchAlertWithOK( diff --git a/Sources/BraveStrings/BraveStrings.swift b/Sources/BraveStrings/BraveStrings.swift index bcd1f47090e..d359cfa7fc6 100644 --- a/Sources/BraveStrings/BraveStrings.swift +++ b/Sources/BraveStrings/BraveStrings.swift @@ -608,6 +608,30 @@ extension Strings { public static let deleteEngineAlertDescription = NSLocalizedString("customSearchEngine.deleteEngineAlertDescription", tableName: "BraveShared", bundle: .module, value: "Deleting a custom search engine while it is default will switch default engine automatically.", comment: "The warning description shown to user when custom search engine will be deleted while it is default search engine.") + + public static let customSearchEngineAddErrorTitle = NSLocalizedString("customSearchEngine.customSearchEngineAddErrorTitle", tableName: "BraveShared", bundle: .module, + value: "Error Adding Custom Search Engine", + comment: "A title explaining that an error shown while adding custom search engine") + + public static let insecureSearchTemplateURLErrorDescription = NSLocalizedString("customSearchEngine.insecureSearchTemplateURLErrorDescription", tableName: "BraveShared", bundle: .module, + value: "Insecure Custom Search Template for", + comment: "A description explaining that search template url is insecure, it is used for instance - Insecure Custom Search Template for Brave Search, Brave Search is a search engineand on a new seperate line") + + public static let insecureSuggestionTemplateURLErrorDescription = NSLocalizedString("customSearchEngine.insecureSuggestionTemplateURLErrorDescription", tableName: "BraveShared", bundle: .module, + value: "Insecure Custom Suggestion Template for", + comment: "A description explaining that suggestion template url is insecure, it is used for instance - Insecure Custom Suggestion Template for Brave Search, Brave Search is name of search engine on a new seperate line") + + public static let searchTemplateTitle = NSLocalizedString("customSearchEngine.searchTemplateTitle", tableName: "BraveShared", bundle: .module, + value: "Search Template:", + comment: "Search Template title - for instance it will be used Search Template: Brave Search - Brave Search is the name of Search Engine on seperate line") + + public static let suggestionTemplateTitle = NSLocalizedString("customSearchEngine.suggestionTemplateTitle", tableName: "BraveShared", bundle: .module, + value: "Suggestion Template:", + comment: "Suggestion Template title - for instance it will be used Suggestion Template: Brave Search - Brave Search is the name of Search Engine on seperate line") + + public static let engineExistsAlertDescription = NSLocalizedString("customSearchEngine.engineAlertExistsAlertDescription", tableName: "BraveShared", bundle: .module, + value: "A search engine with the same name already exists.", + comment: "The warning description shown to user when custom search engine already exists.") } }