Permalink
Browse files

Merge pull request #2248 from dmathieu/safe_gsub

make gsub and sub unavailable in SafeBuffers - Closes #1555
  • Loading branch information...
josevalim committed Sep 8, 2011
2 parents e1b500e + 3718ccd commit b4a6e2f8442ceda118367f9a61c38af754be1cbf
@@ -75,7 +75,8 @@ def html_safe?
module ActiveSupport #:nodoc:
class SafeBuffer < String
- UNSAFE_STRING_METHODS = ["capitalize", "chomp", "chop", "delete", "downcase", "gsub", "lstrip", "next", "reverse", "rstrip", "slice", "squeeze", "strip", "sub", "succ", "swapcase", "tr", "tr_s", "upcase"].freeze
+ UNSAFE_STRING_METHODS = ["capitalize", "chomp", "chop", "delete", "downcase", "lstrip", "next", "reverse", "rstrip", "slice", "squeeze", "strip", "succ", "swapcase", "tr", "tr_s", "upcase"].freeze
+ UNAVAILABLE_STRING_METHODS = ["gsub", "sub"]
alias_method :original_concat, :concat
private :original_concat
@@ -143,17 +144,29 @@ def to_yaml(*args)
UNSAFE_STRING_METHODS.each do |unsafe_method|
class_eval <<-EOT, __FILE__, __LINE__
- def #{unsafe_method}(*args, &block) # def gsub(*args, &block)
+ def #{unsafe_method}(*args, &block) # def capitalize(*args, &block)
to_str.#{unsafe_method}(*args, &block) # to_str.gsub(*args, &block)
end # end
- def #{unsafe_method}!(*args) # def gsub!(*args)
+ def #{unsafe_method}!(*args) # def capitalize!(*args)
@dirty = true # @dirty = true
super # super
end # end
EOT
end
+ UNAVAILABLE_STRING_METHODS.each do |unavailable_method|
+ class_eval <<-EOT, __FILE__, __LINE__
+ def #{unavailable_method}(*args) # def gsub(*args)
+ raise NoMethodError, "#{unavailable_method} cannot be used with a Safe Buffer object. You should use object.to_str.#{unavailable_method}"
+ end # end
+
+ def #{unavailable_method}!(*args) # def gsub!(*args)
+ raise NoMethodError, "#{unavailable_method} cannot be used with a Safe Buffer object. You should use object.to_str.#{unavailable_method}"
+ end # end
+ EOT
+ end
+
protected
def dirty?
@@ -21,7 +21,8 @@ module Inflector
# "words".pluralize # => "words"
# "CamelOctopus".pluralize # => "CamelOctopi"
def pluralize(word)
- result = word.to_s.dup
+ word = deprecate_symbol(word)
+ result = word.to_str.dup
if word.empty? || inflections.uncountables.include?(result.downcase)
result
@@ -40,7 +41,8 @@ def pluralize(word)
# "word".singularize # => "word"
# "CamelOctopi".singularize # => "CamelOctopus"
def singularize(word)
- result = word.to_s.dup
+ word = deprecate_symbol(word)
+ result = word.to_str.dup
if inflections.uncountables.any? { |inflection| result =~ /\b(#{inflection})\Z/i }
result
@@ -66,7 +68,8 @@ def singularize(word)
#
# "SSLError".underscore.camelize # => "SslError"
def camelize(term, uppercase_first_letter = true)
- string = term.to_s
+ term = deprecate_symbol(term)
+ string = term.to_str
if uppercase_first_letter
string = string.sub(/^[a-z\d]*/) { inflections.acronyms[$&] || $&.capitalize }
else
@@ -88,7 +91,8 @@ def camelize(term, uppercase_first_letter = true)
#
# "SSLError".underscore.camelize # => "SslError"
def underscore(camel_cased_word)
- word = camel_cased_word.to_s.dup
+ camel_cased_word = deprecate_symbol(camel_cased_word)
+ word = camel_cased_word.to_str.dup
word.gsub!(/::/, '/')
word.gsub!(/(?:([A-Za-z\d])|^)(#{inflections.acronym_regex})(?=\b|[^a-z])/) { "#{$1}#{$1 && '_'}#{$2.downcase}" }
word.gsub!(/([A-Z\d]+)([A-Z][a-z])/,'\1_\2')
@@ -105,7 +109,8 @@ def underscore(camel_cased_word)
# "employee_salary" # => "Employee salary"
# "author_id" # => "Author"
def humanize(lower_case_and_underscored_word)
- result = lower_case_and_underscored_word.to_s.dup
+ lower_case_and_underscored_word = deprecate_symbol(lower_case_and_underscored_word)
+ result = lower_case_and_underscored_word.to_str.dup
inflections.humans.each { |(rule, replacement)| break if result.gsub!(rule, replacement) }
result.gsub!(/_id$/, "")
result.gsub(/(_)?([a-z\d]*)/i) { "#{$1 && ' '}#{inflections.acronyms[$2] || $2.downcase}" }.gsub(/^\w/) { $&.upcase }
@@ -148,16 +153,18 @@ def tableize(class_name)
# Singular names are not handled correctly:
# "business".classify # => "Busines"
def classify(table_name)
+ table_name = deprecate_symbol(table_name)
# strip out any leading schema name
- camelize(singularize(table_name.to_s.sub(/.*\./, '')))
+ camelize(singularize(table_name.to_str.sub(/.*\./, '')))
end
# Replaces underscores with dashes in the string.
#
# Example:
# "puni_puni" # => "puni-puni"
def dasherize(underscored_word)
- underscored_word.gsub(/_/, '-')
+ underscored_word = deprecate_symbol(underscored_word)
+ underscored_word.to_str.gsub(/_/, '-')
end
# Removes the module part from the expression in the string.
@@ -166,7 +173,8 @@ def dasherize(underscored_word)
# "ActiveRecord::CoreExtensions::String::Inflections".demodulize # => "Inflections"
# "Inflections".demodulize # => "Inflections"
def demodulize(class_name_in_module)
- class_name_in_module.to_s.gsub(/^.*::/, '')
+ class_name_in_module = deprecate_symbol(class_name_in_module)
+ class_name_in_module.to_str.gsub(/^.*::/, '')
end
# Creates a foreign key name from a class name.
@@ -246,5 +254,13 @@ def ordinalize(number)
end
end
end
+
+ def deprecate_symbol(symbol)
+ if symbol.is_a?(Symbol)
+ symbol = symbol.to_s
+ ActiveSupport::Deprecation.warn("Using symbols in inflections is deprecated. Please use to_s to have a string.")
+ end
+ symbol
+ end
end
end
@@ -10,7 +10,7 @@ class Case
end
end
-class InflectorTest < Test::Unit::TestCase
+class InflectorTest < ActiveSupport::TestCase
include InflectorTestCases
def test_pluralize_plurals
@@ -248,12 +248,6 @@ def test_classify
end
end
- def test_classify_with_symbol
- assert_nothing_raised do
- assert_equal 'FooBar', ActiveSupport::Inflector.classify(:foo_bars)
- end
- end
-
def test_classify_with_leading_schema_name
assert_equal 'FooBar', ActiveSupport::Inflector.classify('schema.foo_bar')
end
@@ -319,12 +313,6 @@ def test_underscore_to_lower_camel
end
end
- def test_symbol_to_lower_camel
- SymbolToLowerCamel.each do |symbol, lower_camel|
- assert_equal(lower_camel, ActiveSupport::Inflector.camelize(symbol, false))
- end
- end
-
%w{plurals singulars uncountables humans}.each do |inflection_type|
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def test_clear_#{inflection_type}
@@ -380,6 +368,14 @@ def test_clear_with_default
ActiveSupport::Inflector.inflections.instance_variable_set :@humans, cached_values[3]
end
+ [:pluralize, :singularize, :camelize, :underscore, :humanize, :titleize, :tableize, :classify, :dasherize, :demodulize].each do |method|
+ test "should deprecate symbols on #{method}" do
+ assert_deprecated(/Using symbols in inflections is deprecated/) do
+ ActiveSupport::Inflector.send(method, :foo)
+ end
+ end
+ end
+
Irregularities.each do |irregularity|
singular, plural = *irregularity
ActiveSupport::Inflector.inflections do |inflect|
@@ -120,13 +120,6 @@ module InflectorTestCases
"area51_controller" => "area51Controller"
}
- SymbolToLowerCamel = {
- :product => 'product',
- :special_guest => 'specialGuest',
- :application_controller => 'applicationController',
- :area51_controller => 'area51Controller'
- }
-
CamelToUnderscoreWithoutReverse = {
"HTMLTidy" => "html_tidy",
"HTMLTidyGenerator" => "html_tidy_generator",
@@ -67,42 +67,41 @@ def setup
assert_equal "my_test", str
end
- test "Should not return safe buffer from gsub" do
- altered_buffer = @buffer.gsub('', 'asdf')
- assert_equal 'asdf', altered_buffer
+ test "Should not return safe buffer from capitalize" do
+ altered_buffer = "asdf".html_safe.capitalize
+ assert_equal 'Asdf', altered_buffer
assert !altered_buffer.html_safe?
end
test "Should not return safe buffer from gsub!" do
- @buffer.gsub!('', 'asdf')
- assert_equal 'asdf', @buffer
- assert !@buffer.html_safe?
+ string = "asdf"
+ string.capitalize!
+ assert_equal 'Asdf', string
+ assert !string.html_safe?
end
test "Should escape dirty buffers on add" do
clean = "hello".html_safe
- @buffer.gsub!('', '<>')
- assert_equal "hello&lt;&gt;", clean + @buffer
+ assert_equal "hello&lt;&gt;", clean + '<>'
end
test "Should concat as a normal string when dirty" do
clean = "hello".html_safe
- @buffer.gsub!('', '<>')
- assert_equal "<>hello", @buffer + clean
+ assert_equal "<>hello", '<>' + clean
end
test "Should preserve dirty? status on copy" do
- @buffer.gsub!('', '<>')
- assert !@buffer.dup.html_safe?
+ dirty = "<>"
+ assert !dirty.dup.html_safe?
end
test "Should raise an error when safe_concat is called on dirty buffers" do
- @buffer.gsub!('', '<>')
+ @buffer.capitalize!
assert_raise ActiveSupport::SafeBuffer::SafeConcatError do
@buffer.safe_concat "BUSTED"
end
end
-
+
test "should not fail if the returned object is not a string" do
assert_kind_of NilClass, @buffer.slice("chipchop")
end
@@ -112,4 +111,18 @@ def setup
assert_not_nil dirty
assert !dirty
end
+
+ ["gsub", "sub"].each do |unavailable_method|
+ test "should raise on #{unavailable_method}" do
+ assert_raise NoMethodError, "#{unavailable_method} cannot be used with a Safe Buffer object. You should use object.to_str.#{unavailable_method}" do
+ @buffer.send(unavailable_method, '', '<>')
+ end
+ end
+
+ test "should raise on #{unavailable_method}!" do
+ assert_raise NoMethodError, "#{unavailable_method}! cannot be used with a Safe Buffer object. You should use object.to_str.#{unavailable_method}!" do
+ @buffer.send("#{unavailable_method}!", '', '<>')
+ end
+ end
+ end
end

3 comments on commit b4a6e2f

@tenderlove

This comment has been minimized.

Show comment Hide comment
@tenderlove

tenderlove Sep 8, 2011

Owner

Bro, why did you merge this? It totally breaks AP tests.

Owner

tenderlove replied Sep 8, 2011

Bro, why did you merge this? It totally breaks AP tests.

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Sep 8, 2011

Contributor

Ok. I guess I will revert.

Contributor

josevalim replied Sep 8, 2011

Ok. I guess I will revert.

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn Sep 8, 2011

Owner

Also note that AS core extensions add methods to String some of which use gsub internally.

Owner

fxn replied Sep 8, 2011

Also note that AS core extensions add methods to String some of which use gsub internally.

Please sign in to comment.