Escape $ in javascript_escape but reinforce template literals are NOT supported

Author: josevalim

CHANGELOG.md

@@ -190,7 +190,7 @@ use PhoenixHTMLHelpers
 
 * Enhancements
   * Allow safe content to be given to label
-  * Also escale template literals in `javascript_escape/1`
+  * Also escape template literals in `javascript_escape/1`
 
 * Bug fixes
   * Fix deprecation warnings to point to the correct alternative

lib/phoenix_html.ex

@@ -329,6 +329,12 @@ defmodule Phoenix.HTML do
       $("#container").append("<%= javascript_escape(render("post.html", post: @post)) %>");
 
   It escapes quotes (double and single), double backslashes and others.
+
+  > #### Do not inject into template literals {: .error}
+  >
+  > Do not inject the result of `javascript_escape` into JavaScript
+  > template literals (defined with backticks), given template functions
+  > can change the meaning of the string in unsafe ways.
   """
   @spec javascript_escape(binary) :: binary
   @spec javascript_escape(safe) :: safe
@@ -353,7 +359,7 @@ defmodule Phoenix.HTML do
   defp javascript_escape(<<"\r\n", t::binary>>, acc),
     do: javascript_escape(t, <<acc::binary, ?\\, ?n>>)
 
-  defp javascript_escape(<<h, t::binary>>, acc) when h in [?", ?', ?\\, ?`],
+  defp javascript_escape(<<h, t::binary>>, acc) when h in [?", ?', ?\\, ?`, ?$],
     do: javascript_escape(t, <<acc::binary, ?\\, h>>)
 
   defp javascript_escape(<<h, t::binary>>, acc) when h in [?\r, ?\n],

test/phoenix_html_test.exs

@@ -9,7 +9,7 @@ defmodule Phoenix.HTMLTest do
     assert javascript_escape("\\Double backslash") == "\\\\Double backslash"
     assert javascript_escape("\"Double quote\"") == "\\\"Double quote\\\""
     assert javascript_escape("'Single quote'") == "\\'Single quote\\'"
-    assert javascript_escape("`Backtick`") == "\\`Backtick\\`"
+    assert javascript_escape("`Bac${kt}ick`") == "\\`Bac\\${kt}ick\\`"
     assert javascript_escape("New line\r") == "New line\\n"
     assert javascript_escape("New line\n") == "New line\\n"
     assert javascript_escape("New line\r\n") == "New line\\n"
@@ -134,8 +134,18 @@ defmodule Phoenix.HTMLTest do
     end
 
     test "raises on invalid binary attribute name" do
-      for invalid <- ["", "foo\"bar", "foo'bar", "foo>bar", "foo/bar", "foo=bar",
-                      "foo\tbar", "foo\nbar", "foo\0bar", "foo\x7Fbar"] do
+      for invalid <- [
+            "",
+            "foo\"bar",
+            "foo'bar",
+            "foo>bar",
+            "foo/bar",
+            "foo=bar",
+            "foo\tbar",
+            "foo\nbar",
+            "foo\0bar",
+            "foo\x7Fbar"
+          ] do
         assert_raise ArgumentError, ~r/expected attribute name/, fn ->
           attributes_escape([{invalid, "value"}])
         end