fix(ios): potential thread race in vfs file module

Tencent · Oct 31, 2024 · f7b17e1 · f7b17e1
1 parent 4b00876
commit f7b17e1
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 16 deletions.
diff --git 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<NSURL *> *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
@@ -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<HippyBridgeModule>)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
@@ -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
@@ -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
@@ -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,29 +49,29 @@ - (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.");
 }
 
 - (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.");
 }