Skip to content

Commit

Permalink
- NSString+OCPath: properties for
Browse files Browse the repository at this point in the history
	- recognizing normalized paths
	- recognizing unnormalized / malformed paths
- OCCore
	- absence of parent items is now used as indication that a sub item of the absent parent item does not exist
	- [OCCore trackItemAtPath:trackingHandler:] now returns OCErrorUnnormalizedPath when passing unnormalized paths (containing f.ex. "//", "." or "..")
- new tests covering the new & improved functionality
  • Loading branch information
felix-schwarz committed Oct 1, 2019
1 parent b85482a commit ea3e8c0
Show file tree
Hide file tree
Showing 9 changed files with 496 additions and 0 deletions.
301 changes: 301 additions & 0 deletions 0001-NSString-OCPath-properties-for.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,301 @@
From e71a7193cff72d0afb74d81c144147ddb391d6c4 Mon Sep 17 00:00:00 2001
From: Felix Schwarz <felix.schwarz@iospirit.com>
Date: Tue, 1 Oct 2019 23:18:27 +0200
Subject: [PATCH] - NSString+OCPath: properties for - recognizing
normalized paths - recognizing unnormalized / malformed paths - OCCore
- absence of parent items is now used as indication that a sub item of the
absent parent item does not exist - [OCCore
trackItemAtPath:trackingHandler:] now returns OCErrorUnnormalizedPath when
passing unnormalized paths (containing f.ex. "//", "." or "..") - new tests
covering the new & improved functionality

---
.../Categories/Foundation/NSString+OCPath.h | 3 +
.../Categories/Foundation/NSString+OCPath.m | 29 +++++
ownCloudSDK/Core/ItemList/OCCore+ItemList.m | 32 +++++
ownCloudSDK/Core/OCCore.m | 7 ++
ownCloudSDK/Errors/NSError+OCError.h | 2 +
ownCloudSDK/Errors/NSError+OCError.m | 4 +
.../Resources/en.lproj/Localizable.strings | 3 +
ownCloudSDKTests/CoreTests.m | 115 ++++++++++++++++++
8 files changed, 195 insertions(+)

diff --git a/ownCloudSDK/Categories/Foundation/NSString+OCPath.h b/ownCloudSDK/Categories/Foundation/NSString+OCPath.h
index ea0c393..7a85810 100644
--- a/ownCloudSDK/Categories/Foundation/NSString+OCPath.h
+++ b/ownCloudSDK/Categories/Foundation/NSString+OCPath.h
@@ -28,4 +28,7 @@

- (OCPath)pathForSubdirectoryWithName:(NSString *)subDirectoryName;

+@property(readonly,nonatomic) BOOL isUnnormalizedPath;
+@property(readonly,nonatomic) BOOL isNormalizedDirectoryPath;
+
@end
diff --git a/ownCloudSDK/Categories/Foundation/NSString+OCPath.m b/ownCloudSDK/Categories/Foundation/NSString+OCPath.m
index 67cda65..fe62fe3 100644
--- a/ownCloudSDK/Categories/Foundation/NSString+OCPath.m
+++ b/ownCloudSDK/Categories/Foundation/NSString+OCPath.m
@@ -45,4 +45,33 @@ - (OCPath)pathForSubdirectoryWithName:(NSString *)subDirectoryName
return ([[self stringByAppendingPathComponent:subDirectoryName] normalizedDirectoryPath]);
}

+- (BOOL)isUnnormalizedPath
+{
+ NSArray<NSString *> *pathComponents = [self componentsSeparatedByString:@"/"];
+ NSUInteger idx = 0;
+
+ if (![self hasPrefix:@"/"])
+ {
+ return (YES);
+ }
+
+ for (NSString *pathComponent in pathComponents)
+ {
+ if (([pathComponent isEqual:@""] && (idx!=0) && (idx!=(pathComponents.count-1))) || // Multi-slash "//"
+ [pathComponent isEqual:@"."] || [pathComponent isEqual:@".."]) // ".." or "."
+ {
+ return (YES);
+ }
+
+ idx++;
+ }
+
+ return (NO);
+}
+
+- (BOOL)isNormalizedDirectoryPath
+{
+ return (![self isUnnormalizedPath] && [self hasSuffix:@"/"]);
+}
+
@end
diff --git a/ownCloudSDK/Core/ItemList/OCCore+ItemList.m b/ownCloudSDK/Core/ItemList/OCCore+ItemList.m
index 9ad42f9..b2afe04 100644
--- a/ownCloudSDK/Core/ItemList/OCCore+ItemList.m
+++ b/ownCloudSDK/Core/ItemList/OCCore+ItemList.m
@@ -937,6 +937,38 @@ - (void)_finalizeQueryUpdatesWithQueryResults:(NSMutableArray<OCItem *> *)queryR
OCPath queryItemPath = nil;
OCSyncAnchor syncAnchor = nil;

+ // Queries targeting an item in a subdirectory of taskPath: check if that subdirectory exists
+ if (taskPath.isNormalizedDirectoryPath && [query.queryPath hasPrefix:taskPath] &&
+ (task.cachedSet.state == OCCoreItemListStateSuccess) && (task.retrievedSet.state == OCCoreItemListStateSuccess)
+ )
+ {
+ if (query.state != OCQueryStateIdle)
+ {
+ NSString *queryPathSubfolder;
+
+ if ((queryPathSubfolder = [[query.queryPath substringFromIndex:taskPath.length] componentsSeparatedByString:@"/"].firstObject) != nil)
+ {
+ NSString *queryPathSubpath;
+
+ if (queryResultItemsByPath == nil)
+ {
+ queryResultItemsByPath = [OCCoreItemList itemListWithItems:queryResults].itemsByPath;
+ }
+
+ if ((queryPathSubpath = [taskPath stringByAppendingPathComponent:queryPathSubfolder]) != nil)
+ {
+ if ((queryResultItemsByPath[queryPathSubpath] == nil) &&
+ (queryResultItemsByPath[queryPathSubpath.normalizedDirectoryPath] == nil))
+ {
+ // Relevant parent folder is missing
+ useQueryResults = [NSMutableArray new];
+ setQueryState = OCQueryStateTargetRemoved;
+ }
+ }
+ }
+ }
+ }
+
// Queries targeting a particular item
if ((queryItemPath = query.queryItem.path) != nil)
{
diff --git a/ownCloudSDK/Core/OCCore.m b/ownCloudSDK/Core/OCCore.m
index 8f8f2ca..e116edd 100644
--- a/ownCloudSDK/Core/OCCore.m
+++ b/ownCloudSDK/Core/OCCore.m
@@ -1108,6 +1108,13 @@ - (OCCoreItemTracking)trackItemAtPath:(OCPath)path trackingHandler:(void(^)(NSEr
NSObject *trackingObject = [NSObject new];
__weak NSObject *weakTrackingObject = trackingObject;

+ // Detect unnormalized path
+ if ([path isUnnormalizedPath])
+ {
+ trackingHandler(OCError(OCErrorUnnormalizedPath), nil, YES);
+ return (nil);
+ }
+
[self queueBlock:^{
NSError *error = nil;
OCItem *item = nil;
diff --git a/ownCloudSDK/Errors/NSError+OCError.h b/ownCloudSDK/Errors/NSError+OCError.h
index 2b5cc27..5cc52d0 100644
--- a/ownCloudSDK/Errors/NSError+OCError.h
+++ b/ownCloudSDK/Errors/NSError+OCError.h
@@ -94,6 +94,8 @@ typedef NS_ENUM(NSUInteger, OCError)

OCErrorItemPolicyRedundant, //!< Another item policy of the same kind already includes the item, making the addition of this item policy redundant. Item policy(s) are passed as error.userInfo[OCErrorItemPoliciesKey].
OCErrorItemPolicyMakesRedundant, //!< Other item policies of the same kind covering subsets of this item policy become redundant by the addition of this item policy. Item policy(s) are passed as error.userInfo[OCErrorItemPoliciesKey].
+
+ OCErrorUnnormalizedPath //!< The provided path is not normalized.
};

@class OCIssue;
diff --git a/ownCloudSDK/Errors/NSError+OCError.m b/ownCloudSDK/Errors/NSError+OCError.m
index 723ffac..61fa2ac 100644
--- a/ownCloudSDK/Errors/NSError+OCError.m
+++ b/ownCloudSDK/Errors/NSError+OCError.m
@@ -272,6 +272,10 @@ + (id)provideUserInfoValueForOCError:(NSError *)error userInfoKey:(NSErrorUserIn
case OCErrorItemPolicyMakesRedundant:
unlocalizedString = @"Other item policies of the same kind covering subsets of this item policy become redundant by the addition of this item policy.";
break;
+
+ case OCErrorUnnormalizedPath:
+ unlocalizedString = @"The provided path is not normalized.";
+ break;
}
}
}
diff --git a/ownCloudSDK/Resources/en.lproj/Localizable.strings b/ownCloudSDK/Resources/en.lproj/Localizable.strings
index 7c032f7..82b65ab 100644
--- a/ownCloudSDK/Resources/en.lproj/Localizable.strings
+++ b/ownCloudSDK/Resources/en.lproj/Localizable.strings
@@ -270,3 +270,6 @@

// OCErrorItemPolicyMakesRedundant
"Other item policies of the same kind covering subsets of this item policy become redundant by the addition of this item policy." = "Other item policies of the same kind covering subsets of this item policy become redundant by the addition of this item policy.";
+
+// OCErrorUnnormalizedPath
+"The provided path is not normalized." = "The provided path is not normalized.";
diff --git a/ownCloudSDKTests/CoreTests.m b/ownCloudSDKTests/CoreTests.m
index 07525f2..99c1fc4 100644
--- a/ownCloudSDKTests/CoreTests.m
+++ b/ownCloudSDKTests/CoreTests.m
@@ -1144,6 +1144,121 @@ - (void)testItemTrackingNonExistant
}];
}

+- (void)testItemTrackingDeepNonExistant
+{
+ OCBookmark *bookmark = [OCTestTarget userBookmark];
+ OCCore *core;
+ XCTestExpectation *coreStartedExpectation = [self expectationWithDescription:@"Core started"];
+ XCTestExpectation *coreStoppedExpectation = [self expectationWithDescription:@"Core stopped"];
+ XCTestExpectation *initialTrackingResponseForNonExistantItemExpectation = [self expectationWithDescription:@"Initial tracking response for non-existant item"];
+ __block id itemTrackerNonExistantItem = nil;
+
+ // Create core
+ core = [[OCCore alloc] initWithBookmark:bookmark];
+ core.automaticItemListUpdatesEnabled = NO;
+
+ // Start core
+ [core startWithCompletionHandler:^(OCCore *core, NSError *error) {
+ core.vault.database.itemFilter = self.databaseSanityCheckFilter;
+
+ XCTAssert((error==nil), @"Started with error: %@", error);
+ [coreStartedExpectation fulfill];
+
+ OCLog(@"Vault location: %@", core.vault.rootURL);
+
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(58 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+ });
+
+ itemTrackerNonExistantItem = [core trackItemAtPath:@"/does.not.exist/either/" trackingHandler:^(NSError * _Nullable error, OCItem * _Nullable serverItem, BOOL isInitial) {
+ OCLog(@"Tracked(NE): isInitial=%d error=%@ item=%@", isInitial, error, serverItem);
+
+ if (isInitial)
+ {
+ [initialTrackingResponseForNonExistantItemExpectation fulfill];
+ }
+ else
+ {
+ XCTFail(@"Unexpected non-initial tracking handler invocation (server)");
+ }
+
+ if (serverItem == nil)
+ {
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+ [core stopWithCompletionHandler:^(id sender, NSError *error) {
+ XCTAssert((error==nil), @"Stopped with error: %@", error);
+
+ [coreStoppedExpectation fulfill];
+ }];
+ });
+ }
+ }];
+ }];
+
+ [self waitForExpectationsWithTimeout:60 handler:nil];
+
+ // Erase vault
+ [core.vault eraseSyncWithCompletionHandler:^(id sender, NSError *error) {
+ XCTAssert((error==nil), @"Erased with error: %@", error);
+ }];
+}
+
+- (void)testItemTrackingUnnormalizedPathError
+{
+ OCBookmark *bookmark = [OCTestTarget userBookmark];
+ OCCore *core;
+ XCTestExpectation *coreStartedExpectation = [self expectationWithDescription:@"Core started"];
+ XCTestExpectation *coreStoppedExpectation = [self expectationWithDescription:@"Core stopped"];
+ XCTestExpectation *initialTrackingResponseForNonExistantItemExpectation = [self expectationWithDescription:@"Initial tracking response for non-existant item"];
+ __block id itemTrackerNonExistantItem = nil;
+
+ // Create core
+ core = [[OCCore alloc] initWithBookmark:bookmark];
+ core.automaticItemListUpdatesEnabled = NO;
+
+ // Start core
+ [core startWithCompletionHandler:^(OCCore *core, NSError *error) {
+ core.vault.database.itemFilter = self.databaseSanityCheckFilter;
+
+ XCTAssert((error==nil), @"Started with error: %@", error);
+ [coreStartedExpectation fulfill];
+
+ OCLog(@"Vault location: %@", core.vault.rootURL);
+
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(58 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+ });
+
+ itemTrackerNonExistantItem = [core trackItemAtPath:@"//Photos" trackingHandler:^(NSError * _Nullable error, OCItem * _Nullable serverItem, BOOL isInitial) {
+ OCLog(@"Tracked(NE): isInitial=%d error=%@ item=%@", isInitial, error, serverItem);
+
+ if (isInitial)
+ {
+ [initialTrackingResponseForNonExistantItemExpectation fulfill];
+ }
+ else
+ {
+ XCTFail(@"Unexpected non-initial tracking handler invocation (server)");
+ }
+
+ if ((serverItem == nil) && isInitial && [error isOCErrorWithCode:OCErrorUnnormalizedPath])
+ {
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+ [core stopWithCompletionHandler:^(id sender, NSError *error) {
+ XCTAssert((error==nil), @"Stopped with error: %@", error);
+
+ [coreStoppedExpectation fulfill];
+ }];
+ });
+ }
+ }];
+ }];
+
+ [self waitForExpectationsWithTimeout:60 handler:nil];
+
+ // Erase vault
+ [core.vault eraseSyncWithCompletionHandler:^(id sender, NSError *error) {
+ XCTAssert((error==nil), @"Erased with error: %@", error);
+ }];
+}

- (void)testFavoriteRefresh
{
--
2.21.0 (Apple Git-122)

3 changes: 3 additions & 0 deletions ownCloudSDK/Categories/Foundation/NSString+OCPath.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@

- (OCPath)pathForSubdirectoryWithName:(NSString *)subDirectoryName;

@property(readonly,nonatomic) BOOL isUnnormalizedPath;
@property(readonly,nonatomic) BOOL isNormalizedDirectoryPath;

@end
29 changes: 29 additions & 0 deletions ownCloudSDK/Categories/Foundation/NSString+OCPath.m
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,33 @@ - (OCPath)pathForSubdirectoryWithName:(NSString *)subDirectoryName
return ([[self stringByAppendingPathComponent:subDirectoryName] normalizedDirectoryPath]);
}

- (BOOL)isUnnormalizedPath
{
NSArray<NSString *> *pathComponents = [self componentsSeparatedByString:@"/"];
NSUInteger idx = 0;

if (![self hasPrefix:@"/"])
{
return (YES);
}

for (NSString *pathComponent in pathComponents)
{
if (([pathComponent isEqual:@""] && (idx!=0) && (idx!=(pathComponents.count-1))) || // Multi-slash "//"
[pathComponent isEqual:@"."] || [pathComponent isEqual:@".."]) // ".." or "."
{
return (YES);
}

idx++;
}

return (NO);
}

- (BOOL)isNormalizedDirectoryPath
{
return (![self isUnnormalizedPath] && [self hasSuffix:@"/"]);
}

@end
32 changes: 32 additions & 0 deletions ownCloudSDK/Core/ItemList/OCCore+ItemList.m
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,38 @@ - (void)_finalizeQueryUpdatesWithQueryResults:(NSMutableArray<OCItem *> *)queryR
OCPath queryItemPath = nil;
OCSyncAnchor syncAnchor = nil;

// Queries targeting an item in a subdirectory of taskPath: check if that subdirectory exists
if (taskPath.isNormalizedDirectoryPath && [query.queryPath hasPrefix:taskPath] &&
(task.cachedSet.state == OCCoreItemListStateSuccess) && (task.retrievedSet.state == OCCoreItemListStateSuccess)
)
{
if (query.state != OCQueryStateIdle)
{
NSString *queryPathSubfolder;

if ((queryPathSubfolder = [[query.queryPath substringFromIndex:taskPath.length] componentsSeparatedByString:@"/"].firstObject) != nil)
{
NSString *queryPathSubpath;

if (queryResultItemsByPath == nil)
{
queryResultItemsByPath = [OCCoreItemList itemListWithItems:queryResults].itemsByPath;
}

if ((queryPathSubpath = [taskPath stringByAppendingPathComponent:queryPathSubfolder]) != nil)
{
if ((queryResultItemsByPath[queryPathSubpath] == nil) &&
(queryResultItemsByPath[queryPathSubpath.normalizedDirectoryPath] == nil))
{
// Relevant parent folder is missing
useQueryResults = [NSMutableArray new];
setQueryState = OCQueryStateTargetRemoved;
}
}
}
}
}

// Queries targeting a particular item
if ((queryItemPath = query.queryItem.path) != nil)
{
Expand Down
7 changes: 7 additions & 0 deletions ownCloudSDK/Core/OCCore.m
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,13 @@ - (OCCoreItemTracking)trackItemAtPath:(OCPath)path trackingHandler:(void(^)(NSEr
NSObject *trackingObject = [NSObject new];
__weak NSObject *weakTrackingObject = trackingObject;

// Detect unnormalized path
if ([path isUnnormalizedPath])
{
trackingHandler(OCError(OCErrorUnnormalizedPath), nil, YES);
return (nil);
}

[self queueBlock:^{
NSError *error = nil;
OCItem *item = nil;
Expand Down
Loading

0 comments on commit ea3e8c0

Please sign in to comment.