Fix CFPropertyListCreateDeepCopy for mutable-copy options

ChangeLog

@@ -1,3 +1,30 @@
+2026-04-13  Todd White <todd.white@thalion.global>
+
+	* 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 <frederik@algoriddim.com>
 	* Source/CFDate.c: Fix logic in CFGregorianDateIsValid()
 

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;
         }

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 <CoreFoundation/CFPropertyList.h>
+#include <CoreFoundation/CFArray.h>
+#include <CoreFoundation/CFDictionary.h>
+#include <CoreFoundation/CFString.h>
+#include <CoreFoundation/CFNumber.h>
+#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;
+}