Skip to content
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.

Sign up for GitHub

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

Allow plugins to hook into the WKURLSchemeHandler #1030

Merged
merged 6 commits into from
Jan 31, 2021
Merged

Allow plugins to hook into the WKURLSchemeHandler #1030

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afa8b32
Try out Hook
NiklasMerz Nov 24, 2020
b10a9b2
Implement schemetask stopping
NiklasMerz Nov 25, 2020
ba2f85f
Rename method
NiklasMerz Nov 25, 2020
22dbde0
Fix header
NiklasMerz Nov 25, 2020
924e471
copy object to avoid thread issues
NiklasMerz Dec 17, 2020
b99d68d
Merge branch 'master' into schemehandler-hook
NiklasMerz Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CordovaLib/Classes/Public/CDVURLSchemeHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

@property (nonatomic, strong) CDVViewController* viewController;

@property (nonatomic) CDVPlugin* schemePlugin;

- (instancetype)initWithVC:(CDVViewController *)controller;


Expand Down
94 changes: 60 additions & 34 deletions CordovaLib/Classes/Public/CDVURLSchemeHandler.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Licensed to the Apache Software Foundation (ASF) under one
#import "CDVURLSchemeHandler.h"
#import <MobileCoreServices/MobileCoreServices.h>

#import <objc/message.h>

@implementation CDVURLSchemeHandler


Expand All @@ -39,50 +41,74 @@ - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)ur
NSURL * url = urlSchemeTask.request.URL;
NSString * stringToLoad = url.path;
NSString * scheme = url.scheme;

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 {
startPath = [startPath stringByAppendingPathComponent:stringToLoad];

CDVViewController* vc = (CDVViewController*)self.viewController;

/*
* Give plugins the chance to handle the url
*/
BOOL anyPluginsResponded = NO;
BOOL handledRequest = NO;

for (NSString* pluginName in vc.pluginObjects) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This for loop pattern, which we also see in CDVWebViewEngine.m, occasionally causes crashes in our production apps because vc.pluginObjects can be mutated on a different thread while you're looping. Here's an example:

0   CoreFoundation                	0x1ac96a5ac __exceptionPreprocess + 220 (NSException.m:199)
1   libobjc.A.dylib               	0x1c0a5842c objc_exception_throw + 60 (objc-exception.mm:565)
2   CoreFoundation                	0x1ac969f04 __NSFastEnumerationMutationHandler + 124 (NSEnumerator.m:129)
3   Our App                	0x1022e13ec -[CDVWebViewEngine webView:decidePolicyForNavigationAction:decisionHandler:] + 336 (CDVWebViewEngine.m:543)

There are several right ways to deal with this:

  1. The best would be to use a protocol, like my similar request here: CDVWebViewEngine needs a way to set the websiteDataStore of its configuration #900
  2. You could alternatively use DispatchQueue to safely read and write pluginObjects on separate threads
  3. You could simply make a copy of pluginObjects and enumerate the copy instead

Since #3 is the easiest to write in a PR comment, that would look like the below, but I'd be happy to help you do #1, which would be a lot better.

    /*
     * Give plugins the chance to handle the url
     */
    __block BOOL anyPluginsResponded = NO;
    __block BOOL shouldAllowRequest = NO;
    
    NSDictionary *pluginObjects = [[vc pluginObjects] copy];
    SEL selector = NSSelectorFromString(@"shouldOverrideLoadWithRequest:navigationType:");
    [pluginObjects enumerateKeysAndObjectsUsingBlock:^(id  _Nonnull key, id  _Nonnull plugin, BOOL * _Nonnull stop) {
        if ([plugin respondsToSelector:selector]) {
            anyPluginsResponded = YES;
            // https://issues.apache.org/jira/browse/CB-12497
            int navType = (int)navigationAction.navigationType;
            shouldAllowRequest = (((BOOL (*)(id, SEL, id, int))objc_msgSend)(plugin, selector, navigationAction.request, navType));
            if (!shouldAllowRequest) {
                *stop = YES;
            }
        }
    }];

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated #900 with an example of a protocol that you could use here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your feedback on this. As I am not an ObjectiveC expert I am not sure how to deal with this. I would probably use "#3" for this PR and add a commen to #900 to properly implement the protocol for this and the other affected loop.

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;
}
}
}

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];
}
if (!anyPluginsResponded) {
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 {
startPath = [startPath stringByAppendingPathComponent:stringToLoad];
}
}
}

[urlSchemeTask didReceiveResponse:response];
[urlSchemeTask didReceiveData:data];
[urlSchemeTask didFinish];
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];
[urlSchemeTask didReceiveData:data];
[urlSchemeTask didFinish];
}
}

- (void)webView:(nonnull WKWebView *)webView stopURLSchemeTask:(nonnull 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));
}
}

-(NSString *) getMimeType:(NSString *)fileExtension {
Expand Down