unsignedapps/Vexil

Crash when editing optional enum flag in Vexilographer

Opened this issue · 8 comments

orj commented

Optional enum flags seem to Crash Vexilographer when going into edit mode.

Vexil version: 2.2.1
Swift version: swift-driver version: 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
Environment: Version 14.3.1 (14E300c)

✅ Checklist

- [ ] If possible, I've reproduced the issue using the main branch of this package

🔢 Steps to Reproduce

Replace this paragraph with an explanation of how to reproduce the incorrect behavior. This could include a code listing for a reduced version of your command, or a link to the code that is exhibiting the issue.

🎯 Expected behavior

No crash. Things work as expected.

🕵️‍♀️ Actual behavior

I have defined a flag like this:

@Flag(name: "Application User Mode", default: nil, description: "The application mode for users")
    public var applicationUserMode: ApplicationUserMode?

The ApplicationUserMode is defined as:

public enum ApplicationUserMode: String {
    /// Users are enterprise employees
    case enterpriseEmployees

    /// Users are Apple App Store subscribers
    case individualSubscribers
}

extension ApplicationUserMode: FlagValue, FlagDisplayValue {
    public var flagDisplayValue: String {
        switch self {
        case .enterpriseEmployees:
            return "Enterprise Employees"
        case .individualSubscribers:
            return "Individual Subscribers"
        }
    }
}

When trying to edit this in Vexilographer it crashes with the following stack trace.

[User Defaults] Attempt to set a non-property-list object <null> as an NSUserDefaults/CFPreferences value for key application-user-mode
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'Attempt to insert non-property list object null for key application-user-mode'
*** First throw call stack:
(
	0   CoreFoundation                      0x0000000180437330 __exceptionPreprocess + 172
	1   libobjc.A.dylib                     0x0000000180051274 objc_exception_throw + 56
	2   CoreFoundation                      0x00000001804bd1c8 -[__NSCFString characterAtIndex:].cold.1 + 0
	3   CoreFoundation                      0x00000001803f7158 _CFPrefsValidateValueForKey + 216
	4   CoreFoundation                      0x00000001803f77d4 createDeepCopyOfValueForKey + 160
	5   CoreFoundation                      0x00000001803f74f0 -[CFPrefsSource setValues:forKeys:count:copyValues:removeValuesForKeys:count:from:] + 320
	6   CoreFoundation                      0x00000001803f78b8 -[CFPrefsSource setValue:forKey:from:] + 68
	7   CoreFoundation                      0x00000001804a0338 __76-[_CFXPreferences setValue:forKey:appIdentifier:container:configurationURL:]_block_invoke + 56
	8   CoreFoundation                      0x000000018045fbe4 __108-[_CFXPreferences(SearchListAdditions) withSearchListForIdentifier:container:cloudConfigurationURL:perform:]_block_invoke + 344
	9   CoreFoundation                      0x000000018045f384 normalizeQuintuplet + 348
	10  CoreFoundation                      0x000000018045fa64 -[_CFXPreferences withSearchListForIdentifier:container:cloudConfigurationURL:perform:] + 124
	11  CoreFoundation                      0x00000001804a02d8 -[_CFXPreferences setValue:forKey:appIdentifier:container:configurationURL:] + 100
	12  CoreFoundation                      0x00000001804a4578 _CFPreferencesSetAppValueWithContainerAndConfiguration + 116
	13  Foundation                          0x0000000180bd06e4 -[NSUserDefaults(NSUserDefaults) setObject:forKey:] + 64
	14  ThePeopleSpotSettings               0x000000010499e97c $sSo14NSUserDefaultsC5VexilE12setFlagValue_3keyyxSg_SStKAC0eF0RzlF + 492
	15  ThePeopleSpotSettings               0x000000010499f24c $sSo14NSUserDefaultsC5Vexil15FlagValueSourceA2cDP03setdE0_3keyyqd__Sg_SStKAC0dE0Rd__lFTW + 24
	16  ThePeopleSpotSettings               0x000000010498d814 $s5Vexil4FlagV4save2toyAA0B11ValueSource_p_tKF + 268
	17  ThePeopleSpotSettings               0x000000010498d900 $s5Vexil4FlagVyxGAA03AnyB0A2aEP4save2toyAA0B11ValueSource_p_tKFTW + 40
	18  ThePeopleSpotSettings               0x000000010498b4f4 $s5Vexil8FlagPoleC4save8snapshot2toyAA8SnapshotCyxG_AA0B11ValueSource_ptKFyAA03AnyB0_pKXEfU_ + 92
	19  ThePeopleSpotSettings               0x000000010498b54c $s5Vexil8FlagPoleC4save8snapshot2toyAA8SnapshotCyxG_AA0B11ValueSource_ptKFyAA03AnyB0_pKXEfU_TA + 24
	20  libswiftCore.dylib                  0x000000018bd57490 $sSTsE7forEachyyy7ElementQzKXEKF + 440
	21  ThePeopleSpotSettings               0x000000010498b430 $s5Vexil8FlagPoleC4save8snapshot2toyAA8SnapshotCyxG_AA0B11ValueSource_ptKF + 180
	22  ThePeopleSpotSettings               0x0000000104e20068 $s14Vexillographer16FlagValueManagerC03setbC0_3keyyqd__Sg_SStK5Vexil0bC0Rd__lF + 296
	23  ThePeopleSpotSettings               0x0000000104e206a0 $s14Vexillographer16FlagValueManagerC08setBoxedC0_4type3keyy0fC4TypeQyd__Sg_qd__mSStK5Vexil0bC0Rd__lF + 404
	24  ThePeopleSpotSettings               0x0000000104dea5dc $s7SwiftUI7BindingV14VexillographerE3key7manager12defaultValue11transformerACyxGSS_AD04FlagH7ManagerCyqd_0_Gqd_1_qd__mtc07EditingH0Qyd__RszAD05BoxedjH11TransformerRd__5Vexil0J9ContainerRd_0_AP0jH0Rd_1_0mH4TypeQyd_1_08OriginalH0Rtd__r1_lufcyxcfU0_ + 412
	25  ThePeopleSpotSettings               0x0000000104dea95c $s7SwiftUI7BindingV14VexillographerE3key7manager12defaultValue11transformerACyxGSS_AD04FlagH7ManagerCyqd_0_Gqd_1_qd__mtc07EditingH0Qyd__RszAD05BoxedjH11TransformerRd__5Vexil0J9ContainerRd_0_AP0jH0Rd_1_0mH4TypeQyd_1_08OriginalH0Rtd__r1_lufcyxcfU0_TA + 76
orj commented

I have noticed in extension UserDefaults: FlagValueSource method setFlagValue<Value>() that value is a ApplicationUserMode?? (note the double optional). The outer optional is non-nil as it contains the inner nil. So the initial guard let value = value passes but when attempting to insert the value.boxedFlagValue it resolves to nil.

orj commented

Perhaps I'm holding it wrong?

orj commented

I note that if I make the enum CaseIterable it doesn't crash. But the editor is a bit "meh" and doesn't work quite right.

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-05.at.17.01.42.mp4
huwr commented

I feel like the last problem could perhaps be addressed or improved by #112

Pretty sure the crash occurs here:

public func setFlagValue<Value>(_ value: Value?, key: String) throws where Value: FlagValue {
guard let value = value else {
removeObject(forKey: key)
return
}
set(value.boxedFlagValue.object, forKey: key)
}

One way to stop the crash would be to use nil when .boxedFlagValue.object == NSNull():

if value.boxedFlagValue.object == NSNull() {
  set(nil, forKey: key)
} else {
  set(value.boxedFlagValue.object, forKey: key)
}

This doesn't quite work for setting optional values though. It simply removes the flag rather than setting it to nil. Using Data() instead of nil could be an option. I'm not 100% on how UserDefaults differs across platforms and not too familiar with Vexil so opted to leave it out of #112.

A small change to OptionalCaseIterableFlagControl would also be needed.

@orj Here's a branch with the above changes. Any chance you could give it a test drive and make sure it doesn't break anything else? I was only testing with an optional case iterable control too.
main...patANZx:Vexil:fix/vexillographer-optional-flags

I have noticed in extension UserDefaults: FlagValueSource method setFlagValue() that value is a ApplicationUserMode?? (note the double optional). The outer optional is non-nil as it contains the inner nil. So the initial guard let value = value passes but when attempting to insert the value.boxedFlagValue it resolves to nil.

@orj Oh I missed this message. Didn't realise you had already figured it out :P Not sure if the above fix is suitable but it's been working okay with the OptionalCaseIterableFlagControl for me.

huwr commented

The double-optional problem is something I also encountered but never bothered to investigate: #109 so if deemed suitable your solution would fix a few

@huwr I'm guessing some more work would need to be done with the text editor to get it working fully. I haven't used it much but I remember having a hard time with optional numbers.