Skip to content

Conversation

@graycreate
Copy link
Member

Summary

  • Sync HTTPCookieStorage cookies to WKWebView before loading URL
  • InAppBrowser now shares login state with the main app

Changes

Added syncCookiesAndLoad() method in WebViewHostController that:

  1. Gets all cookies from HTTPCookieStorage.shared
  2. Sets each cookie in WKWebsiteDataStore.httpCookieStore
  3. Uses DispatchGroup to ensure all cookies are set before loading the URL

Test plan

  • Login to V2EX in the app
  • Open an external V2EX link in the builtin browser
  • Verify that the user is still logged in within the WebView

🤖 Generated with Claude Code

Sync HTTPCookieStorage cookies to WKWebView before loading URL,
allowing the builtin browser to share login state with the main app.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copilot AI review requested due to automatic review settings December 28, 2025 13:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds cookie synchronization between the app's HTTPCookieStorage and WKWebView in the InAppBrowser to maintain user login state when opening V2EX links in the built-in browser.

Key changes:

  • Added syncCookiesAndLoad() method that syncs all cookies from HTTPCookieStorage to WKWebView before loading the URL
  • Modified viewDidAppear to call syncCookiesAndLoad() instead of directly loading the URL
  • Uses DispatchGroup to ensure all cookies are set before the URL request is made

Comment on lines +251 to +256
// Sync cookies from HTTPCookieStorage to WKWebView
let cookies = HTTPCookieStorage.shared.cookies ?? []
let cookieStore = webView.configuration.websiteDataStore.httpCookieStore

let group = DispatchGroup()
for cookie in cookies {
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All cookies from HTTPCookieStorage are being synced to the WebView without domain filtering. This is a security concern because the InAppBrowser can open external URLs (not just V2EX URLs, as seen in URLRouter's .external(url: url) case). This means V2EX login cookies could be exposed to arbitrary third-party websites.

Consider filtering cookies to only sync those whose domain matches the target URL's domain. For example, only sync cookies with domain matching "v2ex.com" when opening V2EX URLs.

Suggested change
// Sync cookies from HTTPCookieStorage to WKWebView
let cookies = HTTPCookieStorage.shared.cookies ?? []
let cookieStore = webView.configuration.websiteDataStore.httpCookieStore
let group = DispatchGroup()
for cookie in cookies {
// Sync only cookies whose domain matches the URL's host into WKWebView
guard let targetHost = url.host?.lowercased(), !targetHost.isEmpty else {
// If the URL has no host, just load it without syncing cookies
webView.load(URLRequest(url: url))
return
}
let allCookies = HTTPCookieStorage.shared.cookies ?? []
let matchingCookies = allCookies.filter { cookie in
let domain = cookie.domain.lowercased()
// Handle leading-dot cookie domains (e.g. ".v2ex.com")
if domain.hasPrefix(".") {
let trimmedDomain = String(domain.dropFirst())
return targetHost == trimmedDomain || targetHost.hasSuffix("." + trimmedDomain)
} else {
return targetHost == domain
}
}
// If there are no matching cookies, just load the URL directly
guard !matchingCookies.isEmpty else {
webView.load(URLRequest(url: url))
return
}
let cookieStore = webView.configuration.websiteDataStore.httpCookieStore
let group = DispatchGroup()
for cookie in matchingCookies {

Copilot uses AI. Check for mistakes.
Comment on lines +258 to +260
cookieStore.setCookie(cookie) {
group.leave()
}
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cookie syncing operation has no error handling. If setCookie fails for any reason, the failure is silently ignored. Consider logging errors or implementing retry logic to ensure critical authentication cookies are properly set before loading the URL.

Copilot uses AI. Check for mistakes.
Comment on lines +256 to +261
for cookie in cookies {
group.enter()
cookieStore.setCookie(cookie) {
group.leave()
}
}
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are many cookies in HTTPCookieStorage, syncing them all sequentially could cause a noticeable delay before the URL loads. This could impact user experience, especially if the user has cookies from many different domains. Consider only syncing cookies that are relevant to the target URL's domain to improve performance.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Code Coverage Report ❌

Current coverage: 31.84%

@graycreate graycreate merged commit 83eaed4 into main Dec 28, 2025
12 checks passed
@graycreate graycreate deleted the feature/inappbrowser-cookie-sharing branch December 28, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants