Skip to content

Commit

Permalink
fix(ios): potential thread race in vfs file module (#4103)
Browse files Browse the repository at this point in the history
  • Loading branch information
wwwcg authored Oct 30, 2024
1 parent 84a02fa commit 9aa260c
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 16 deletions.
2 changes: 1 addition & 1 deletion framework/ios/base/bridge/HippyBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 13 additions & 4 deletions framework/ios/base/bridge/HippyBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ @interface HippyBridge() {

@implementation HippyBridge

@synthesize sandboxDirectory = _sandboxDirectory;
@synthesize imageLoader = _imageLoader;
@synthesize imageProviders = _imageProviders;
@synthesize startTime = _startTime;
Expand Down Expand Up @@ -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];
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/vfs/ios/HippyFileHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions modules/vfs/ios/HippyFileHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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/";
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
19 changes: 12 additions & 7 deletions tests/ios/HippyFileHandlerTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.");
}

Expand Down

0 comments on commit 9aa260c

Please sign in to comment.