From d27e2447aba193ae524698c2ba91a0c36914b806 Mon Sep 17 00:00:00 2001
From: Joel Drapper <joel@drapper.me>
Date: Fri, 6 Sep 2024 22:17:55 +0100
Subject: [PATCH] Safe values (#772)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Remove `Phlex::SGML#unsafe_raw`
- Add `Phlex::SGML#safe` which creates a `Phlex::SGML::SafeValue`
- Add `Phlex::SGML::SafeObject` a generic interface for safe objects,
which `Phlex::SGML::SafeValue` implements
- Add `Phlex::SGML#raw`, which takes a SafeObject
- Update `__attributes__` to allow the use of unsafe attribute names
when the values are SafeObjects.

Example
```ruby
def view_template
  raw 🦺 "<script>"
  a href: 🦺 "javascript:alert(1)"
  a onclick: 🦺 "alert(1)"
end
```

Thanks @bradgessler for writing the original implementation of this PR.
#719

Closes #718
---
 .ruby-version                             |  2 +-
 .tool-versions                            |  2 +-
 lib/phlex/sgml.rb                         | 51 ++++++++++++++---------
 lib/phlex/sgml/safe_object.rb             |  7 ++++
 lib/phlex/sgml/safe_value.rb              | 11 +++++
 quickdraw/sgml/safe_value.test.rb         | 14 +++++++
 test/phlex/view/{unsafe_raw.rb => raw.rb} | 18 ++++++--
 7 files changed, 80 insertions(+), 25 deletions(-)
 create mode 100644 lib/phlex/sgml/safe_object.rb
 create mode 100644 lib/phlex/sgml/safe_value.rb
 create mode 100644 quickdraw/sgml/safe_value.test.rb
 rename test/phlex/view/{unsafe_raw.rb => raw.rb} (55%)

diff --git a/.ruby-version b/.ruby-version
index 619b5376..fa7adc7a 100644
--- a/.ruby-version
+++ b/.ruby-version
@@ -1 +1 @@
-3.3.3
+3.3.5
diff --git a/.tool-versions b/.tool-versions
index d554c9c4..1dd19980 100644
--- a/.tool-versions
+++ b/.tool-versions
@@ -1 +1 @@
-ruby 3.3.3
+ruby 3.3.5
diff --git a/lib/phlex/sgml.rb b/lib/phlex/sgml.rb
index 1fa92b9f..dce7c162 100644
--- a/lib/phlex/sgml.rb
+++ b/lib/phlex/sgml.rb
@@ -2,6 +2,9 @@
 
 # **Standard Generalized Markup Language** for behaviour common to {HTML} and {SVG}.
 class Phlex::SGML
+	autoload :SafeObject, "phlex/sgml/safe_object"
+	autoload :SafeValue, "phlex/sgml/safe_value"
+
 	include Phlex::Helpers
 
 	class << self
@@ -188,13 +191,18 @@ def comment(&)
 	# This method is very dangerous and should usually be avoided. It will output the given String without any HTML safety. You should never use this method to output unsafe user input.
 	# @param content [String|nil]
 	# @return [nil]
-	def unsafe_raw(content = nil)
-		return nil unless content
+	def raw(content)
+		case content
+		when Phlex::SGML::SafeObject
+			context = @_context
+			return if context.fragments && !context.in_target_fragment
 
-		context = @_context
-		return if context.fragments && !context.in_target_fragment
+			context.buffer << content.to_s
+		when nil # do nothing
+		else
+			raise Phlex::ArgumentError.new("You passed an unsafe object to `raw`.")
+		end
 
-		context.buffer << content
 		nil
 	end
 
@@ -229,20 +237,12 @@ def tag(name, ...)
 		end
 	end
 
-	def unsafe_tag(name, ...)
-		normalized_name = case name
-			when Symbol then name.name.downcase
-			when String then name.downcase
-			else raise Phlex::ArgumentError.new("Expected the tag name as a Symbol or String.")
-		end
-
-		if registered_elements[normalized_name]
-			public_send(normalized_name, ...)
-		else
-			raise Phlex::ArgumentError.new("Unknown tag: #{normalized_name}")
-		end
+	def safe(value)
+		Phlex::SGML::SafeValue.new(value)
 	end
 
+	alias_method :🦺, :safe
+
 	private
 
 	# @api private
@@ -425,10 +425,19 @@ def __attributes__(attributes, buffer = +"")
 			end
 
 			lower_name = name.downcase
-			next if lower_name == "href" && v.to_s.downcase.delete("^a-z:").start_with?("javascript:")
 
-			# Detect unsafe attribute names. Attribute names are considered unsafe if they match an event attribute or include unsafe characters.
-			if Phlex::HTML::EVENT_ATTRIBUTES.include?(lower_name.delete("^a-z-")) || name.match?(/[<>&"']/)
+			unless Phlex::SGML::SafeObject === v
+				if lower_name == "href" && v.to_s.downcase.delete("^a-z:").start_with?("javascript:")
+					next
+				end
+
+				# Detect unsafe attribute names. Attribute names are considered unsafe if they match an event attribute or include unsafe characters.
+				if Phlex::HTML::EVENT_ATTRIBUTES.include?(lower_name.delete("^a-z-"))
+					raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.")
+				end
+			end
+
+			if name.match?(/[<>&"']/)
 				raise Phlex::ArgumentError.new("Unsafe attribute name detected: #{k}.")
 			end
 
@@ -467,6 +476,8 @@ def __attributes__(attributes, buffer = +"")
 				buffer << " " << name << '="' << value.gsub('"', "&quot;") << '"'
 			when Set
 				buffer << " " << name << '="' << __nested_tokens__(v.to_a) << '"'
+			when Phlex::SGML::SafeObject
+				buffer << " " << name << '="' << v.to_s.gsub('"', "&quot;") << '"'
 			else
 				value = if v.respond_to?(:to_phlex_attribute_value)
 					v.to_phlex_attribute_value
diff --git a/lib/phlex/sgml/safe_object.rb b/lib/phlex/sgml/safe_object.rb
new file mode 100644
index 00000000..c2af909c
--- /dev/null
+++ b/lib/phlex/sgml/safe_object.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+# @api private
+module Phlex::SGML::SafeObject
+	# This is included in objects that are safe to render in an SGML context.
+	# They must implement a `to_s` method that returns a string.
+end
diff --git a/lib/phlex/sgml/safe_value.rb b/lib/phlex/sgml/safe_value.rb
new file mode 100644
index 00000000..7fd3b3ef
--- /dev/null
+++ b/lib/phlex/sgml/safe_value.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class Phlex::SGML::SafeValue
+	include Phlex::SGML::SafeObject
+
+	def initialize(to_s)
+		@to_s = to_s
+	end
+
+	attr_reader :to_s
+end
diff --git a/quickdraw/sgml/safe_value.test.rb b/quickdraw/sgml/safe_value.test.rb
new file mode 100644
index 00000000..e973a92d
--- /dev/null
+++ b/quickdraw/sgml/safe_value.test.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class Example < Phlex::HTML
+	def view_template
+		a(
+			onclick: safe("window.history.back()"),
+			href: safe("javascript:window.history.back()"),
+		)
+	end
+end
+
+test "safe value" do
+	expect(Example.call) == %(<a onclick="window.history.back()" href="javascript:window.history.back()"></a>)
+end
diff --git a/test/phlex/view/unsafe_raw.rb b/test/phlex/view/raw.rb
similarity index 55%
rename from test/phlex/view/unsafe_raw.rb
rename to test/phlex/view/raw.rb
index c71d17e2..c455bc6e 100644
--- a/test/phlex/view/unsafe_raw.rb
+++ b/test/phlex/view/raw.rb
@@ -3,10 +3,22 @@
 describe Phlex::HTML do
 	extend ViewHelper
 
-	with "raw content" do
+	with "raw unsafe content" do
 		view do
 			def view_template
-				unsafe_raw %(<h1 class="test">Hello</h1>)
+				raw %(<h1 class="test">Hello</h1>)
+			end
+		end
+
+		it "renders the correct output" do
+			expect { output }.to raise_exception ArgumentError
+		end
+	end
+
+	with "raw safe content" do
+		view do
+			def view_template
+				raw safe %(<h1 class="test">Hello</h1>)
 			end
 		end
 
@@ -18,7 +30,7 @@ def view_template
 	with "nil content" do
 		view do
 			def view_template
-				unsafe_raw nil
+				raw nil
 			end
 		end