diff --git a/ChangeLog b/ChangeLog index 0604f3de..c915f44b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,30 @@ +2026-04-13 Todd White + + * Source/CFPropertyList.c: Fix CFPropertyListCreateDeepCopy for + mutable-copy options. Two related bugs prevented the function + from working: + (1) The function dispatches on file-scope _kCF*TypeID variables + that are lazily populated by CFPropertyListInitTypeIDs. The + only other caller that initialised those variables was + CFPlistTypeIsValid, so CFPropertyListCreateDeepCopy would drop + into its trailing `else` branch and return NULL whenever it + was the first plist-API call in the process. Add an explicit + CFPropertyListInitTypeIDs() at the top of the function. + (2) The mutable-array and mutable-dictionary branches called + CFArrayApplyFunction / CFDictionaryApplyFunction on the newly + created empty destination container rather than on the source + plist. The apply function therefore visited zero elements and + every mutable deep copy came back empty. On Linux the + iteration also read uninitialised slots in the destination + container and crashed via CFRetain(NULL) inside + CFArrayCopyFunction. Iterate on `plist` in both branches so + the copy actually walks the source values. + * Tests/CFPropertyList/deepcopy.m: New regression test covering + the mutable-array copy path (count, element identity, genuine + mutability via append), the immutable-array copy path (guards + the branch that was never broken), the mutable-dictionary copy + path, and a nested mutable deep copy. + 2021-09-30 Frederik Seiffert * Source/CFDate.c: Fix logic in CFGregorianDateIsValid() diff --git a/Source/CFPropertyList.c b/Source/CFPropertyList.c index 9ffa1f98..9d24c870 100644 --- a/Source/CFPropertyList.c +++ b/Source/CFPropertyList.c @@ -257,6 +257,17 @@ CFPropertyListCreateDeepCopy (CFAllocatorRef alloc, CFPropertyListRef plist, CFPropertyListRef copy; CFTypeID typeID; + /* The function dispatches on file-scope CFTypeID variables that are + * lazily populated by CFPropertyListInitTypeIDs. If this is the + * first plist-API call in the process those variables are all zero, + * every `typeID == _kCF*TypeID` comparison fails, and the function + * drops into the trailing `else` branch and returns NULL. The only + * other caller that initialised the IDs was CFPlistTypeIsValid, so + * the bug only surfaced for code paths that did not happen to go + * through a validation call first. + */ + CFPropertyListInitTypeIDs (); + typeID = CFGetTypeID (plist); if (typeID == _kCFArrayTypeID) { @@ -293,7 +304,12 @@ CFPropertyListCreateDeepCopy (CFAllocatorRef alloc, CFPropertyListRef plist, ctx.alloc = alloc; ctx.container = (CFTypeRef) array; range = CFRangeMake (0, cnt); - CFArrayApplyFunction (array, range, CFArrayCopyFunction, &ctx); + /* Iterate over the source array `plist`, not the empty + * destination `array` - otherwise the apply function visits + * zero elements and the mutable deep copy always comes back + * empty. + */ + CFArrayApplyFunction (plist, range, CFArrayCopyFunction, &ctx); copy = array; } @@ -338,7 +354,12 @@ CFPropertyListCreateDeepCopy (CFAllocatorRef alloc, CFPropertyListRef plist, ctx.opts = opts; ctx.alloc = alloc; ctx.container = (CFTypeRef) dict; - CFDictionaryApplyFunction (dict, CFDictionaryCopyFunction, &ctx); + /* Iterate over the source dictionary `plist`, not the empty + * destination `dict` - otherwise the apply function visits + * zero entries and the mutable deep copy always comes back + * empty. Same bug as the array branch above. + */ + CFDictionaryApplyFunction (plist, CFDictionaryCopyFunction, &ctx); copy = dict; } diff --git a/Tests/CFPropertyList/deepcopy.m b/Tests/CFPropertyList/deepcopy.m new file mode 100644 index 00000000..12fd6543 --- /dev/null +++ b/Tests/CFPropertyList/deepcopy.m @@ -0,0 +1,182 @@ +/* + * deepcopy.m - regression test for CFPropertyListCreateDeepCopy mutable copies. + * + * Before the fix, the mutable-copy branch of CFPropertyListCreateDeepCopy + * called the apply function on the *destination* container - which had + * just been created empty - rather than on the source. The apply + * function therefore visited zero elements and the result came back + * empty. Both the array and the dictionary cases shared the same bug. + * + * This test exercises: + * + * - a mutable deep copy of an array preserves element count, order, + * and value equality; + * - a mutable deep copy of an array is actually mutable (i.e. the + * returned type is genuinely a mutable container, not an immutable + * one reinterpret-cast as mutable); + * - a mutable deep copy of a dictionary preserves key/value count and + * value equality by key; + * - a mutable deep copy of a dictionary is actually mutable; + * - an immutable deep copy of an array preserves its elements (the + * non-buggy branch - guards against a fix that accidentally + * regresses the already-working path); + * - a mutable deep copy of a nested container preserves the nested + * contents (since the recursive descent goes back through the same + * mutable-copy path, an off-by-nothing bug at the outer level would + * not reliably show up in a flat test). + * + * All tests use only the public CoreFoundation C API - no Objective-C + * runtime assumptions, no blocks, no platform-specific system calls - + * so the test builds under every compiler GNUstep's libs-corebase + * supports. + */ + +#include +#include +#include +#include +#include +#include "../CFTesting.h" + +int main (void) +{ + CFStringRef s1; + CFStringRef s2; + SInt32 i32; + CFNumberRef n; + const void *aValues[3]; + CFArrayRef srcArray; + CFMutableArrayRef mArrayCopy; + CFArrayRef iArrayCopy; + CFMutableDictionaryRef srcDict; + CFMutableDictionaryRef mDictCopy; + CFArrayRef inner; + CFMutableArrayRef outer; + CFMutableArrayRef mOuterCopy; + + s1 = CFSTR("hello"); + s2 = CFSTR("world"); + i32 = 42; + n = CFNumberCreate (NULL, kCFNumberSInt32Type, &i32); + + aValues[0] = s1; + aValues[1] = s2; + aValues[2] = n; + srcArray = CFArrayCreate (NULL, aValues, 3, &kCFTypeArrayCallBacks); + + /* Array: mutable deep copy preserves count. */ + mArrayCopy = (CFMutableArrayRef) CFPropertyListCreateDeepCopy (NULL, + srcArray, kCFPropertyListMutableContainersAndLeaves); + PASS_CF(mArrayCopy != NULL, + "mutable deep copy of a 3-element array is non-NULL"); + PASS_CF(CFArrayGetCount (mArrayCopy) == 3, + "mutable deep copy of a 3-element array has 3 elements " + "(was 0 before the CFArrayApplyFunction source/destination fix)"); + + /* Array: element values match. */ + if (mArrayCopy != NULL && CFArrayGetCount (mArrayCopy) == 3) + { + PASS_CFEQ(CFArrayGetValueAtIndex (mArrayCopy, 0), s1, + "mutable array deep copy preserves element 0"); + PASS_CFEQ(CFArrayGetValueAtIndex (mArrayCopy, 1), s2, + "mutable array deep copy preserves element 1"); + PASS_CFEQ(CFArrayGetValueAtIndex (mArrayCopy, 2), n, + "mutable array deep copy preserves element 2"); + } + + /* Array: the result is genuinely mutable - if the fix is applied, + * appending to the copy should succeed and the count should grow. */ + if (mArrayCopy != NULL) + { + CFArrayAppendValue (mArrayCopy, s1); + PASS_CF(CFArrayGetCount (mArrayCopy) == 4, + "mutable array deep copy is genuinely mutable (append grows count)"); + CFRelease (mArrayCopy); + } + + /* Array: immutable deep copy was never broken - guard it against a + * regression that mistakenly fixes both branches. */ + iArrayCopy = (CFArrayRef) CFPropertyListCreateDeepCopy (NULL, srcArray, + kCFPropertyListImmutable); + PASS_CF(iArrayCopy != NULL, + "immutable deep copy of an array is non-NULL"); + PASS_CF(iArrayCopy != NULL && CFArrayGetCount (iArrayCopy) == 3, + "immutable deep copy preserves count (non-buggy branch still works)"); + if (iArrayCopy != NULL) + { + CFRelease (iArrayCopy); + } + + /* Dictionary: mutable deep copy preserves count. The audit caught + * the array bug but missed the identical bug in the dictionary + * branch of CFPropertyListCreateDeepCopy - this test covers it. */ + srcDict = CFDictionaryCreateMutable (NULL, 0, + &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + CFDictionaryAddValue (srcDict, CFSTR("first"), s1); + CFDictionaryAddValue (srcDict, CFSTR("second"), s2); + CFDictionaryAddValue (srcDict, CFSTR("number"), n); + + mDictCopy = (CFMutableDictionaryRef) CFPropertyListCreateDeepCopy (NULL, + srcDict, kCFPropertyListMutableContainersAndLeaves); + PASS_CF(mDictCopy != NULL, + "mutable deep copy of a 3-entry dictionary is non-NULL"); + PASS_CF(mDictCopy != NULL && CFDictionaryGetCount (mDictCopy) == 3, + "mutable deep copy of a 3-entry dictionary has 3 entries " + "(was 0 before the CFDictionaryApplyFunction source/destination fix)"); + + /* Dictionary: lookups by key return the right values. */ + if (mDictCopy != NULL && CFDictionaryGetCount (mDictCopy) == 3) + { + PASS_CFEQ(CFDictionaryGetValue (mDictCopy, CFSTR("first")), s1, + "mutable dict deep copy preserves value for \"first\""); + PASS_CFEQ(CFDictionaryGetValue (mDictCopy, CFSTR("second")), s2, + "mutable dict deep copy preserves value for \"second\""); + PASS_CFEQ(CFDictionaryGetValue (mDictCopy, CFSTR("number")), n, + "mutable dict deep copy preserves value for \"number\""); + } + + /* Dictionary: the result is genuinely mutable. */ + if (mDictCopy != NULL) + { + CFDictionaryAddValue (mDictCopy, CFSTR("extra"), s1); + PASS_CF(CFDictionaryGetCount (mDictCopy) == 4, + "mutable dict deep copy is genuinely mutable (add grows count)"); + CFRelease (mDictCopy); + } + + CFRelease (srcDict); + + /* Nested: a mutable deep copy of an array-containing-array must + * preserve both the outer and inner contents. Because + * CFPropertyListCreateDeepCopy recurses through itself, a nested + * test catches bugs where the fix only works at the top level. */ + inner = srcArray; + aValues[0] = inner; + aValues[1] = s1; + outer = CFArrayCreateMutable (NULL, 2, &kCFTypeArrayCallBacks); + CFArrayAppendValue (outer, inner); + CFArrayAppendValue (outer, s1); + + mOuterCopy = (CFMutableArrayRef) CFPropertyListCreateDeepCopy (NULL, outer, + kCFPropertyListMutableContainersAndLeaves); + PASS_CF(mOuterCopy != NULL, + "mutable deep copy of a nested array is non-NULL"); + PASS_CF(mOuterCopy != NULL && CFArrayGetCount (mOuterCopy) == 2, + "outer nested array deep copy preserves count"); + if (mOuterCopy != NULL && CFArrayGetCount (mOuterCopy) == 2) + { + CFArrayRef nestedCopy + = (CFArrayRef) CFArrayGetValueAtIndex (mOuterCopy, 0); + PASS_CF(nestedCopy != NULL && CFArrayGetCount (nestedCopy) == 3, + "inner nested array is also deep-copied with all elements"); + } + if (mOuterCopy != NULL) + { + CFRelease (mOuterCopy); + } + + CFRelease (outer); + CFRelease (srcArray); + CFRelease (n); + return 0; +}