aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_dispatch/http/request.rb7
-rw-r--r--actionpack/lib/action_dispatch/middleware/params_parser.rb4
-rw-r--r--actionpack/test/controller/webservice_test.rb13
-rw-r--r--actionpack/test/dispatch/request/json_params_parsing_test.rb15
-rw-r--r--actionpack/test/dispatch/request/xml_params_parsing_test.rb17
-rw-r--r--activerecord/test/cases/relation/where_test.rb6
-rw-r--r--activesupport/CHANGELOG.md7
-rw-r--r--activesupport/lib/active_support/core_ext/hash/conversions.rb27
-rw-r--r--activesupport/test/core_ext/hash_ext_test.rb28
9 files changed, 15 insertions, 109 deletions
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 31b36eedb0..452809a689 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -276,14 +276,15 @@ module ActionDispatch
LOCALHOST =~ remote_addr && LOCALHOST =~ remote_ip
end
+ protected
+
# Remove nils from the params hash
def deep_munge(hash)
- hash.each do |k, v|
+ hash.each_value do |v|
case v
when Array
v.grep(Hash) { |x| deep_munge(x) }
v.compact!
- hash[k] = nil if v.empty?
when Hash
deep_munge(v)
end
@@ -292,8 +293,6 @@ module ActionDispatch
hash
end
- protected
-
def parse_query(qs)
deep_munge(super)
end
diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb
index 951f28f535..2c98ca03a8 100644
--- a/actionpack/lib/action_dispatch/middleware/params_parser.rb
+++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb
@@ -47,12 +47,12 @@ module ActionDispatch
when Proc
strategy.call(request.raw_post)
when :xml_simple, :xml_node
- data = request.deep_munge(Hash.from_xml(request.body.read) || {})
+ data = Hash.from_xml(request.raw_post) || {}
data.with_indifferent_access
when :yaml
YAML.load(request.raw_post)
when :json
- data = request.deep_munge ActiveSupport::JSON.decode(request.body)
+ data = ActiveSupport::JSON.decode(request.raw_post)
data = {:_json => data} unless data.is_a?(Hash)
data.with_indifferent_access
else
diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb
index 2602540fbe..c0b9833603 100644
--- a/actionpack/test/controller/webservice_test.rb
+++ b/actionpack/test/controller/webservice_test.rb
@@ -116,19 +116,6 @@ class WebServiceTest < ActionDispatch::IntegrationTest
end
end
- def test_post_xml_using_a_disallowed_type_attribute
- $stderr = StringIO.new
- with_test_route_set do
- post '/', '<foo type="symbol">value</foo>', 'CONTENT_TYPE' => 'application/xml'
- assert_response 500
-
- post '/', '<foo type="yaml">value</foo>', 'CONTENT_TYPE' => 'application/xml'
- assert_response 500
- end
- ensure
- $stderr = STDERR
- end
-
def test_register_and_use_yaml
with_test_route_set do
with_params_parsers Mime::YAML => Proc.new { |d| YAML.load(d) } do
diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb
index 2c4a6c2147..c0c3147e37 100644
--- a/actionpack/test/dispatch/request/json_params_parsing_test.rb
+++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb
@@ -30,21 +30,6 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest
)
end
- test "nils are stripped from collections" do
- assert_parses(
- {"person" => nil},
- "{\"person\":[null]}", { 'CONTENT_TYPE' => 'application/json' }
- )
- assert_parses(
- {"person" => ['foo']},
- "{\"person\":[\"foo\",null]}", { 'CONTENT_TYPE' => 'application/json' }
- )
- assert_parses(
- {"person" => nil},
- "{\"person\":[null, null]}", { 'CONTENT_TYPE' => 'application/json' }
- )
- end
-
test "logs error if parsing unsuccessful" do
with_test_routing do
output = StringIO.new
diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb
index f13b64a3c7..cb68667002 100644
--- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb
+++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb
@@ -30,23 +30,6 @@ class XmlParamsParsingTest < ActionDispatch::IntegrationTest
assert_equal "<ok>bar</ok>", resp.body
end
- def assert_parses(expected, xml)
- with_test_routing do
- post "/parse", xml, default_headers
- assert_response :ok
- assert_equal(expected, TestController.last_request_parameters)
- end
- end
-
- test "nils are stripped from collections" do
- assert_parses(
- {"hash" => { "person" => nil} },
- "<hash><person type=\"array\"><person nil=\"true\"/></person></hash>")
- assert_parses(
- {"hash" => { "person" => ['foo']} },
- "<hash><person type=\"array\"><person>foo</person><person nil=\"true\"/></person>\n</hash>")
- end
-
test "parses hash params" do
with_test_routing do
xml = "<person><name>David</name></person>"
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index d1c3690478..297e865308 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -90,12 +90,6 @@ module ActiveRecord
[[], {}, nil, ""].each do |blank|
assert_equal 4, Edge.where(blank).order("sink_id").to_a.size
end
- def test_where_with_table_name_and_empty_array
- assert_equal 0, Post.where(:id => []).count
- end
-
- def test_where_with_empty_hash_and_no_foreign_key
- assert_equal 0, Edge.where(:sink => {}).count
end
end
end
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 5848f9712f..08bec2f4ae 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,12 +1,5 @@
## Rails 4.0.0 (unreleased) ##
-* Hash.from_xml raises when it encounters type="symbol" or type="yaml".
- Use Hash.from_trusted_xml to parse this XML.
-
- CVE-2013-0156
-
- *Jeremy Kemper*
-
* Deprecate `assert_present` and `assert_blank` in favor of
`assert object.blank?` and `assert object.present?`
diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb
index 8930376ac8..6cb7434e5f 100644
--- a/activesupport/lib/active_support/core_ext/hash/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb
@@ -101,33 +101,17 @@ class Hash
#
# hash = Hash.from_xml(xml)
# # => {"hash"=>{"foo"=>1, "bar"=>2}}
- #
- # DisallowedType is raise if the XML contains attributes with <tt>type="yaml"</tt> or
- # <tt>type="symbol"</tt>. Use <tt>Hash.from_trusted_xml</tt> to parse this XML.
- def from_xml(xml, disallowed_types = nil)
- ActiveSupport::XMLConverter.new(xml, disallowed_types).to_h
+ def from_xml(xml)
+ ActiveSupport::XMLConverter.new(xml).to_h
end
- # Builds a Hash from XML just like <tt>Hash.from_xml</tt>, but also allows Symbol and YAML.
- def from_trusted_xml(xml)
- from_xml xml, []
- end
end
end
module ActiveSupport
class XMLConverter # :nodoc:
- class DisallowedType < StandardError
- def initialize(type)
- super "Disallowed type attribute: #{type.inspect}"
- end
- end
-
- DISALLOWED_TYPES = %w(symbol yaml)
-
- def initialize(xml, disallowed_types = nil)
+ def initialize(xml)
@xml = normalize_keys(XmlMini.parse(xml))
- @disallowed_types = disallowed_types || DISALLOWED_TYPES
end
def to_h
@@ -135,6 +119,7 @@ module ActiveSupport
end
private
+
def normalize_keys(params)
case params
when Hash
@@ -160,10 +145,6 @@ module ActiveSupport
end
def process_hash(value)
- if value.include?('type') && !value['type'].is_a?(Hash) && @disallowed_types.include?(value['type'])
- raise DisallowedType, value['type']
- end
-
if become_array?(value)
_, entries = Array.wrap(value.detect { |k,v| not v.is_a?(String) })
if entries.nil? || value['__content__'].try(:empty?)
diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb
index 30d95b75bc..84dd9bc983 100644
--- a/activesupport/test/core_ext/hash_ext_test.rb
+++ b/activesupport/test/core_ext/hash_ext_test.rb
@@ -1015,10 +1015,12 @@ class HashToXmlTest < ActiveSupport::TestCase
<replies-close-in type="integer">2592000000</replies-close-in>
<written-on type="date">2003-07-16</written-on>
<viewed-at type="datetime">2003-07-16T09:28:00+0000</viewed-at>
+ <content type="yaml">--- \n1: should be an integer\n:message: Have a nice day\narray: \n- should-have-dashes: true\n should_have_underscores: true\n</content>
<author-email-address>david@loudthinking.com</author-email-address>
<parent-id></parent-id>
<ad-revenue type="decimal">1.5</ad-revenue>
<optimum-viewing-angle type="float">135</optimum-viewing-angle>
+ <resident type="symbol">yes</resident>
</topic>
EOT
@@ -1031,10 +1033,12 @@ class HashToXmlTest < ActiveSupport::TestCase
:replies_close_in => 2592000000,
:written_on => Date.new(2003, 7, 16),
:viewed_at => Time.utc(2003, 7, 16, 9, 28),
+ :content => { :message => "Have a nice day", 1 => "should be an integer", "array" => [{ "should-have-dashes" => true, "should_have_underscores" => true }] },
:author_email_address => "david@loudthinking.com",
:parent_id => nil,
:ad_revenue => BigDecimal("1.50"),
:optimum_viewing_angle => 135.0,
+ :resident => :yes
}.stringify_keys
assert_equal expected_topic_hash, Hash.from_xml(topic_xml)["topic"]
@@ -1048,6 +1052,7 @@ class HashToXmlTest < ActiveSupport::TestCase
<approved type="boolean"></approved>
<written-on type="date"></written-on>
<viewed-at type="datetime"></viewed-at>
+ <content type="yaml"></content>
<parent-id></parent-id>
</topic>
EOT
@@ -1058,6 +1063,7 @@ class HashToXmlTest < ActiveSupport::TestCase
:approved => nil,
:written_on => nil,
:viewed_at => nil,
+ :content => nil,
:parent_id => nil
}.stringify_keys
@@ -1284,28 +1290,6 @@ class HashToXmlTest < ActiveSupport::TestCase
assert_equal expected_product_hash, Hash.from_xml(product_xml)["product"]
end
- def test_from_xml_raises_on_disallowed_type_attributes
- assert_raise ActiveSupport::XMLConverter::DisallowedType do
- Hash.from_xml '<product><name type="foo">value</name></product>', %w(foo)
- end
- end
-
- def test_from_xml_disallows_symbol_and_yaml_types_by_default
- assert_raise ActiveSupport::XMLConverter::DisallowedType do
- Hash.from_xml '<product><name type="symbol">value</name></product>'
- end
-
- assert_raise ActiveSupport::XMLConverter::DisallowedType do
- Hash.from_xml '<product><name type="yaml">value</name></product>'
- end
- end
-
- def test_from_trusted_xml_allows_symbol_and_yaml_types
- expected = { 'product' => { 'name' => :value }}
- assert_equal expected, Hash.from_trusted_xml('<product><name type="symbol">value</name></product>')
- assert_equal expected, Hash.from_trusted_xml('<product><name type="yaml">:value</name></product>')
- end
-
def test_should_use_default_value_for_unknown_key
hash_wia = HashWithIndifferentAccess.new(3)
assert_equal 3, hash_wia[:new_key]