From f7b17e1ef08a186c19bdcf20ffcc0576812a335a Mon Sep 17 00:00:00 2001 From: wwwcg Date: Wed, 30 Oct 2024 20:14:00 +0800 Subject: [PATCH] fix(ios): potential thread race in vfs file module --- framework/ios/base/bridge/HippyBridge.h | 2 +- framework/ios/base/bridge/HippyBridge.mm | 17 +++++++++++++---- modules/vfs/ios/HippyFileHandler.h | 4 ++-- modules/vfs/ios/HippyFileHandler.mm | 5 +++-- tests/ios/HippyFileHandlerTest.mm | 19 ++++++++++++------- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/framework/ios/base/bridge/HippyBridge.h b/framework/ios/base/bridge/HippyBridge.h index addcc0193e9..784e1ff3e71 100644 --- a/framework/ios/base/bridge/HippyBridge.h +++ b/framework/ios/base/bridge/HippyBridge.h @@ -214,7 +214,7 @@ HIPPY_EXTERN NSString *HippyBridgeModuleNameForClass(Class bridgeModuleClass); @property (nonatomic, copy, readonly) NSArray *bundleURLs; /// Path of sandbox directory -@property (nonatomic, strong) NSString *sandboxDirectory; +@property (nonatomic, copy) NSString *sandboxDirectory; /// Shared data between different rootViews on same bridge. /// Set by HippyRootView when runHippyApplication. diff --git a/framework/ios/base/bridge/HippyBridge.mm b/framework/ios/base/bridge/HippyBridge.mm index 22e30951d91..62e8de82d4a 100644 --- a/framework/ios/base/bridge/HippyBridge.mm +++ b/framework/ios/base/bridge/HippyBridge.mm @@ -192,6 +192,7 @@ @interface HippyBridge() { @implementation HippyBridge +@synthesize sandboxDirectory = _sandboxDirectory; @synthesize imageLoader = _imageLoader; @synthesize imageProviders = _imageProviders; @synthesize startTime = _startTime; @@ -1218,11 +1219,19 @@ - (void)registerModuleForFrameUpdates:(id)module withModuleDa [_displayLink registerModuleForFrameUpdates:module withModuleData:moduleData]; } +- (NSString *)sandboxDirectory { + @synchronized (self) { + return _sandboxDirectory; + } +} + - (void)setSandboxDirectory:(NSString *)sandboxDirectory { - if (![_sandboxDirectory isEqual:sandboxDirectory]) { - _sandboxDirectory = sandboxDirectory; - if (sandboxDirectory) { - [self.javaScriptExecutor setSandboxDirectory:sandboxDirectory]; + @synchronized (self) { + if (![_sandboxDirectory isEqual:sandboxDirectory]) { + _sandboxDirectory = sandboxDirectory; + if (sandboxDirectory) { + [self.javaScriptExecutor setSandboxDirectory:sandboxDirectory]; + } } } } diff --git a/modules/vfs/ios/HippyFileHandler.h b/modules/vfs/ios/HippyFileHandler.h index 5d306bc7f1b..3ca65ee4356 100644 --- a/modules/vfs/ios/HippyFileHandler.h +++ b/modules/vfs/ios/HippyFileHandler.h @@ -43,8 +43,8 @@ class HippyFileHandler : public VFSUriHandler { /// Convert relative addresses(such as hpfile://) to absolute paths /// - Parameters: /// - hippyFileUrl: file url - /// - hippySandboxDirectory: sandbox directory of hippy app - static NSURL *AbsoluteURLFromHippyFileURL(NSURL *hippyFileUrl, NSURL *hippySandboxDirectory); + /// - bridge: HippyBridge, use to get sandbox url + static NSURL *AbsoluteURLFromHippyFileURL(NSURL *hippyFileUrl, HippyBridge *bridge); virtual void RequestUntrustedContent(NSURLRequest *request, NSDictionary *extraInfo, diff --git a/modules/vfs/ios/HippyFileHandler.mm b/modules/vfs/ios/HippyFileHandler.mm index f74be7b4218..cd308b70c49 100644 --- a/modules/vfs/ios/HippyFileHandler.mm +++ b/modules/vfs/ios/HippyFileHandler.mm @@ -41,7 +41,7 @@ FOOTSTONE_UNIMPLEMENTED(); } -NSURL *HippyFileHandler::AbsoluteURLFromHippyFileURL(NSURL *fileUrl, NSURL *hippySandboxDirectory) { +NSURL *HippyFileHandler::AbsoluteURLFromHippyFileURL(NSURL *fileUrl, HippyBridge *bridge) { static NSString *defaultHippyLocalFileURLPrefix = @"hpfile://"; static NSString *hippyLocalRelativeFilePathPrefix = @"./"; static NSString *hippyLocalAppBundleFilePathPrefix = @"appbundle/"; @@ -55,6 +55,7 @@ if ([path hasPrefix:hippyLocalRelativeFilePathPrefix]) { // Hippy Sandbox Relative Path NSString *relativePath = [path substringFromIndex:hippyLocalRelativeFilePathPrefix.length]; + NSURL *hippySandboxDirectory = [NSURL URLWithString:bridge.sandboxDirectory]; absoluteURL = [NSURL fileURLWithPath:relativePath relativeToURL:hippySandboxDirectory]; } else if ([path hasPrefix:hippyLocalAppBundleFilePathPrefix]) { // App Bundle Path @@ -90,7 +91,7 @@ return; } - NSURL *absoluteURL = AbsoluteURLFromHippyFileURL(url, [NSURL URLWithString:bridge.sandboxDirectory]); + NSURL *absoluteURL = AbsoluteURLFromHippyFileURL(url, bridge); if ([absoluteURL isFileURL] || [absoluteURL isFileReferenceURL]) { void (^opBlock)() = ^{ NSError *error; diff --git a/tests/ios/HippyFileHandlerTest.mm b/tests/ios/HippyFileHandlerTest.mm index dd98c276155..e414fdc618d 100644 --- a/tests/ios/HippyFileHandlerTest.mm +++ b/tests/ios/HippyFileHandlerTest.mm @@ -27,14 +27,19 @@ @interface HippyFileHandlerTest : XCTestCase /// Test sandboxDirectory for file handler -@property (nonatomic, strong) NSURL *sandboxDirectory; +@property (nonatomic, strong) NSString *sandboxDirectory; + +/// HippyBridge +@property (nonatomic, strong) HippyBridge *bridge; @end @implementation HippyFileHandlerTest - (void)setUp { - self.sandboxDirectory = [NSURL fileURLWithPath:@"/path/to/sandbox"]; + self.sandboxDirectory = @"/path/to/sandbox"; + self.bridge = [[HippyBridge alloc] initWithDelegate:nil moduleProvider:nil launchOptions:nil executorKey:nil]; + self.bridge.sandboxDirectory = self.sandboxDirectory; } - (void)tearDown { @@ -44,7 +49,7 @@ - (void)tearDown { - (void)testAbsoluteURLFromHippyFileURL_AppBundlePath { NSURL *fileUrl = [NSURL URLWithString:@"hpfile://appbundle/testfile.txt"]; NSURL *expectedURL = [[NSBundle mainBundle] URLForResource:@"testfile" withExtension:@"txt"]; - NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl,self.sandboxDirectory); + NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl, self.bridge); XCTAssertEqualObjects(resultURL, expectedURL, @"The URLs should be equal for app bundle paths."); } @@ -52,21 +57,21 @@ - (void)testAbsoluteURLFromHippyFileURL_ContainerPath { NSURL *fileUrl = [NSURL URLWithString:@"hpfile://container/Documents/testfile.txt"]; NSString *containerPath = [NSHomeDirectory() stringByAppendingPathComponent:@"Documents/testfile.txt"]; NSURL *expectedURL = [NSURL fileURLWithPath:containerPath]; - NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl,self.sandboxDirectory); + NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl, self.bridge); XCTAssertEqualObjects(resultURL, expectedURL, @"The URLs should be equal for container paths."); } - (void)testAbsoluteURLFromHippyFileURL_SandboxRelativePath { NSURL *fileUrl = [NSURL URLWithString:@"hpfile://./testfile.txt"]; - NSURL *expectedURL = [NSURL fileURLWithPath:@"testfile.txt" relativeToURL:self.sandboxDirectory]; - NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl,self.sandboxDirectory); + NSURL *expectedURL = [NSURL fileURLWithPath:@"testfile.txt" relativeToURL:[NSURL URLWithString:self.sandboxDirectory]]; + NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl, self.bridge); XCTAssertEqualObjects(resultURL, expectedURL, @"The URLs should be equal for sandbox relative paths."); } - (void)testAbsoluteURLFromHippyFileURL_InvalidPrefix { NSURL *fileUrl = [NSURL URLWithString:@"invalid://testfile.txt"]; NSURL *expectedURL = fileUrl; - NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl,self.sandboxDirectory); + NSURL *resultURL = HippyFileHandler::AbsoluteURLFromHippyFileURL(fileUrl, self.bridge); XCTAssertEqualObjects(resultURL, expectedURL, @"The URLs should be equal for invalid prefixes."); }