refactor: Define a protocol for scheme handling plugins (apache#1479)

dpogue · web-flow · commit c3d5949de0ba · 2024-08-28T02:23:07.000-07:00
We don't rely on the protocol itself being implemented by the plugins (we continue to check with `-respondsToSelector:`) but this allows us to avoid `objc_msgSend` and provides a way to document some of this plugin behaviour that is not otherwise explained. This should also resolve the unsafe plugin iteration issue that was mentioned in apacheGH-1272 and apacheGH-1030 by always iterating over an array of plugin objects that is a copy (due to calling `-allValues`).
diff --git a/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVURLSchemeHandler.h b/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVURLSchemeHandler.h
@@ -17,19 +17,14 @@
  under the License.
  */
 
-#import <Foundation/Foundation.h>
 #import <WebKit/WebKit.h>
-#import <Cordova/CDVViewController.h>
-#import <Cordova/CDVPlugin.h>
 
+@class CDVViewController;
 
 @interface CDVURLSchemeHandler : NSObject <WKURLSchemeHandler>
+NS_ASSUME_NONNULL_BEGIN
 
-@property (nonatomic, weak) CDVViewController* viewController;
-
-@property (nonatomic) CDVPlugin* schemePlugin;
-
-- (instancetype)initWithVC:(CDVViewController *)controller;
-
+- (instancetype)initWithViewController:(CDVViewController *)controller;
 
+NS_ASSUME_NONNULL_END
 @end
diff --git a/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVURLSchemeHandler.m b/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVURLSchemeHandler.m
@@ -19,99 +19,104 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 
 #import "CDVURLSchemeHandler.h"
+#import <Cordova/CDVViewController.h>
+#import <Cordova/CDVPlugin.h>
+#import <Foundation/Foundation.h>
 #import <MobileCoreServices/MobileCoreServices.h>
 
-#import <objc/message.h>
+@interface CDVURLSchemeHandler ()
 
-@implementation CDVURLSchemeHandler
+@property (nonatomic, weak) CDVViewController *viewController;
+@property (nonatomic) NSMapTable <id <WKURLSchemeTask>, CDVPlugin <CDVPluginSchemeHandler> *> *handlerMap;
+
+@end
 
+@implementation CDVURLSchemeHandler
 
-- (instancetype)initWithVC:(CDVViewController *)controller
+- (instancetype)initWithViewController:(CDVViewController *)controller
 {
     self = [super init];
     if (self) {
         _viewController = controller;
+        _handlerMap = [NSMapTable weakToWeakObjectsMapTable];
     }
     return self;
 }
 
 - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)urlSchemeTask
 {
+    // Give plugins the chance to handle the url
+    for (CDVPlugin *plugin in self.viewController.enumerablePlugins) {
+        if ([plugin respondsToSelector:@selector(overrideSchemeTask:)]) {
+            CDVPlugin <CDVPluginSchemeHandler> *schemePlugin = (CDVPlugin<CDVPluginSchemeHandler> *)plugin;
+            if ([schemePlugin overrideSchemeTask:urlSchemeTask]) {
+                // Store the plugin that is handling this particular request
+                [self.handlerMap setObject:schemePlugin forKey:urlSchemeTask];
+                return;
+            }
+        }
+    }
+
+    // Indicate that we are handling this task, by adding an entry with a null plugin
+    // We do this so that we can (in future) detect if the task is cancelled before we finished feeding it response data
+    [self.handlerMap setObject:(id)[NSNull null] forKey:urlSchemeTask];
+
     NSString * startPath = [[NSBundle mainBundle] pathForResource:self.viewController.webContentFolderName ofType: nil];
     NSURL * url = urlSchemeTask.request.URL;
     NSString * stringToLoad = url.path;
     NSString * scheme = url.scheme;
-    
-    CDVViewController* vc = (CDVViewController*)self.viewController;
-
-    /*
-     * Give plugins the chance to handle the url
-     */
-    BOOL anyPluginsResponded = NO;
-    BOOL handledRequest = NO;
-
-    NSDictionary *pluginObjects = [[vc pluginObjects] copy];
-    for (NSString* pluginName in pluginObjects) {
-        self.schemePlugin = [vc.pluginObjects objectForKey:pluginName];
-        SEL selector = NSSelectorFromString(@"overrideSchemeTask:");
-        if ([self.schemePlugin respondsToSelector:selector]) {
-            handledRequest = (((BOOL (*)(id, SEL, id <WKURLSchemeTask>))objc_msgSend)(self.schemePlugin, selector, urlSchemeTask));
-            if (handledRequest) {
-                anyPluginsResponded = YES;
-                break;
-            }
-        }
-    }
 
-    if (!anyPluginsResponded) {
-        if ([scheme isEqualToString:self.viewController.appScheme]) {
-            if ([stringToLoad hasPrefix:@"/_app_file_"]) {
-                startPath = [stringToLoad stringByReplacingOccurrencesOfString:@"/_app_file_" withString:@""];
+    if ([scheme isEqualToString:self.viewController.appScheme]) {
+        if ([stringToLoad hasPrefix:@"/_app_file_"]) {
+            startPath = [stringToLoad stringByReplacingOccurrencesOfString:@"/_app_file_" withString:@""];
+        } else {
+            if ([stringToLoad isEqualToString:@""] || [url.pathExtension isEqualToString:@""]) {
+                startPath = [startPath stringByAppendingPathComponent:self.viewController.startPage];
             } else {
-                if ([stringToLoad isEqualToString:@""] || [url.pathExtension isEqualToString:@""]) {
-                    startPath = [startPath stringByAppendingPathComponent:self.viewController.startPage];
-                } else {
-                    startPath = [startPath stringByAppendingPathComponent:stringToLoad];
-                }
+                startPath = [startPath stringByAppendingPathComponent:stringToLoad];
             }
         }
+    }
 
-        NSError * fileError = nil;
-        NSData * data = nil;
-        if ([self isMediaExtension:url.pathExtension]) {
-            data = [NSData dataWithContentsOfFile:startPath options:NSDataReadingMappedIfSafe error:&fileError];
-        }
-        if (!data || fileError) {
-            data =  [[NSData alloc] initWithContentsOfFile:startPath];
-        }
-        NSInteger statusCode = 200;
-        if (!data) {
-            statusCode = 404;
-        }
-        NSURL * localUrl = [NSURL URLWithString:url.absoluteString];
-        NSString * mimeType = [self getMimeType:url.pathExtension];
-        id response = nil;
-        if (data && [self isMediaExtension:url.pathExtension]) {
-            response = [[NSURLResponse alloc] initWithURL:localUrl MIMEType:mimeType expectedContentLength:data.length textEncodingName:nil];
-        } else {
-            NSDictionary * headers = @{ @"Content-Type" : mimeType, @"Cache-Control": @"no-cache"};
-            response = [[NSHTTPURLResponse alloc] initWithURL:localUrl statusCode:statusCode HTTPVersion:nil headerFields:headers];
-        }
+    NSError * fileError = nil;
+    NSData * data = nil;
+    if ([self isMediaExtension:url.pathExtension]) {
+        data = [NSData dataWithContentsOfFile:startPath options:NSDataReadingMappedIfSafe error:&fileError];
+    }
+    if (!data || fileError) {
+        data =  [[NSData alloc] initWithContentsOfFile:startPath];
+    }
+    NSInteger statusCode = 200;
+    if (!data) {
+        statusCode = 404;
+    }
+    NSURL * localUrl = [NSURL URLWithString:url.absoluteString];
+    NSString * mimeType = [self getMimeType:url.pathExtension];
+    id response = nil;
+    if (data && [self isMediaExtension:url.pathExtension]) {
+        response = [[NSURLResponse alloc] initWithURL:localUrl MIMEType:mimeType expectedContentLength:data.length textEncodingName:nil];
+    } else {
+        NSDictionary * headers = @{ @"Content-Type" : mimeType, @"Cache-Control": @"no-cache"};
+        response = [[NSHTTPURLResponse alloc] initWithURL:localUrl statusCode:statusCode HTTPVersion:nil headerFields:headers];
+    }
 
-        [urlSchemeTask didReceiveResponse:response];
-        if (data) {
-            [urlSchemeTask didReceiveData:data];
-        }
-        [urlSchemeTask didFinish];
+    [urlSchemeTask didReceiveResponse:response];
+    if (data) {
+        [urlSchemeTask didReceiveData:data];
     }
+    [urlSchemeTask didFinish];
+
+    [self.handlerMap removeObjectForKey:urlSchemeTask];
 }
 
-- (void)webView:(nonnull WKWebView *)webView stopURLSchemeTask:(nonnull id<WKURLSchemeTask>)urlSchemeTask
+- (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)urlSchemeTask
 {
-    SEL selector = NSSelectorFromString(@"stopSchemeTask:");
-    if (self.schemePlugin != nil && [self.schemePlugin respondsToSelector:selector]) {
-        (((void (*)(id, SEL, id <WKURLSchemeTask>))objc_msgSend)(self.schemePlugin, selector, urlSchemeTask));
+    CDVPlugin <CDVPluginSchemeHandler> *plugin = [self.handlerMap objectForKey:urlSchemeTask];
+    if (![plugin isEqual:[NSNull null]] && [plugin respondsToSelector:@selector(stopSchemeTask:)]) {
+        [plugin stopSchemeTask:urlSchemeTask];
     }
+
+    [self.handlerMap removeObjectForKey:urlSchemeTask];
 }
 
 -(NSString *) getMimeType:(NSString *)fileExtension {
diff --git a/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVWebViewEngine.m b/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVWebViewEngine.m
@@ -215,7 +215,7 @@ - (void)pluginInitialize
 
     // Do not configure the scheme handler if the scheme is default (file)
     if(!self.cdvIsFileScheme) {
-        self.schemeHandler = [[CDVURLSchemeHandler alloc] initWithVC:vc];
+        self.schemeHandler = [[CDVURLSchemeHandler alloc] initWithViewController:vc];
         [configuration setURLSchemeHandler:self.schemeHandler forURLScheme:scheme];
     }
 
@@ -551,8 +551,7 @@ - (void) webView: (WKWebView *) webView decidePolicyForNavigationAction: (WKNavi
     BOOL anyPluginsResponded = NO;
     BOOL shouldAllowRequest = NO;
 
-    for (NSString* pluginName in vc.pluginObjects) {
-        CDVPlugin* plugin = [vc.pluginObjects objectForKey:pluginName];
+    for (CDVPlugin *plugin in vc.enumerablePlugins) {
         SEL selector = NSSelectorFromString(@"shouldOverrideLoadWithRequest:navigationType:");
         if ([plugin respondsToSelector:selector]) {
             anyPluginsResponded = YES;
diff --git a/CordovaLib/Classes/Public/CDVViewController.m b/CordovaLib/Classes/Public/CDVViewController.m
@@ -67,6 +67,7 @@ @implementation CDVViewController
 @synthesize splashBackgroundColor = _splashBackgroundColor;
 @synthesize settings = _settings;
 @dynamic webView;
+@dynamic enumerablePlugins;
 
 #pragma mark - Initializers
 
@@ -152,6 +153,13 @@ - (void)dealloc
 
 #pragma mark - Getters & Setters
 
+- (NSArray <CDVPlugin *> *)enumerablePlugins
+{
+    @synchronized(_pluginObjects) {
+        return [_pluginObjects allValues];
+    }
+}
+
 - (NSString *)wwwFolderName
 {
     return self.webContentFolderName;
@@ -460,15 +468,11 @@ - (void)onAppDidBecomeActive:(NSNotification *)notification
 
 - (void)didReceiveMemoryWarning
 {
-    // iterate through all the plugin objects, and call hasPendingOperation
-    // if at least one has a pending operation, we don't call [super didReceiveMemoryWarning]
-
-    NSEnumerator* enumerator = [self.pluginObjects objectEnumerator];
-    CDVPlugin* plugin;
-
     BOOL doPurge = YES;
 
-    while ((plugin = [enumerator nextObject])) {
+    // iterate through all the plugin objects, and call hasPendingOperation
+    // if at least one has a pending operation, we don't call [super didReceiveMemoryWarning]
+    for (CDVPlugin *plugin in self.enumerablePlugins) {
         if (plugin.hasPendingOperation) {
             NSLog(@"Plugin '%@' has a pending operation, memory purge is delayed for didReceiveMemoryWarning.", NSStringFromClass([plugin class]));
             doPurge = NO;
@@ -676,9 +680,13 @@ - (nullable CDVPlugin *)getCommandInstance:(NSString *)pluginName
         return nil;
     }
 
-    id obj = [self.pluginObjects objectForKey:className];
+    id obj = nil;
+    @synchronized(_pluginObjects) {
+        obj = [_pluginObjects objectForKey:className];
+    }
+
     if (!obj) {
-        obj = [[NSClassFromString(className)alloc] initWithWebViewEngine:_webViewEngine];
+        obj = [[NSClassFromString(className) alloc] initWithWebViewEngine:_webViewEngine];
         if (!obj) {
             NSString* fullClassName = [NSString stringWithFormat:@"%@.%@",
                                        NSBundle.mainBundle.infoDictionary[@"CFBundleExecutable"],
diff --git a/CordovaLib/Classes/Public/CDVWebViewProcessPoolFactory.m b/CordovaLib/Classes/Public/CDVWebViewProcessPoolFactory.m
@@ -17,6 +17,7 @@ Licensed to the Apache Software Foundation (ASF) under one
  under the License.
  */
 
+#import <WebKit/WebKit.h>
 #import <Cordova/CDVWebViewProcessPoolFactory.h>
 
 static CDVWebViewProcessPoolFactory *factory = nil;
diff --git a/CordovaLib/CordovaLib.docc/CordovaLib.md b/CordovaLib/CordovaLib.docc/CordovaLib.md
@@ -36,6 +36,9 @@ For more information about Apache Cordova, visit [https://cordova.apache.org](ht
 ### Cordova plugins
 
 - ``CDVPlugin``
+- ``CDVPluginSchemeHandler``
+
+### Plugin communication
 - ``CDVPluginResult``
 - ``CDVCommandStatus``
 - ``CDVInvokedUrlCommand``
diff --git a/CordovaLib/include/Cordova/CDVPlugin.h b/CordovaLib/include/Cordova/CDVPlugin.h
@@ -27,6 +27,9 @@
 #import <Cordova/CDVWebViewEngineProtocol.h>
 #import <Cordova/CDVInvokedUrlCommand.h>
 
+// Forward declaration to avoid bringing WebKit API into public headers
+@protocol WKURLSchemeTask;
+
 #ifndef __swift__
 // This global extension to the UIView class causes issues for Swift subclasses
 // of UIView with their own scrollView properties, so we're removing it from
@@ -79,3 +82,51 @@ extern const NSNotificationName CDVViewWillTransitionToSizeNotification;
 - (id)appDelegate;
 
 @end
+
+#pragma mark - Plugin protocols
+
+/**
+ A protocol for Cordova plugins to intercept handling of WebKit resource
+ loading for a custom URL scheme.
+
+ Your plugin should implement this protocol if it wants to intercept requests
+ to a custom URL scheme and provide its own resource loading. Otherwise,
+ Cordova will use its default resource loading behavior from the app bundle.
+
+ When a WebKit-based web view encounters a resource that uses a custom scheme,
+ it creates a WKURLSchemeTask object and Cordova passes it to the methods of
+ your scheme handler plugin for processing. Use the ``overrideSchemeTask:``
+ method to indicate that your plugin will handle the request and to begin
+ loading the resource. While your handler loads the object, Cordova may call
+ your plugin’s ``stopSchemeTask:`` method to notify you that the resource is no
+ longer needed.
+ */
+@protocol CDVPluginSchemeHandler <NSObject>
+
+/**
+ Asks your plugin to handle the specified request and begin loading data.
+
+ If your plugin intends to handle the request and return data, this method
+ should return `YES` as soon as possible to prevent the default request
+ handling. If this method returns `NO`, Cordova will handle the resource
+ loading using its default behavior.
+
+ Note that all methods of the task object must be called on the main thread.
+
+ - Parameters:
+   - task: The task object that identifies the resource to load. You also use
+     this object to report the progress of the load operation back to the web
+     view.
+ - Returns: A Boolean value indicating if the plugin is handling the request.
+ */
+- (BOOL)overrideSchemeTask:(id <WKURLSchemeTask>)task;
+
+/**
+ Asks your plugin to stop loading the data for the specified resource.
+
+ - Parameters:
+   - task: The task object that identifies the resource the web view no
+   longer needs.
+ */
+- (void)stopSchemeTask:(id <WKURLSchemeTask>)task;
+@end
diff --git a/CordovaLib/include/Cordova/CDVViewController.h b/CordovaLib/include/Cordova/CDVViewController.h
@@ -18,8 +18,6 @@
  */
 
 #import <UIKit/UIKit.h>
-#import <WebKit/WebKit.h>
-#import <Foundation/NSJSONSerialization.h>
 #import <Cordova/CDVAvailability.h>
 #import <Cordova/CDVInvokedUrlCommand.h>
 #import <Cordova/CDVCommandDelegate.h>
@@ -67,9 +65,16 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic, readonly, nullable, weak) IBOutlet UIView *webView;
 
-@property (nonatomic, readonly, strong) NSDictionary<NSString *, CDVPlugin *> *pluginObjects;
+@property (nonatomic, readonly, strong) NSDictionary<NSString *, CDVPlugin *> *pluginObjects CDV_DEPRECATED(8, "Internal implementation detail, should not be used");
 @property (nullable, nonatomic, readonly, strong) NSDictionary<NSString *, NSString *> *pluginsMap CDV_DEPRECATED(8, "Internal implementation detail, should not be used");
 
+/**
+ An array of loaded Cordova plugin instances.
+
+ This array is safe to iterate using a `for...in` loop.
+ */
+@property (nonatomic, readonly, copy) NSArray <CDVPlugin *> *enumerablePlugins;
+
 @property (nonatomic, readwrite, copy) NSString *appScheme;
 
 @property (nonatomic, readonly, strong) CDVCommandQueue *commandQueue;
diff --git a/CordovaLib/include/Cordova/CDVWebViewEngineProtocol.h b/CordovaLib/include/Cordova/CDVWebViewEngineProtocol.h
@@ -18,13 +18,14 @@
  */
 
 #import <UIKit/UIKit.h>
-#import <WebKit/WebKit.h>
 
 #define kCDVWebViewEngineScriptMessageHandlers @"kCDVWebViewEngineScriptMessageHandlers"
 #define kCDVWebViewEngineWKNavigationDelegate @"kCDVWebViewEngineWKNavigationDelegate"
 #define kCDVWebViewEngineWKUIDelegate @"kCDVWebViewEngineWKUIDelegate"
 #define kCDVWebViewEngineWebViewPreferences @"kCDVWebViewEngineWebViewPreferences"
 
+@class WKWebViewConfiguration;
+
 @protocol CDVWebViewEngineProtocol <NSObject>
 
 NS_ASSUME_NONNULL_BEGIN
diff --git a/CordovaLib/include/Cordova/CDVWebViewProcessPoolFactory.h b/CordovaLib/include/Cordova/CDVWebViewProcessPoolFactory.h
@@ -17,7 +17,7 @@
  under the License.
  */
 
-#import <WebKit/WebKit.h>
+@class WKProcessPool;
 
 @interface CDVWebViewProcessPoolFactory : NSObject
 @property (nonatomic, retain) WKProcessPool* sharedPool;
diff --git a/CordovaLib/include/Cordova/NSDictionary+CordovaPreferences.h b/CordovaLib/include/Cordova/NSDictionary+CordovaPreferences.h
