Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#278 migration support for database delivered in framework #286

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#278 migration support for database delivered in framework #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Incremental Store/EncryptedStore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ extern NSString * const EncryptedStoreErrorMessageKey;
extern NSString * const EncryptedStoreDatabaseLocation;
extern NSString * const EncryptedStoreCacheSize;
extern NSString * const EncryptedStoreFileManagerOption;
extern NSString * const EncryptedStoreBundleId;

typedef NS_ENUM(NSInteger, EncryptedStoreError)
{
EncryptedStoreErrorIncorrectPasscode = 6000,
Expand Down
22 changes: 16 additions & 6 deletions Incremental Store/EncryptedStore.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
NSString * const EncryptedStoreDatabaseLocation = @"EncryptedStoreDatabaseLocation";
NSString * const EncryptedStoreCacheSize = @"EncryptedStoreCacheSize";
NSString * const EncryptedStoreFileManagerOption = @"EncryptedStoreFileManagerOption";
NSString * const EncryptedStoreBundleId = @"EncryptedStoreBundleId";

static void dbsqliteRegExp(sqlite3_context *context, int argc, const char **argv);
static void dbsqliteStripCase(sqlite3_context *context, int argc, const char **argv);
Expand Down Expand Up @@ -965,12 +966,21 @@ - (BOOL)loadMetadata:(NSError **)error {
if ([newModel isConfiguration:nil compatibleWithStoreMetadata:metadata]){
return YES;
}

// load the old model:
NSMutableArray *bundles = [NSMutableArray array];
NSBundle *bundle = self.fileManager.configuration.bundle;
[bundles addObject:bundle];
NSManagedObjectModel *oldModel = [NSManagedObjectModel mergedModelFromBundles:bundles

// load the old model
NSBundle *modelBundle;
NSString *bundleId = [options objectForKey:EncryptedStoreBundleId];
if (bundleId.length > 0)
{
modelBundle = [NSBundle bundleWithIdentifier:bundleId];
}

if (modelBundle == nil)
{
modelBundle = [NSBundle mainBundle];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the main bundle is fetched and then later added to an array on line 983. However according to Apple documentation on the mainBundle method the method can return nil. Therefore an error will occur when attempting to add nil to an array.

This method may return a valid bundle object even for unbundled apps. It may also return nil if the bundle object could not be created, so always check the return value.

You could add a additional check and error code for this case if you wanted to make this code as resilient as possible.

typedef NS_ENUM(NSInteger, EncryptedStoreError)
{
    EncryptedStoreErrorIncorrectPasscode = 6000,
    EncryptedStoreErrorMigrationFailed,
    EncryptedStoreErrorModelLoadFailed
};

if (modelBundle == nil)
{
    if (error)
    {
        NSDictionary * userInfo = @{EncryptedStoreErrorMessageKey : @"Main Bundle not found, cannot load model"};
        *error = [NSError errorWithDomain: EncryptedStoreErrorDomain 
                                     code: EncryptedStoreErrorModelLoadFailed 
                                 userInfo: userInfo];
    }
    return NO;
}

Copy link
Contributor Author

@tomasz-czyzak tomasz-czyzak Mar 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree with you about handling mainBundle method however it is kind of overkill since if mainBundle is nil then we are in more serious troubles than handling error correctly,
Also getting modelBundle from mainBundle assumes that model is delivered inside app while issue which has been fixed by this PR is to migrate correctly model delivered in framework

Anyway feel free to raise separate PR for fixing mainBundle potential nil return.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, perhaps it would be an extreme edge case.

}

NSManagedObjectModel *oldModel = [NSManagedObjectModel mergedModelFromBundles:@[modelBundle]
forStoreMetadata:metadata];

if (oldModel && newModel) {
Expand Down