-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[webview_flutter_wkwebview] Fixes loadFlutterAsset
exception and updates native wrapper for SecTrust
and SecCertificate
#9016
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
Conversation
loadFlutterAsset
exception and updates native wrapper for SecTrust
and SecCertificate
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.
For future reference, I ran this in a terminal to generate this file
# Generate a private key
openssl genpkey -algorithm RSA -out test_key.pem -pkeyopt rsa_keygen_bits:2048
# Generate a Certificate Signing Request (CSR) - fill in dummy info
openssl req -new -key test_key.pem -out test_csr.pem
# Generate a self-signed certificate (e.g., valid for 5000 days) and export to DER format
openssl x509 -req -days 5000 -in test_csr.pem -signkey test_key.pem -out test_cert.der -outform DER
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.
LGTM with small comments.
|
||
var url: URL? = bundle.url( | ||
forResource: (assetFilePath! as NSString).deletingPathExtension, | ||
withExtension: (assetFilePath! as NSString).pathExtension) |
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.
If you use guard
for the nil check on line 27, can't you avoid the force unwrap here and below?
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.
I tried changing line 27 to:
guard assetFilePath != nil else {
return nil
}
and I still needed the force unwrap here. Is this what you were thinking?
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.
I'm not @stuartmorgan-g, but you could write it as
guard let assetFilePath else {
return nil
}
and then you can use the variable assetFilePath
without force unwrapping it.
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.
For the versions of Swift we support it's still guard let assetFilePath = assetFilePath else
.
...iew_flutter_wkwebview/Sources/webview_flutter_wkwebview/SecCertificateProxyAPIDelegate.swift
Outdated
Show resolved
Hide resolved
...n/webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/SecTrustProxyAPIDelegate.swift
Outdated
Show resolved
Hide resolved
...webview_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart
Outdated
Show resolved
Hide resolved
9aa83d9
to
f0f4579
Compare
…ion and updates native wrapper for `SecTrust` and `SecCertificate` (flutter/packages#9016)
Adds support to the native wrapper to handle
SecTrust
andSecCertificate
.This is a part of landing #7893 by splitting of the native wrapper implementation.
Also fixes flutter/flutter#162938 and adds an integration test for
loadFlutterAsset
.Pre-Review Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3