-
Notifications
You must be signed in to change notification settings - Fork 405
Block external domain navigation and handle 404 errors in webview #4137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6659ad9
346a769
17ba48f
076b5c2
af6c400
b00f6d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1288,6 +1288,47 @@ extension WebViewController { | |
| } | ||
| } | ||
|
|
||
| func webView( | ||
| _ webView: WKWebView, | ||
| decidePolicyFor navigationAction: WKNavigationAction, | ||
| decisionHandler: @escaping (WKNavigationActionPolicy) -> Void | ||
| ) { | ||
| // Only check domain for user-initiated link clicks | ||
| guard navigationAction.navigationType == .linkActivated else { | ||
| // Allow all other navigation types (back/forward, reload, programmatic, etc.) | ||
| decisionHandler(.allow) | ||
| return | ||
| } | ||
|
|
||
| guard let targetURL = navigationAction.request.url else { | ||
| decisionHandler(.allow) | ||
| return | ||
| } | ||
|
|
||
| // Allow special schemes like about:blank | ||
| if let scheme = targetURL.scheme?.lowercased(), ["about", "file"].contains(scheme) { | ||
| decisionHandler(.allow) | ||
| return | ||
| } | ||
|
|
||
| // Check if the target URL belongs to the active server domain | ||
| guard let activeURL = server.info.connection.activeURL() else { | ||
| // If there's no active URL, allow navigation (let other error handling deal with it) | ||
| decisionHandler(.allow) | ||
| return | ||
| } | ||
|
|
||
| // Allow navigation within the same domain | ||
| if targetURL.baseIsEqual(to: activeURL) { | ||
| decisionHandler(.allow) | ||
| } else { | ||
| // URL is outside the active domain - cancel and show alert | ||
| Current.Log.warning("Navigation blocked: URL \(targetURL) is outside active domain \(activeURL)") | ||
| decisionHandler(.cancel) | ||
| showNavigationErrorAlert() | ||
| } | ||
| } | ||
|
|
||
| func webView( | ||
| _ webView: WKWebView, | ||
| decidePolicyFor navigationResponse: WKNavigationResponse, | ||
|
|
@@ -1311,8 +1352,13 @@ extension WebViewController { | |
|
|
||
| // error response, let's inspect if it's restoring a page or normal navigation | ||
| if navigationResponse.response.url != initialURL { | ||
| // just a normal loading error | ||
| decisionHandler(.allow) | ||
| // Normal loading error (not initial URL restoration) | ||
| // Cancel the navigation, go back if possible, and show alert | ||
| decisionHandler(.cancel) | ||
| if webView.canGoBack { | ||
| webView.goBack() | ||
| } | ||
| showNavigationErrorAlert() | ||
|
Comment on lines
+1355
to
+1361
|
||
| } else { | ||
| // first: clear that saved url, it's bad | ||
| initialURL = nil | ||
|
|
@@ -1329,6 +1375,24 @@ extension WebViewController { | |
| } | ||
| } | ||
|
|
||
| private func showNavigationErrorAlert() { | ||
| let alert = UIAlertController( | ||
| title: L10n.Alerts.NavigationError.title, | ||
| message: L10n.Alerts.NavigationError.message, | ||
| preferredStyle: .alert | ||
| ) | ||
| alert.addAction(.init(title: L10n.okLabel, style: .default)) | ||
|
|
||
| DispatchQueue.main.async { [weak self] in | ||
| guard let self else { return } | ||
| if presentedViewController != nil { | ||
| Current.Log.info("Navigation error alert not shown because another view is already presented") | ||
| return | ||
| } | ||
| present(alert, animated: true) | ||
| } | ||
|
Comment on lines
+1386
to
+1393
|
||
| } | ||
|
|
||
| // WKUIDelegate | ||
| func webView( | ||
| _ webView: WKWebView, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message combines two distinct error scenarios (external domain navigation and 404 errors) into a single message. This could be confusing for users since they describe different problems with different causes. Consider either: