Skip to content

Commit f778b5f

Browse files
Sebastian Glödeclaude
Sebastian Glöde
and
claude
committed
Address code review feedback
Fixes based on Copilot review comments: WKSchemeHandler.kt: - Remove unused HTTPMethod import - Add try-catch around interceptor call with proper cleanup in finally block - Fix task cancellation to call failTask before returning - Use mainDocumentURL heuristic for isForMainFrame detection - Add null-safety for NSURL.URLWithString and NSHTTPURLResponse - Fix Content-Type header precedence (mimeType takes priority) - Add thread-safety documentation comment WebView.ios.kt: - Use remember(navigator) to recreate handler when navigator changes - Add reserved schemes validation (http, https, file, ftp, about, data, javascript) - Improve documentation for customSchemes parameter WebRequestInterceptResult.kt: - Update documentation to clarify platform support (iOS only for now) - Document that mimeType takes precedence over Content-Type header Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f570b9b commit f778b5f

3 files changed

Lines changed: 102 additions & 38 deletions

File tree

‎webview/src/commonMain/kotlin/com/multiplatform/webview/request/WebRequestInterceptResult.kt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@ sealed interface WebRequestInterceptResult {
1616
* Respond with custom data instead of making a network request.
1717
* This allows implementing custom URL schemes or serving local content.
1818
*
19-
* Note: This result type requires a custom URL scheme handler to be registered
20-
* on iOS (via WKURLSchemeHandler) or Android (via shouldInterceptRequest).
19+
* Platform support:
20+
* - **iOS**: Supported via custom URL scheme handler registered with [WKURLSchemeHandler].
21+
* Use [PlatformWebViewParams.customSchemes] to register your custom schemes.
22+
* - **Android**: Not currently implemented. The Respond result will be logged as a warning
23+
* and the request will be rejected. Future implementation would use shouldInterceptRequest.
24+
* - **Desktop**: Not supported. The Respond result will be logged as a warning
25+
* and the request will be rejected.
2126
*
2227
* @param data The response body as a byte array
23-
* @param mimeType The MIME type of the response (e.g., "text/html", "application/json")
28+
* @param mimeType The MIME type of the response (e.g., "text/html", "application/json").
29+
* This takes precedence over any "Content-Type" header in [headers].
2430
* @param statusCode The HTTP status code (default: 200)
25-
* @param headers Optional response headers
31+
* @param headers Optional response headers. Note: "Content-Type" will be overridden by [mimeType].
2632
*/
2733
class Respond(
2834
val data: ByteArray,

‎webview/src/iosMain/kotlin/com/multiplatform/webview/request/WKSchemeHandler.kt‎

Lines changed: 62 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import kotlinx.cinterop.ExperimentalForeignApi
77
import kotlinx.cinterop.ObjCSignatureOverride
88
import kotlinx.cinterop.addressOf
99
import kotlinx.cinterop.usePinned
10-
import platform.Foundation.HTTPMethod
1110
import platform.Foundation.NSData
1211
import platform.Foundation.NSHTTPURLResponse
1312
import platform.Foundation.NSURL
1413
import platform.Foundation.allHTTPHeaderFields
1514
import platform.Foundation.create
15+
import platform.Foundation.mainDocumentURL
1616
import platform.WebKit.WKURLSchemeHandlerProtocol
1717
import platform.WebKit.WKURLSchemeTaskProtocol
1818
import platform.WebKit.WKWebView
@@ -22,6 +22,9 @@ import platform.darwin.NSObject
2222
* WKURLSchemeHandler implementation for custom URL schemes.
2323
* This allows intercepting requests with custom schemes (e.g., "app://", "local://")
2424
* and providing custom responses.
25+
*
26+
* Note: WKURLSchemeHandler methods are called on the main thread by WebKit,
27+
* so the activeTasks map access is thread-safe in this context.
2528
*/
2629
@OptIn(ExperimentalForeignApi::class, BetaInteropApi::class)
2730
class WKSchemeHandler(
@@ -46,10 +49,15 @@ class WKSchemeHandler(
4649
headerMap[it.key.toString()] = it.value.toString()
4750
}
4851

52+
// WKURLSchemeTaskProtocol does not expose frame info directly.
53+
// Use mainDocumentURL as a heuristic: if the request URL matches mainDocumentURL
54+
// (or mainDocumentURL is null), treat it as main frame.
55+
val isForMainFrame = request.mainDocumentURL == null || request.mainDocumentURL == request.URL
56+
4957
val webRequest = WebRequest(
5058
url = url,
5159
headers = headerMap,
52-
isForMainFrame = true,
60+
isForMainFrame = isForMainFrame,
5361
isRedirect = false,
5462
method = request.HTTPMethod ?: "GET",
5563
)
@@ -63,30 +71,37 @@ class WKSchemeHandler(
6371
return
6472
}
6573

66-
// Call the interceptor
67-
val result = interceptor.onInterceptUrlRequest(webRequest, navigator)
68-
69-
// Check if task was cancelled
70-
if (activeTasks[taskId] != true) {
71-
KLogger.info { "WKSchemeHandler: Task was cancelled" }
72-
return
73-
}
74-
75-
when (result) {
76-
is WebRequestInterceptResult.Respond -> {
77-
respondWithData(startURLSchemeTask, result, url)
78-
}
79-
is WebRequestInterceptResult.Reject -> {
80-
failTask(startURLSchemeTask, "Request rejected by interceptor")
74+
try {
75+
// Call the interceptor
76+
val result = interceptor.onInterceptUrlRequest(webRequest, navigator)
77+
78+
// Check if task was cancelled
79+
if (activeTasks[taskId] != true) {
80+
KLogger.info { "WKSchemeHandler: Task was cancelled" }
81+
failTask(startURLSchemeTask, "Task was cancelled")
82+
activeTasks.remove(taskId)
83+
return
8184
}
82-
else -> {
83-
// For Allow and Modify, we can't actually make the request
84-
// because this is a custom scheme. Return an error.
85-
failTask(startURLSchemeTask, "Custom scheme requires Respond result")
85+
86+
when (result) {
87+
is WebRequestInterceptResult.Respond -> {
88+
respondWithData(startURLSchemeTask, result, url)
89+
}
90+
is WebRequestInterceptResult.Reject -> {
91+
failTask(startURLSchemeTask, "Request rejected by interceptor")
92+
}
93+
else -> {
94+
// For Allow and Modify, we can't actually make the request
95+
// because this is a custom scheme. Return an error.
96+
failTask(startURLSchemeTask, "Custom scheme requires Respond result")
97+
}
8698
}
99+
} catch (e: Exception) {
100+
KLogger.e { "WKSchemeHandler: Exception in request interceptor: ${e.message}" }
101+
failTask(startURLSchemeTask, "Request interceptor threw an exception: ${e.message}")
102+
} finally {
103+
activeTasks.remove(taskId)
87104
}
88-
89-
activeTasks.remove(taskId)
90105
}
91106

92107
@ObjCSignatureOverride
@@ -102,24 +117,43 @@ class WKSchemeHandler(
102117
url: String,
103118
) {
104119
try {
120+
// Validate URL
121+
val nsUrl = NSURL.URLWithString(url)
122+
if (nsUrl == null) {
123+
val message = "WKSchemeHandler: Invalid URL: $url"
124+
KLogger.e { message }
125+
failTask(task, message)
126+
return
127+
}
128+
105129
// Build response headers
130+
// Add custom headers first, then set Content-Type from mimeType to ensure
131+
// mimeType takes precedence over any Content-Type in headers
106132
val headerFields = mutableMapOf<Any?, Any?>()
107-
headerFields["Content-Type"] = result.mimeType
108-
headerFields["Content-Length"] = result.data.size.toString()
109133
result.headers.forEach { (key, value) ->
110-
headerFields[key] = value
134+
// Skip Content-Type from headers - we use result.mimeType instead
135+
if (!key.equals("Content-Type", ignoreCase = true)) {
136+
headerFields[key] = value
137+
}
111138
}
139+
headerFields["Content-Type"] = result.mimeType
140+
headerFields["Content-Length"] = result.data.size.toString()
112141

113142
// Create HTTP response
114143
val response = NSHTTPURLResponse(
115-
uRL = NSURL.URLWithString(url)!!,
144+
uRL = nsUrl,
116145
statusCode = result.statusCode.toLong(),
117146
HTTPVersion = "HTTP/1.1",
118147
headerFields = headerFields,
119148
)
120149

150+
if (response == null) {
151+
failTask(task, "Failed to create HTTP response")
152+
return
153+
}
154+
121155
// Send response
122-
task.didReceiveResponse(response!!)
156+
task.didReceiveResponse(response)
123157

124158
// Send data
125159
if (result.data.isNotEmpty()) {

‎webview/src/iosMain/kotlin/com/multiplatform/webview/web/WebView.ios.kt‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,19 @@ actual data class WebViewFactoryParam(
6262
/**
6363
* iOS-specific WebView parameters.
6464
*
65-
* @param customSchemes List of custom URL schemes to register (e.g., "app", "local").
65+
* @param customSchemes List of custom URL schemes to register at WebView creation time
66+
* (for example, "app", "local"). These schemes are added to the
67+
* underlying [WKWebViewConfiguration] when the WebView is created
68+
* and cannot be added to or removed from an existing WebView instance.
69+
*
6670
* Requests to these schemes will be handled by the RequestInterceptor,
67-
* which should return WebRequestInterceptResult.Respond with the response data.
71+
* which should return [WebRequestInterceptResult.Respond] with the
72+
* response data.
73+
*
74+
* Note: WKWebView does not allow certain built-in schemes such as
75+
* "http", "https", "file", "ftp", "about", "data", or "javascript"
76+
* to be used as custom schemes. These reserved schemes will be
77+
* automatically filtered out and not registered.
6878
*/
6979
actual class PlatformWebViewParams(
7080
val customSchemes: List<String> = emptyList(),
@@ -102,7 +112,8 @@ fun IOSWebView(
102112
)
103113
}
104114
val navigationDelegate = remember { WKNavigationDelegate(state, navigator) }
105-
val schemeHandler = remember { WKSchemeHandler(navigator) }
115+
// Recreate scheme handler if navigator changes to avoid stale state
116+
val schemeHandler = remember(navigator) { WKSchemeHandler(navigator) }
106117
val scope = rememberCoroutineScope()
107118

108119
UIKitView(
@@ -131,9 +142,22 @@ fun IOSWebView(
131142
)
132143

133144
// Register custom URL scheme handlers
134-
platformWebViewParams?.customSchemes?.forEach { scheme ->
135-
setURLSchemeHandler(schemeHandler, forURLScheme = scheme)
136-
}
145+
// Filter out reserved schemes that WKWebView doesn't allow
146+
val reservedSchemes = setOf(
147+
"http", "https", "file", "ftp", "about", "data", "javascript"
148+
)
149+
platformWebViewParams?.customSchemes
150+
?.filter { scheme ->
151+
val normalized = scheme.lowercase()
152+
val isReserved = normalized in reservedSchemes
153+
if (isReserved) {
154+
println("WKWebView: Skipping registration of reserved URL scheme: $scheme")
155+
}
156+
!isReserved
157+
}
158+
?.forEach { scheme ->
159+
setURLSchemeHandler(schemeHandler, forURLScheme = scheme)
160+
}
137161
}
138162
factory(WebViewFactoryParam(config))
139163
.apply {

0 commit comments

Comments
 (0)