From 8dddf6a11505d6b047da7403831dac711543fd49 Mon Sep 17 00:00:00 2001 From: Christopher Snowhill Date: Mon, 20 Jun 2022 22:10:43 -0700 Subject: [PATCH] [Sandbox] Refine broker to return handle to token Sandbox Broker now returns a handle to the exact path object that was retained by the caller, so it will be released correctly, regardless of what happens to the list of bookmarked paths. Also refined the bookmark path comparison function. For existing paths, it will find the first match. For new paths, it will prefer the longest path instead, to try to find the deepest matching bookmark. Signed-off-by: Christopher Snowhill --- .../ArchiveSource/ArchiveContainer.m | 4 +- .../ArchiveSource/ArchiveSource.h | 3 +- .../ArchiveSource/ArchiveSource.m | 9 +- Plugins/FileSource/FileSource.h | 2 + Plugins/FileSource/FileSource.m | 11 +- Plugins/MIDI/MIDI/MIDIDecoder.h | 2 +- Plugins/MIDI/MIDI/MIDIDecoder.mm | 13 +- Plugins/TagLib/TagLibMetadataReader.m | 4 +- Utils/SandboxBroker.h | 4 +- Utils/SandboxBroker.m | 148 ++++++++---------- 10 files changed, 98 insertions(+), 102 deletions(-) diff --git a/Plugins/ArchiveSource/ArchiveSource/ArchiveContainer.m b/Plugins/ArchiveSource/ArchiveSource/ArchiveContainer.m index 9e1544c64..6b7c94de5 100644 --- a/Plugins/ArchiveSource/ArchiveSource/ArchiveContainer.m +++ b/Plugins/ArchiveSource/ArchiveSource/ArchiveContainer.m @@ -48,7 +48,7 @@ static NSString *g_make_unpack_path(NSString *archive, NSString *file, NSString id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker beginFolderAccess:url]; + const void *sbHandle = [sandboxBroker beginFolderAccess:url]; fex_t *fex; fex_err_t error = fex_open(&fex, [[url path] UTF8String]); @@ -68,7 +68,7 @@ static NSString *g_make_unpack_path(NSString *archive, NSString *file, NSString fex_close(fex); - [sandboxBroker endFolderAccess:url]; + [sandboxBroker endFolderAccess:sbHandle]; return files; } diff --git a/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.h b/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.h index 853ddfbfe..fe290bc26 100644 --- a/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.h +++ b/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.h @@ -20,7 +20,8 @@ NSUInteger size; NSURL *_url; - NSURL *fileURL; + + const void *sbHandle; } @end diff --git a/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.m b/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.m index 2b146afe0..9c1d7f3ee 100644 --- a/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.m +++ b/Plugins/ArchiveSource/ArchiveSource/ArchiveSource.m @@ -85,12 +85,10 @@ static BOOL g_parse_unpack_path(NSString *src, NSString **archive, NSString **fi if(![type isEqualToString:@"fex"]) return NO; - fileURL = [NSURL fileURLWithPath:archive]; - id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker beginFolderAccess:fileURL]; + sbHandle = [sandboxBroker beginFolderAccess:[NSURL fileURLWithPath:archive]]; fex_err_t error; @@ -161,11 +159,12 @@ static BOOL g_parse_unpack_path(NSString *src, NSString **archive, NSString **fi fex = NULL; } - if(fileURL) { + if(sbHandle) { id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker endFolderAccess:fileURL]; + [sandboxBroker endFolderAccess:sbHandle]; + sbHandle = NULL; } } diff --git a/Plugins/FileSource/FileSource.h b/Plugins/FileSource/FileSource.h index 72394ada3..3b57ffb13 100644 --- a/Plugins/FileSource/FileSource.h +++ b/Plugins/FileSource/FileSource.h @@ -21,6 +21,8 @@ FILE *_fd; NSURL *_url; + + const void *sbHandle; } @end diff --git a/Plugins/FileSource/FileSource.m b/Plugins/FileSource/FileSource.m index bb350fe52..70d975fa6 100644 --- a/Plugins/FileSource/FileSource.m +++ b/Plugins/FileSource/FileSource.m @@ -31,7 +31,7 @@ id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker beginFolderAccess:url]; + sbHandle = [sandboxBroker beginFolderAccess:url]; NSString *path = [url path]; @@ -131,10 +131,13 @@ fex = NULL; } - id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); - id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; + if(sbHandle) { + id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); + id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker endFolderAccess:_url]; + [sandboxBroker endFolderAccess:sbHandle]; + sbHandle = NULL; + } } - (NSURL *)url { diff --git a/Plugins/MIDI/MIDI/MIDIDecoder.h b/Plugins/MIDI/MIDI/MIDIDecoder.h index 4f4ec49a1..d7446d51d 100644 --- a/Plugins/MIDI/MIDI/MIDIDecoder.h +++ b/Plugins/MIDI/MIDI/MIDIDecoder.h @@ -24,7 +24,7 @@ class BMPlayer; MIDIPlayer* player; midi_container midi_file; - NSURL* sandboxURL; + const void* sbHandle; NSString* globalSoundFontPath; BOOL soundFontsAssigned; diff --git a/Plugins/MIDI/MIDI/MIDIDecoder.mm b/Plugins/MIDI/MIDI/MIDIDecoder.mm index deb844ad4..660d5f9bb 100644 --- a/Plugins/MIDI/MIDI/MIDIDecoder.mm +++ b/Plugins/MIDI/MIDI/MIDIDecoder.mm @@ -168,12 +168,14 @@ static OSType getOSType(const char *in_) { globalSoundFontPath = [[NSUserDefaults standardUserDefaults] stringForKey:@"soundFontPath"]; if(globalSoundFontPath && [globalSoundFontPath length] > 0) { - sandboxURL = [NSURL fileURLWithPath:[globalSoundFontPath stringByDeletingLastPathComponent]]; + NSURL *sandboxURL = [NSURL fileURLWithPath:[globalSoundFontPath stringByDeletingLastPathComponent]]; id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker beginFolderAccess:sandboxURL]; + sbHandle = [sandboxBroker beginFolderAccess:sandboxURL]; + } else { + sbHandle = NULL; } // First detect if soundfont has gone AWOL @@ -345,13 +347,12 @@ static OSType getOSType(const char *in_) { delete player; player = NULL; - if(sandboxURL) { + if(sbHandle) { id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker endFolderAccess:sandboxURL]; - - sandboxURL = nil; + [sandboxBroker endFolderAccess:sbHandle]; + sbHandle = NULL; } } diff --git a/Plugins/TagLib/TagLibMetadataReader.m b/Plugins/TagLib/TagLibMetadataReader.m index 4fe014002..d7647e373 100644 --- a/Plugins/TagLib/TagLibMetadataReader.m +++ b/Plugins/TagLib/TagLibMetadataReader.m @@ -31,7 +31,7 @@ id sandboxBrokerClass = NSClassFromString(@"SandboxBroker"); id sandboxBroker = [sandboxBrokerClass sharedSandboxBroker]; - [sandboxBroker beginFolderAccess:url]; + const void *sbHandle = [sandboxBroker beginFolderAccess:url]; NSMutableDictionary *dict = [[NSMutableDictionary alloc] init]; @@ -237,7 +237,7 @@ } } - [sandboxBroker endFolderAccess:url]; + [sandboxBroker endFolderAccess:sbHandle]; return dict; } diff --git a/Utils/SandboxBroker.h b/Utils/SandboxBroker.h index cc11e4ea3..6ce46d4c7 100644 --- a/Utils/SandboxBroker.h +++ b/Utils/SandboxBroker.h @@ -18,8 +18,8 @@ NS_ASSUME_NONNULL_BEGIN - (id)init; - (void)shutdown; -- (void)beginFolderAccess:(NSURL *)fileUrl; -- (void)endFolderAccess:(NSURL *)fileUrl; +- (const void *)beginFolderAccess:(NSURL *)fileUrl; +- (void)endFolderAccess:(const void *)handle; @end diff --git a/Utils/SandboxBroker.m b/Utils/SandboxBroker.m index 667e1976b..8c4b33078 100644 --- a/Utils/SandboxBroker.m +++ b/Utils/SandboxBroker.m @@ -125,112 +125,102 @@ static NSURL *urlWithoutFragment(NSURL *u) { } } -- (void)recursivePathTest:(NSURL *)url removing:(BOOL)removing { - NSArray *pathComponents = [url pathComponents]; ++ (BOOL)isPath:(NSURL *)path aSubdirectoryOf:(NSURL *)directory { + NSArray *pathComponents = [path pathComponents]; + NSArray *directoryComponents = [directory pathComponents]; - NSSortDescriptor *sortDescriptor = [NSSortDescriptor sortDescriptorWithKey:@"path" ascending:NO]; + if([pathComponents count] < [directoryComponents count]) + return NO; - for(size_t i = [pathComponents count]; i > 0; --i) { - NSArray *partialComponents = [pathComponents subarrayWithRange:NSMakeRange(0, i)]; - NSURL *partialUrl = [NSURL fileURLWithPathComponents:partialComponents]; - NSString *matchString = [[partialUrl path] stringByAppendingString:@"*"]; - - NSPredicate *predicate = [NSPredicate predicateWithFormat:@"path like %@", matchString]; - - NSArray *matchingObjects = [storage filteredArrayUsingPredicate:predicate]; - - if(matchingObjects && [matchingObjects count] > 0) { - if([matchingObjects count] > 1) { - matchingObjects = [matchingObjects sortedArrayUsingDescriptors:@[sortDescriptor]]; - } - - for(SandboxEntry *entry in matchingObjects) { - if([entry.path isEqualToString:[partialUrl path]]) { - if(!removing) { - entry.refCount += 1; - return; - } else { - if(entry.refCount > 1) { - entry.refCount -= 1; - return; - } else { - if(entry.secureUrl) { - [entry.secureUrl stopAccessingSecurityScopedResource]; - entry.secureUrl = nil; - } - entry.refCount = 0; - - [storage removeObject:entry]; - return; - } - } - } - } - } + for(size_t i = 0; i < [directoryComponents count]; ++i) { + if(![pathComponents[i] isEqualToString:directoryComponents[i]]) + return NO; } - if(removing) return; + return YES; +} + +- (SandboxEntry *)recursivePathTest:(NSURL *)url { + for(SandboxEntry *entry in storage) { + if([SandboxBroker isPath:url aSubdirectoryOf:[NSURL fileURLWithPath:entry.path]]) { + entry.refCount += 1; + return entry; + } + } NSPersistentContainer *pc = [NSApp sharedPersistentContainer]; - for(size_t i = [pathComponents count]; i > 0; --i) { - NSArray *partialComponents = [pathComponents subarrayWithRange:NSMakeRange(0, i)]; - NSURL *partialUrl = [NSURL fileURLWithPathComponents:partialComponents]; - NSString *matchString = [[partialUrl path] stringByAppendingString:@"*"]; + NSSortDescriptor *sortDescriptor = [NSSortDescriptor sortDescriptorWithKey:@"path.length" ascending:NO]; - NSPredicate *predicate = [NSPredicate predicateWithFormat:@"path like %@", matchString]; + NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"SandboxToken"]; + request.sortDescriptors = @[sortDescriptor]; - NSFetchRequest *request = [NSFetchRequest fetchRequestWithEntityName:@"SandboxToken"]; - request.predicate = predicate; - request.sortDescriptors = @[sortDescriptor]; + NSError *error = nil; + NSArray *results = [pc.viewContext executeFetchRequest:request error:&error]; - NSError *error = nil; - NSArray *results = [pc.viewContext executeFetchRequest:request error:&error]; + if(results && [results count] > 0) { + for(SandboxToken *token in results) { + if([SandboxBroker isPath:url aSubdirectoryOf:[NSURL fileURLWithPath:token.path]]) { + SandboxEntry *entry = [[SandboxEntry alloc] initWithToken:token]; - if(results && [results count] > 0) { - for(SandboxToken *token in results) { - if([token.path isEqualToString:[partialUrl path]]) { - SandboxEntry *entry = [[SandboxEntry alloc] initWithToken:token]; + [storage addObject:entry]; - [storage addObject:entry]; - - BOOL isStale; - NSError *err = nil; - NSURL *secureUrl = [NSURL URLByResolvingBookmarkData:token.bookmark options:NSURLBookmarkResolutionWithSecurityScope relativeToURL:nil bookmarkDataIsStale:&isStale error:&err]; - if(!secureUrl && err) { - ALog(@"Failed to access bookmark for URL: %@, error: %@", token.path, [err localizedDescription]); - return; - } - - entry.secureUrl = secureUrl; - - [secureUrl startAccessingSecurityScopedResource]; - - return; + BOOL isStale; + NSError *err = nil; + NSURL *secureUrl = [NSURL URLByResolvingBookmarkData:token.bookmark options:NSURLBookmarkResolutionWithSecurityScope relativeToURL:nil bookmarkDataIsStale:&isStale error:&err]; + if(!secureUrl && err) { + ALog(@"Failed to access bookmark for URL: %@, error: %@", token.path, [err localizedDescription]); + return nil; } + + entry.secureUrl = secureUrl; + + [secureUrl startAccessingSecurityScopedResource]; + + return entry; } } } - return; + return nil; } -- (void)beginFolderAccess:(NSURL *)fileUrl { +- (const void *)beginFolderAccess:(NSURL *)fileUrl { NSURL *folderUrl = [urlWithoutFragment(fileUrl) URLByDeletingLastPathComponent]; - if(![folderUrl isFileURL]) return; - if(![NSApp respondsToSelector:@selector(sharedPersistentContainer)]) return; + if(![folderUrl isFileURL]) return NULL; + if(![NSApp respondsToSelector:@selector(sharedPersistentContainer)]) return NULL; + + SandboxEntry *entry; @synchronized(self) { - [self recursivePathTest:folderUrl removing:NO]; + entry = [self recursivePathTest:folderUrl]; + } + + if(entry) { + return CFBridgingRetain(entry); + } else { + return NULL; } } -- (void)endFolderAccess:(NSURL *)fileUrl { - NSURL *folderUrl = [urlWithoutFragment(fileUrl) URLByDeletingLastPathComponent]; - if(![folderUrl isFileURL]) return; +- (void)endFolderAccess:(const void *)handle { + if(!handle) return; + SandboxEntry *entry = CFBridgingRelease(handle); + if(!entry) return; @synchronized(self) { - [self recursivePathTest:folderUrl removing:YES]; + if(entry.refCount > 1) { + entry.refCount -= 1; + return; + } else { + if(entry.secureUrl) { + [entry.secureUrl stopAccessingSecurityScopedResource]; + entry.secureUrl = nil; + } + entry.refCount = 0; + + [storage removeObject:entry]; + } } }