Crashing on passwordObject

[brennanMKE]

I am using the code currently on the branch but when unarchiveObjectWithData: is called it causes an exception. I cannot explain it and I do not know how to get around it. The exception provides no information in the latest version of Xcode.

Any ideas?

• (id)passwordObject {
  if ([self.passwordData length]) {
  return [NSKeyedUnarchiver unarchiveObjectWithData:self.passwordData];
  }
  return nil;
  }

[brennanMKE]

I managed to get more details. It apparently does not like the archive. I will have to figure out how to determine if the NSData value will cause a crash or not before trying to use it. It would probably a good idea to ensure this method does a check as well.

[NSKeyedUnarchiver initForReadingWithData:]: incomprehensible archive (0x61, 0x62, 0x63, 0x31, 0x32, 0x33, 0x0, 0x0)'
*** First throw call stack:
(0x335df3e7 0x3b2da963 0x335df307 0x33e779d1 0x33e7f991 0x62ee5 0x35aef 0x35267 0x333df 0x3544eaa1 0x3544e625 0x35446833 0x353eed1f 0x353ee7ad 0x353ee1ef 0x371065f7 0x37106227 0x335b43e7 0x335b438b 0x335b320f 0x3352623d 0x335260c9 0x3544546d 0x354422b9 0x33295 0x3b707b20)
libc++abi.dylib: terminate called throwing an exception

[calebd]

Is the item you are trying to fetch through passwordObject one that you created through that same property, or some other way? For example, setting a password with password or passwordData, then calling passwordObject on that same item, or another fetched instance of that item, would be a programmer error.

[calebd]

Basically, in order for passwordObject to successfully unarchive the data, it has to be created as an archive in the first place.

[brennanMKE]

I am working on figuring it out.

[calebd]

The tests in the password_object pass so if you find something that's broken let me know!

[brennanMKE]

I cleared my keychain and now I can save and fetch the passwordObject property without the exception.

It may have been caused by the data being saved with different formats, though I do not understand it since if it was stored as NSData, NSString or NSDictionary it should not fail with an exception. I wonder if there is a safer way to unarchive data from the keychain. Since there were items stored prior to moving back to SSKeyChain those may have been the cause.

[calebd]

Again, NSKeyedUnarchiver will only be able to read blobs created by NSKeyedArchiver. So it would not be able to read a blob created previously by interpreting a string as UTF-8 data, or JSON data for example. It is completely safe if you run it through the same archive / unarchive process every time. Going forward, you can set a string object on passwordObject instead of password, and it will work properly but we can't expect NSKeyedUnarchiver to be able to take any data it is handed and give us something back.

[brennanMKE]

Do you know of a way to determine what the blob is so if it is not something NSKeyedUnarchiver can handle it simply does not try? Crashing seems like a harsh penalty. I'd prefer it to provide a method which can set an error reference.

[paulmelnikow]

@brennanMKE, have you tried wrapping the call in a @try block?

[calebd]

It uses NSPropertyListBinaryFormat_v1_0 to encode and decode the data. I guess you could validate that yourself or something? Seems like a lot of work and I really don't anticipate anyone running into this any more than say trying to JSON decode arbitrary data (which, incidentally, also throws an exception).

[brennanMKE]

The way I would see NSKeyedUnarchiver being a real problem is initially storing the password as an NSString in version 1.0 of the application and then in 2.0 it is stored as an NSDictionary using NSCoding. Unarchiving the NSString would cause it to crash, so being able to detect what format it would help.

I've seen apps which have failed and even crashed with new versions possibly because of a keychain issue like this.

To prevent this problem what I may do is use a Service name of AppName-1.0 for version 1.0 and when version 2.0 comes out I would migrate all accounts in the AppName-1.0 Service over to AppName-2.0 and serialize the passwordObject appropriate for that version, then delete all accounts under AppName-1.0.

This could even make sense if both versions are serializing an NSDictionary but the new version has a different value or a modified structure.

It seems like a lot of work though. Do you think this is the only way to go?

[calebd]

Right. I think about it the same as I would any database schema. I first try loading current data. If none is found you fallback, migrate to the newest form, then delete them once you successfully save. Going forward you could include a version number in the password object (if you are using a dictionary) that can indicate what the contents look like.

[brennanMKE]

I think I will do that and always use an NSDictionary and use one of the values as the version as you suggest.

Thanks for the sanity check.