Skip to content

Commit

Permalink
Safe values (#772)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
joeldrapper authored Sep 6, 2024
1 parent 1f8ec39 commit d27e244
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.3.3
3.3.5
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ruby 3.3.3
ruby 3.3.5
51 changes: 31 additions & 20 deletions lib/phlex/sgml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions lib/phlex/sgml/safe_object.rb
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions lib/phlex/sgml/safe_value.rb
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions quickdraw/sgml/safe_value.test.rb
Original file line number Diff line number Diff line change
@@ -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
18 changes: 15 additions & 3 deletions test/phlex/view/unsafe_raw.rb → test/phlex/view/raw.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -18,7 +30,7 @@ def view_template
with "nil content" do
view do
def view_template
unsafe_raw nil
raw nil
end
end

Expand Down

0 comments on commit d27e244

Please sign in to comment.