crash with "[__NSTaggedDate UTF8String]" error in some cases

[Zajkowski, James]

Hi all,

I'm having an issue with munki not notifying the user under 10.7 - I've not verified this in 10.6.

The issue seems to be that the LastNotifiedDate field gets unarchived as an NSDate, not an NSString, and NSDates don't have the method -UTF8String.

Traceback points to the call to NSDate.dateWithString_(lastNotifiedString) on line 274 of managedsoftwareupdates - lastNotifiedString is already an NSDate, and apparently some kind of silent type coercion is trying to change lastNotifiedString into a string by sending -UTF8String.

This is a trivial patch that just removes the intermediate string object. It seems to work fine for me on 10.7.

--Jim

diff --git a/code/client/managedsoftwareupdate b/code/client/managedsoftwareupdate
index a2bc98c..a9a97e1 100755
--- a/code/client/managedsoftwareupdate
+++ b/code/client/managedsoftwareupdate
@@ -266,12 +266,11 @@ def notifyUserOfUpdates(force=False):
# someone is logged in, and we have updates.
# if we haven't notified in a while, notify:
user_was_notified = False
- lastNotifiedString = munkicommon.pref('LastNotifiedDate')
+ lastNotifiedDate = munkicommon.pref('LastNotifiedDate')
daysBetweenNotifications = munkicommon.pref('DaysBetweenNotifications')
now = NSDate.new()
nextNotifyDate = now
- if lastNotifiedString:
- lastNotifiedDate = NSDate.dateWithString_(lastNotifiedString)
+ if lastNotifiedDate:
interval = daysBetweenNotifications * (24 * 60 * 60)
if daysBetweenNotifications > 0:
# we make this adjustment so a 'daily' notification
--
1.7.3.4

[Greg Neagle]

On Aug 2, 2011, at 9:35 AM, Zajkowski, James wrote:

> Hi all,
>
> I'm having an issue with munki not notifying the user under 10.7 - I've not verified this in 10.6.

I think I've seen this as well but haven't had time to investigate.

>
> The issue seems to be that the LastNotifiedDate field gets unarchived as an NSDate, not an NSString, and NSDates don't have the method -UTF8String.

At one point FoundationPlist did all the reading of /Library/Preferences/ManagedInstalls.plist and converted all NSDate/CFDates to strings (which worked around a problem I've since forgotten; perhaps we needed to convert to Python date objects.) In most cases we have moved to using NSDate methods when working with dates instead of the Python date methods now.

Now we are using CFPreferencesCopyAppValue instead of NSPropertyListSerialization.propertyListFromData_mutabilityOption_format_errorDescription_,
and perhaps the types being returned are different so that this code no longer does what is intended:

if type(pref_value).__name__ in ['__NSCFDate', '__NSDate', '__CFDate']:
# convert NSDate/CFDates to strings
pref_value = str(pref_value)

I think the right thing is indeed to stop converting NS/CFDates to strings, but we'll need to scour all the code to remove everything that is doing the conversions -- especially anything still converting stuff to Python dates.

[Greg Neagle]

Yup, dates are unarchived as different types under Lion:

root# cd /usr/local/munki/munkilib/
shrewd:munkilib root# python
Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05)
[GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import munkicommon
>>> type(munkicommon.pref('LastNotifiedDate'))
<objective-c class __NSTaggedDate at 0x7fff77dee588>

compare with Snow Leopard:

% cd /usr/local/munki/munkilib/
gneagle@mariobros:munkilib % python
Python 2.6.1 (r261:67515, Jun 24 2010, 21:47:49)
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import munkicommon
>>> type(munkicommon.pref('LastNotifiedDate'))
<type 'str'>

(The unarchived date's type must have matched one of ['__NSCFDate', '__NSDate', '__CFDate'])

So your patch will break on 10.5/10.6. We need to go through and fix everything to remove the date-to-string-and-back conversions, but a quick fix for now would be to add "__NSTaggedDate" to the list of date classes we convert:

In munkicommon.py, lines 833-835 (The pref() function):

if type(pref_value).__name__ in ['__NSCFDate', '__NSDate', '__CFDate', '__NSTaggedDate']:

# convert NSDate/CFDates to strings
pref_value = str(pref_value)

[Greg Neagle]

I've verified my quick fix addresses the issue under Lion and checked it into trunk as r1261.

We'll readdress the general NSDate issues at a later date.

-Greg

[Zajkowski, James]

My python is limited to cargo-coding, but it feels like this should be coded to identify concrete classes in a class cluster rather than having the concrete classes be specifically enumerated; NSObject's -isKindOfClass: comes to mind.

Apologies about the patch not working on 10.6, we're replacing our rather hairy NetInstall-backed system with munki starting with 10.7, so I don't have a test machine running 10.6 at the moment.

--Jim

[Greg Neagle]

On Aug 2, 2011, at 11:23 AM, Zajkowski, James wrote:

> My python is limited to cargo-coding, but it feels like this should be coded to identify concrete classes in a class cluster rather than having the concrete classes be specifically enumerated; NSObject's -isKindOfClass: comes to mind.

And my Cocoa is similarly weak. I don't know what the "class cluster" that would include all those subclasses might be.

-Greg

[Greg Neagle]

I'm guessing you are suggesting something like:

# python
Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05)
[GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

>>> from Foundation import NSDate
>>> from CoreFoundation import CFPreferencesCopyAppValue
>>> d = CFPreferencesCopyAppValue('LastNotifiedDate', 'ManagedInstalls')
>>> d.isKindOfClass_(NSDate)
True

-Greg

[Zajkowski, James]

NSDate is the principal class for that group. My suggestion would be rather than doing a string comparison on the list of classes, either use python's isinstance(obj, class) or send isKindOfClass_:

Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05)
[GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import munkicommon

>>> from Foundation import *
>>> d = munkicommon.pref('LastNotifiedDate')
>>> type(d)
<objective-c class __NSTaggedDate at 0x7fff7bec5588>
>>> d.isKindOfClass_(NSDate)
True
>>> isinstance(d, NSDate)
True

This patch does that:

code/client/munkilib/munkicommon.py | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/code/client/munkilib/munkicommon.py b/code/client/munkilib/munkicommon.py
index 8b2ae64..8453c69 100644
--- a/code/client/munkilib/munkicommon.py
+++ b/code/client/munkilib/munkicommon.py
@@ -830,8 +830,7 @@ def pref(pref_name):
# /Library/Preferences/<BUNDLE_ID>.plist for admin
# discoverability
set_pref(pref_name, pref_value)
- if type(pref_value).__name__ in ['__NSCFDate', '__NSDate', '__CFDate',
- '__NSTaggedDate']:
+ if isinstance(pref_value, NSDate):

# convert NSDate/CFDates to strings
pref_value = str(pref_value)

return pref_value
--
1.7.3.4

[Greg Neagle]

Thanks -- that seems a good approach for now.

-Greg