Skip to content

Commit

Permalink
Copy CGI.escapeHTML to ERB::Util.html_escape
Browse files Browse the repository at this point in the history
  • Loading branch information
k0kubun committed Nov 4, 2022
1 parent 48a7566 commit ac9b219
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 4 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@
/spec/reports/
/tmp/
Gemfile.lock
*.so
*.gem
7 changes: 5 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@ source 'https://rubygems.org'

gemspec

gem 'rake'
gem 'test-unit'
group :development do
gem 'rake'
gem 'rake-compiler'
gem 'test-unit'
end
6 changes: 6 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ Rake::TestTask.new(:test) do |t|
t.test_files = FileList['test/**/test_*.rb']
end

if RUBY_ENGINE != 'jruby'
require 'rake/extensiontask'
Rake::ExtensionTask.new('erb')
task test: :compile
end

task :sync_tool do
require 'fileutils'
FileUtils.cp '../ruby/tool/lib/core_assertions.rb', './test/lib'
Expand Down
5 changes: 4 additions & 1 deletion erb.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ Gem::Specification.new do |spec|
spec.executables = ['erb']
spec.require_paths = ['lib']

if RUBY_ENGINE != 'jruby'
if RUBY_ENGINE == 'jruby'
spec.platform = 'java'
else
spec.required_ruby_version = '>= 2.7.0'
spec.extensions = ['ext/erb/extconf.rb']
end

spec.add_dependency 'cgi', '>= 0.3.3'
Expand Down
96 changes: 96 additions & 0 deletions ext/erb/erb.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
#include "ruby.h"
#include "ruby/encoding.h"

static VALUE rb_cERB, rb_mEscape;

#define HTML_ESCAPE_MAX_LEN 6

static const struct {
uint8_t len;
char str[HTML_ESCAPE_MAX_LEN+1];
} html_escape_table[UCHAR_MAX+1] = {
#define HTML_ESCAPE(c, str) [c] = {rb_strlen_lit(str), str}
HTML_ESCAPE('\'', "'"),
HTML_ESCAPE('&', "&"),
HTML_ESCAPE('"', """),
HTML_ESCAPE('<', "&lt;"),
HTML_ESCAPE('>', "&gt;"),
#undef HTML_ESCAPE
};

static inline void
preserve_original_state(VALUE orig, VALUE dest)
{
rb_enc_associate(dest, rb_enc_get(orig));
}

static inline long
escaped_length(VALUE str)
{
const long len = RSTRING_LEN(str);
if (len >= LONG_MAX / HTML_ESCAPE_MAX_LEN) {
ruby_malloc_size_overflow(len, HTML_ESCAPE_MAX_LEN);
}
return len * HTML_ESCAPE_MAX_LEN;
}

static VALUE
optimized_escape_html(VALUE str)
{
VALUE vbuf;
char *buf = ALLOCV_N(char, vbuf, escaped_length(str));
const char *cstr = RSTRING_PTR(str);
const char *end = cstr + RSTRING_LEN(str);

char *dest = buf;
while (cstr < end) {
const unsigned char c = *cstr++;
uint8_t len = html_escape_table[c].len;
if (len) {
memcpy(dest, html_escape_table[c].str, len);
dest += len;
}
else {
*dest++ = c;
}
}

VALUE escaped;
if (RSTRING_LEN(str) < (dest - buf)) {
escaped = rb_str_new(buf, dest - buf);
preserve_original_state(str, escaped);
}
else {
escaped = rb_str_dup(str);
}
ALLOCV_END(vbuf);
return escaped;
}

static VALUE
cgiesc_escape_html(VALUE self, VALUE str)
{
StringValue(str);

if (rb_enc_str_asciicompat_p(str)) {
return optimized_escape_html(str);
}
else {
return rb_call_super(1, &str);
}
}

static VALUE
erb_escape_html(VALUE self, VALUE str)
{
str = rb_funcall(str, rb_intern("to_s"), 0);
return cgiesc_escape_html(self, str);
}

void
Init_erb(void)
{
rb_cERB = rb_define_class("ERB", rb_cObject);
rb_mEscape = rb_define_module_under(rb_cERB, "Escape");
rb_define_method(rb_mEscape, "html_escape", erb_escape_html, 1);
}
2 changes: 2 additions & 0 deletions ext/erb/extconf.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
require 'mkmf'
create_makefile 'erb'
12 changes: 11 additions & 1 deletion lib/erb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,6 @@ def def_class(superklass=Object, methodname='result')
class ERB
# A utility module for conversion routines, often handy in HTML generation.
module Util
public
#
# A utility method for escaping HTML tag characters in _s_.
#
Expand All @@ -1002,6 +1001,17 @@ module Util
def html_escape(s)
CGI.escapeHTML(s.to_s)
end
end

begin
require 'erb.so'
rescue LoadError
else
private_constant :Escape

This comment has been minimized.

Copy link
@jeremyevans

jeremyevans Nov 4, 2022

Contributor

Could you consider not making this a private constant? I would like to use it if available in Erubi and maybe Roda?

This comment has been minimized.

Copy link
@k0kubun

k0kubun Nov 4, 2022

Author Member

I'd be happy to make it usable from Erubi and Roda. Just to understand the context more, can you describe the blocker to just use ERB::Util.html_escape from there?

This comment has been minimized.

Copy link
@jeremyevans

jeremyevans Nov 4, 2022

Contributor

I'd like to only use it if I know the ERB version is faster. With ERB::Util.html_escape, I'm not sure whether I'm getting the faster version, correct? I'm guessing the reason prepend is used here is that there is a slower version of ERB::Util#html_escape already, which ERB::Escape#html_escape overrides? Otherwise, you would probably use include and not prepend.

This comment has been minimized.

Copy link
@k0kubun

k0kubun Nov 4, 2022

Author Member

I'm guessing the reason prepend is used here is that there is a slower version of ERB::Util#html_escape already, which ERB::Escape#html_escape overrides?

Kind of. It's to support non-optimizable encodings. It ultimately fallbacks to gsub. FYI I needed to avoid using prepend to make ActiveSupport's monkey patch (😞) happy, so I removed it anyway #28.

With ERB::Util.html_escape, I'm not sure whether I'm getting the faster version, correct?

Yeah, right. One option could be to require erb >= next_version in your dependency, but unfortunately it doesn't work for Ruby 2.7 (where ERB was still not a default gem). If you want to use defined?(ERB::Escape), I support your use case.

Plus, I also want a version that's never monkey-patched by Rails, so I might just add it for that reason too.

Util.prepend(Escape)
end

module Util
alias h html_escape
module_function :h
module_function :html_escape
Expand Down
13 changes: 13 additions & 0 deletions test/erb/test_erb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,24 @@ def test_html_escape
assert_equal("", ERB::Util.html_escape(""))
assert_equal("abc", ERB::Util.html_escape("abc"))
assert_equal("&lt;&lt;", ERB::Util.html_escape("<\<"))
assert_equal("&#39;&amp;&quot;&gt;&lt;", ERB::Util.html_escape("'&\"><"))

assert_equal("", ERB::Util.html_escape(nil))
assert_equal("123", ERB::Util.html_escape(123))
end

def test_html_escape_to_s
object = Object.new
def object.to_s
"object"
end
assert_equal("object", ERB::Util.html_escape(object))
end

def test_html_escape_extension
assert_nil(ERB::Util.method(:html_escape).source_location)
end if RUBY_ENGINE == 'ruby'

def test_concurrent_default_binding
# This test randomly fails with JRuby -- NameError: undefined local variable or method `template2'
pend if RUBY_ENGINE == 'jruby'
Expand Down

0 comments on commit ac9b219

Please sign in to comment.