<rdar://85460617>
Created attachment 447389 [details] Patch v1.0
Comment on attachment 447389 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=447389&action=review I think this should have an availability annotation to prevent use on macOS after Big Sur. Something like this from _WKAttachment.h: - (void)requestInfo:(void(^)(_WKAttachmentInfo * _Nullable, NSError * _Nullable))completionHandler WK_API_DEPRECATED_WITH_REPLACEMENT("-info", macos(10.14, 10.14.4), ios(12.0, 12.2)); > Source/WebKit/ChangeLog:12 > + automationmation clients when using the `acceptInsecureCerts` capability. Nit: typo
Comment on attachment 447389 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=447389&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:671 > + _websiteDataStore->setAllowsAnySSLCertificateForWebSocket(allow); I've made a mistake here. This ends up enabling/disabling insecure certs globally, not scoped to a network session. I need to reengineer this.
Created attachment 447481 [details] Patch v2.0
Comment on attachment 447481 [details] Patch v2.0 View in context: https://bugs.webkit.org/attachment.cgi?id=447481&action=review Not a full review, but one style comment (please other reviewers, don’t hesitate to review) > Source/WebKit/NetworkProcess/NetworkSession.h:210 > + bool acceptInsecureCertificatesForWebSockets() const { return m_acceptInsecureCertificatesForWebSockets; } Often we steer away from names like this for getters, because they are ambiguous. This could be a function telling the network session that it should accept certificates. And a caller that thought it would do that won’t even get a compile time error. An alternative is to name such functions shouldAccept... or some other non-verb phrase, a style we have borrowed from Apple’s Cocoa naming conventions. > Source/WebKit/UIProcess/API/Cocoa/_WKWebsiteDataStoreConfiguration.h:58 > +@property (nonatomic, setter=_setAcceptInsecureCertificatesForWebSockets:) BOOL _acceptInsecureCertificatesForWebSockets WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); Same comment for this public property. Note for example, for above it says "shouldRunServiceWorkersOnMainThreadForTesting.". > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStoreConfiguration.h:201 > + bool acceptInsecureCertificatesForWebSockets() const { return m_acceptInsecureCertificatesForWebSockets; } Here too.
Comment on attachment 447481 [details] Patch v2.0 Oops, looks like I missed that we have three files named `SocketStreamHandleImpl.h`, and so platforms that use soup or curl backed sockets are failing to build.
Created attachment 447492 [details] Patch v2.1 - Implement naming suggestion, fix non-Cocoa builds
Comment on attachment 447492 [details] Patch v2.1 - Implement naming suggestion, fix non-Cocoa builds r=me
Committed r287781 (245841@main): <https://commits.webkit.org/245841@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447492 [details].