Limit escaping to only quotes and trailing backslashes

Author: lukaszsamson

lib/mix/lib/mix/utils.ex

@@ -482,28 +482,39 @@ defmodule Mix.Utils do
   end
 
   defp quoted(data) do
-    escaped_data =
-      data
-      |> to_string()
-      # escape \
-      |> String.replace("\\", "\\\\")
-      # escape " (required in DOT quoted strings)
-      |> String.replace("\"", "\\\"")
-      # escape other special characters
-      |> String.replace("\0", "\\0")
-      |> String.replace("\a", "\\a")
-      |> String.replace("\b", "\\b")
-      |> String.replace("\d", "\\d")
-      |> String.replace("\e", "\\e")
-      |> String.replace("\f", "\\f")
-      |> String.replace("\n", "\\n")
-      |> String.replace("\r", "\\r")
-      |> String.replace("\t", "\\t")
-      |> String.replace("\v", "\\v")
-
+    string = to_string(data)
+    escaped_data = escape_dot_string(string)
     [?", escaped_data, ?"]
   end
 
+  # Escape a string for DOT format according to GraphViz specification https://graphviz.org/doc/info/lang.html
+  # - Only quotes need escaping
+  # - String must not end with an odd number of backslashes (would escape the closing quote)
+  defp escape_dot_string(string) do
+    escape_dot_string(string, [], 0)
+  end
+
+  defp escape_dot_string(<<>>, acc, backslash_count) do
+    if rem(backslash_count, 2) == 1 do
+      # Odd number of trailing backslashes - add one more to make it even
+      [acc, "\\"]
+    else
+      acc
+    end
+  end
+
+  defp escape_dot_string(<<?"::utf8, rest::binary>>, acc, _backslash_count) do
+    escape_dot_string(rest, [acc, "\\\""], 0)
+  end
+
+  defp escape_dot_string(<<"\\"::utf8, rest::binary>>, acc, backslash_count) do
+    escape_dot_string(rest, [acc, "\\"], backslash_count + 1)
+  end
+
+  defp escape_dot_string(<<char::utf8, rest::binary>>, acc, _backslash_count) do
+    escape_dot_string(rest, [acc, char], 0)
+  end
+
   @doc false
   @deprecated "Use Macro.underscore/1 instead"
   def underscore(value) do

lib/mix/test/mix/utils_test.exs

@@ -243,43 +243,105 @@ defmodule Mix.UtilsTest do
       end)
     end
 
-    test "escapes newlines" do
-      in_tmp("dot_quotes", fn ->
+    test "preserves newlines and other control characters" do
+      in_tmp("dot_newlines", fn ->
         callback = fn node -> {{node, nil}, []} end
 
         Mix.Utils.write_dot_graph!("graph.dot", "graph", ["foo \nbar\r\nbaz"], callback, [])
 
         assert File.read!("graph.dot") == """
                digraph "graph" {
-                 "foo \\nbar\\r\\nbaz"
+                 "foo 
+               bar\r
+               baz"
                }
                """
       end)
     end
 
-    test "escapes backslashes" do
+    test "handles backslashes correctly" do
       in_tmp("dot_backslashes", fn ->
         callback = fn node -> {{node, nil}, []} end
 
         Mix.Utils.write_dot_graph!("graph.dot", "graph", ["foo\\bar"], callback, [])
 
         assert File.read!("graph.dot") == """
                digraph "graph" {
-                 "foo\\\\bar"
+                 "foo\\bar"
                }
                """
       end)
     end
 
-    test "escapes control characters" do
-      in_tmp("dot_tabs", fn ->
+    test "quote and backslash combinations" do
+      in_tmp("dot_complex", fn ->
+        callback = fn node -> {{node, nil}, []} end
+
+        test_cases = [
+          # "fo"o" -> "fo\"o"
+          {"fo\"o", "fo\\\"o"},
+          # "fo\"o" -> "fo\\\"o"
+          {"fo\\\"o", "fo\\\\\"o"},
+          # "fo\o" -> "fo\o"
+          {"fo\\o", "fo\\o"},
+          # "fo\\o" -> "fo\\o"
+          {"fo\\\\o", "fo\\\\o"},
+          # "fo\\\o" -> "fo\\\o"
+          {"fo\\\\\\o", "fo\\\\\\o"}
+        ]
+
+        Enum.each(test_cases, fn {input, expected} ->
+          Mix.Utils.write_dot_graph!("graph.dot", "graph", [input], callback, [])
+          content = File.read!("graph.dot")
+          assert content == "digraph \"graph\" {\n  \"#{expected}\"\n}\n"
+        end)
+      end)
+    end
+
+    test "escapes backslash at end of string" do
+      in_tmp("dot_end_backslash", fn ->
+        callback = fn node -> {{node, nil}, []} end
+
+        test_cases = [
+          # "fo\" -> "fo\\" (add backslash)
+          {"fo\\", "fo\\\\"},
+          # "fo\\" -> "fo\\" (already valid)
+          {"fo\\\\", "fo\\\\"},
+          # "fo\\\" -> "fo\\\\" (add backslash)
+          {"fo\\\\\\", "fo\\\\\\\\"}
+        ]
+
+        Enum.each(test_cases, fn {input, expected} ->
+          Mix.Utils.write_dot_graph!("graph.dot", "graph", [input], callback, [])
+          content = File.read!("graph.dot")
+          assert content == "digraph \"graph\" {\n  \"#{expected}\"\n}\n"
+        end)
+      end)
+    end
+
+    test "handles empty strings" do
+      in_tmp("dot_empty", fn ->
         callback = fn node -> {{node, nil}, []} end
 
-        Mix.Utils.write_dot_graph!("graph.dot", "graph", ["foo\v\t\f\e\d\b\a\0bar"], callback, [])
+        Mix.Utils.write_dot_graph!("graph.dot", "graph", [""], callback, [])
+
+        assert File.read!("graph.dot") == """
+               digraph "graph" {
+                 ""
+               }
+               """
+      end)
+    end
+
+    test "handles edge labels with escaping" do
+      in_tmp("dot_edge_labels", fn ->
+        callback = fn node -> {{node, "edge \"label\""}, []} end
+
+        Mix.Utils.write_dot_graph!("graph.dot", "graph", ["node"], callback, [])
 
         assert File.read!("graph.dot") == """
                digraph "graph" {
-                 "foo\\v\\t\\f\\e\\d\\b\\a\\0bar"
+                 "node" [label="edge \\"label\\""]
                }
                """
       end)