From 78b2f18fcf6ada621b45c707f114e2cd26e7e33e Mon Sep 17 00:00:00 2001
From: Linnnus <linnnus@users.noreply.github.com>
Date: Sat, 16 Nov 2024 20:15:53 +0100
Subject: [PATCH] lib.generators.toPlist: Make escaping optional as per PR
 comment

As discussed, this patch makes the following changes:

1. Escaping is only done when called with `{ escape = true; }`.
2. A warning is emitted if `{ escape = false; }`
3. Escaping is disabled by default, retaining backwards compatibility.

I have also added a test case for both the escaped and non-escaped
usage.
---
 lib/generators.nix                            | 21 +++++---
 lib/tests/misc.nix                            | 27 ++++++++++-
 ...t => test-to-plist-escaped-expected.plist} |  0
 .../test-to-plist-unescaped-expected.plist    | 48 +++++++++++++++++++
 4 files changed, 88 insertions(+), 8 deletions(-)
 rename lib/tests/{test-to-plist-expected.plist => test-to-plist-escaped-expected.plist} (100%)
 create mode 100644 lib/tests/test-to-plist-unescaped-expected.plist

diff --git a/lib/generators.nix b/lib/generators.nix
index e29bf5a02f3d9e..391492309e9ecc 100644
--- a/lib/generators.nix
+++ b/lib/generators.nix
@@ -549,13 +549,17 @@ in rec {
 
     # Inputs
 
-    Options
-    : Empty set, there may be configuration options in the future
+    Structured function argument
+
+    : escape (optional, default: `false`)
+      : If this option is true, XML special characters are escaped in string values and keys
 
     Value
       : The value to be converted to Plist
   */
-  toPlist = {}: v: let
+  toPlist = {
+    escape ? false
+  }: v: let
     expr = ind: x:
       if x == null  then "" else
       if isBool x   then bool ind x else
@@ -569,10 +573,12 @@ in rec {
 
     literal = ind: x: ind + x;
 
+    maybeEscapeXML = if escape then escapeXML else x: x;
+
     bool = ind: x: literal ind  (if x then "<true/>" else "<false/>");
     int = ind: x: literal ind "<integer>${toString x}</integer>";
-    str = ind: x: literal ind "<string>${escapeXML x}</string>";
-    key = ind: x: literal ind "<key>${escapeXML x}</key>";
+    str = ind: x: literal ind "<string>${maybeEscapeXML x}</string>";
+    key = ind: x: literal ind "<key>${maybeEscapeXML x}</key>";
     float = ind: x: literal ind "<real>${toString x}</real>";
 
     indent = ind: expr "\t${ind}";
@@ -598,7 +604,10 @@ in rec {
       (expr "\t${ind}" value)
     ]) x));
 
-  in ''<?xml version="1.0" encoding="UTF-8"?>
+  in
+  # TODO: As discussed in #356502, we should do the actual deprecation in a future release cycle.
+  lib.warnIf (!escape) "Using `lib.generators.toPlist` without escape = true; is deprecated"
+  ''<?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
 ${expr "" v}
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index fc9bd0a404a966..2128cf17a3334d 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -1635,8 +1635,31 @@ runTests {
     expected  = "«foo»";
   };
 
-  testToPlist = {
+  testToPlistUnescaped = {
     expr = mapAttrs (const (generators.toPlist { })) {
+      value = {
+        nested.values = {
+          int = 42;
+          float = 0.1337;
+          bool = true;
+          emptystring = "";
+          string = "fn\${o}\"r\\d";
+          newlinestring = "\n";
+          path = /. + "/foo";
+          null_ = null;
+          list = [ 3 4 "test" ];
+          emptylist = [];
+          attrs = { foo = null; "foo b/ar" = "baz"; };
+          emptyattrs = {};
+          "keys are not <escaped>" = "and < neither are string values";
+        };
+      };
+    };
+    expected = { value = builtins.readFile ./test-to-plist-unescaped-expected.plist; };
+  };
+
+  testToPlistEscaped = {
+    expr = mapAttrs (const (generators.toPlist { escape = true; })) {
       value = {
         nested.values = {
           int = 42;
@@ -1655,7 +1678,7 @@ runTests {
         };
       };
     };
-    expected = { value = builtins.readFile ./test-to-plist-expected.plist; };
+    expected = { value = builtins.readFile ./test-to-plist-escaped-expected.plist; };
   };
 
   testToLuaEmptyAttrSet = {
diff --git a/lib/tests/test-to-plist-expected.plist b/lib/tests/test-to-plist-escaped-expected.plist
similarity index 100%
rename from lib/tests/test-to-plist-expected.plist
rename to lib/tests/test-to-plist-escaped-expected.plist
diff --git a/lib/tests/test-to-plist-unescaped-expected.plist b/lib/tests/test-to-plist-unescaped-expected.plist
new file mode 100644
index 00000000000000..cad684188bcf1c
--- /dev/null
+++ b/lib/tests/test-to-plist-unescaped-expected.plist
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>nested</key>
+	<dict>
+		<key>values</key>
+		<dict>
+			<key>attrs</key>
+			<dict>
+				<key>foo b/ar</key>
+				<string>baz</string>
+			</dict>
+			<key>bool</key>
+			<true/>
+			<key>emptyattrs</key>
+			<dict>
+
+			</dict>
+			<key>emptylist</key>
+			<array>
+
+			</array>
+			<key>emptystring</key>
+			<string></string>
+			<key>float</key>
+			<real>0.133700</real>
+			<key>int</key>
+			<integer>42</integer>
+			<key>keys are not <escaped></key>
+			<string>and < neither are string values</string>
+			<key>list</key>
+			<array>
+				<integer>3</integer>
+				<integer>4</integer>
+				<string>test</string>
+			</array>
+			<key>newlinestring</key>
+			<string>
+</string>
+			<key>path</key>
+			<string>/foo</string>
+			<key>string</key>
+			<string>fn${o}"r\d</string>
+		</dict>
+	</dict>
+</dict>
+</plist>