lib.generators.toPlist: escape XML syntax in strings & keys

Before this patch, code like this would break generate invalid XML: lib.generators.toPlist {} "ab<cd" That's obviously bad, since the call to toPlist often happens through indirection, as is the case in e.g. the nix-darwin project. A user might not realize that they have to escape the strings. This patch adds the argument 'escape' to lib.generators.plist and emits a warning if it is not set to true. In a future release, this behavior should become the default.
NixOS · Nov 22, 2024 · 9da2f54 · 9da2f54
1 parent 122adac
commit 9da2f54
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 9 deletions.
diff --git a/lib/generators.nix b/lib/generators.nix
@@ -70,6 +70,7 @@ let
     split
     toJSON
     typeOf
+    escapeXML
     ;
 
   ## -- HELPER FUNCTIONS & DEFAULTS --
@@ -548,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
@@ -568,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>${x}</string>";
-    key = ind: x: literal ind "<key>${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}";
@@ -597,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
@@ -1635,7 +1635,7 @@ runTests {
     expected  = "«foo»";
   };
 
-  testToPlist = {
+  testToPlistUnescaped = {
     expr = mapAttrs (const (generators.toPlist { })) {
       value = {
         nested.values = {
@@ -1651,10 +1651,34 @@ runTests {
           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-expected.plist; };
+    expected = { value = builtins.readFile ./test-to-plist-unescaped-expected.plist; };
+  };
+
+  testToPlistEscaped = {
+    expr = mapAttrs (const (generators.toPlist { escape = true; })) {
+      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 <escaped>" = "and < so are string values";
+        };
+      };
+    };
+    expected = { value = builtins.readFile ./test-to-plist-escaped-expected.plist; };
   };
 
   testToLuaEmptyAttrSet = {

diff --git a/lib/tests/test-to-plist-escaped-expected.plist b/lib/tests/test-to-plist-escaped-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 &lt;escaped&gt;</key>
+			<string>and &lt; so 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}&quot;r\d</string>
+		</dict>
+	</dict>
+</dict>
+</plist>
diff --git a/lib/tests/test-to-plist-expected.plist → ...ts/test-to-plist-unescaped-expected.plist b/lib/tests/test-to-plist-expected.plist → ...ts/test-to-plist-unescaped-expected.plist
@@ -27,6 +27,8 @@
 			<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>
@@ -43,4 +45,4 @@
 		</dict>
 	</dict>
 </dict>
-</plist>
+</plist>