diff options
Diffstat (limited to 'actionpack')
23 files changed, 156 insertions, 73 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 931313612c..d473ab427d 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add application/gzip as a default mime type. + + *Mehmet Emin İNAÇ* + * Add request encoding and response parsing to integration tests. What previously was: diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 1641d01c30..f6e67b02d7 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -175,10 +175,7 @@ module ActionController body = [body] unless body.nil? || body.respond_to?(:each) response.reset_body! return unless body - body.each { |part| - next if part.empty? - response.write part - } + response.body = body super end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 6586985ff5..b2f0b382b9 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -379,7 +379,8 @@ module ActionController #:nodoc: def xor_byte_strings(s1, s2) s2_bytes = s2.bytes - s1.bytes.map.with_index { |c1, i| c1 ^ s2_bytes[i] }.pack('c*') + s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } + s2_bytes.pack('C*') end # The form's authenticity parameter. Override to provide your own. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d3382ef296..25ec3cf5b6 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -109,7 +109,7 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :inspect, + delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these @@ -122,16 +122,6 @@ module ActionController cattr_accessor :always_permitted_parameters self.always_permitted_parameters = %w( controller action ) - def self.const_missing(const_name) - return super unless const_name == :NEVER_UNPERMITTED_PARAMS - ActiveSupport::Deprecation.warn(<<-MSG.squish) - `ActionController::Parameters::NEVER_UNPERMITTED_PARAMS` has been deprecated. - Use `ActionController::Parameters.always_permitted_parameters` instead. - MSG - - always_permitted_parameters - end - # Returns a new instance of <tt>ActionController::Parameters</tt>. # Also, sets the +permitted+ attribute to the default value of # <tt>ActionController::Parameters.permit_all_parameters</tt>. @@ -154,17 +144,21 @@ module ActionController end # Returns true if another +Parameters+ object contains the same content and - # permitted flag, or other Hash-like object contains the same content. This - # override is in place so you can perform a comparison with `Hash`. - def ==(other_hash) - if other_hash.respond_to?(:permitted?) - super + # permitted flag. + def ==(other) + if other.respond_to?(:permitted?) + self.permitted? == other.permitted? && self.parameters == other.parameters + elsif other.is_a?(Hash) + ActiveSupport::Deprecation.warn <<-WARNING.squish + Comparing equality between `ActionController::Parameters` and a + `Hash` is deprecated and will be removed in Rails 5.1. Please only do + comparisons between instances of `ActionController::Parameters`. If + you need to compare to a hash, first convert it using + `ActionController::Parameters#new`. + WARNING + @parameters == other.with_indifferent_access else - if other_hash.is_a?(Hash) - @parameters == other_hash.with_indifferent_access - else - @parameters == other_hash - end + @parameters == other end end @@ -584,6 +578,10 @@ module ActionController dup end + def inspect + "<#{self.class} #{@parameters}>" + end + def method_missing(method_sym, *args, &block) if @parameters.respond_to?(method_sym) message = <<-DEPRECATE.squish @@ -603,12 +601,14 @@ module ActionController end protected + attr_reader :parameters + def permitted=(new_permitted) @permitted = new_permitted end def fields_for_style? - @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } + @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } end private diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 8356d1a238..66cea88256 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -28,7 +28,8 @@ Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form # http://www.ietf.org/rfc/rfc4627.txt # http://www.json.org/JSONRequest.html -Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest application/vnd.api+json ) +Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) Mime::Type.register "application/pdf", :pdf, [], %w(pdf) Mime::Type.register "application/zip", :zip, [], %w(zip) +Mime::Type.register "application/gzip", :gzip, %w(application/x-gzip), %w(gz) diff --git a/actionpack/lib/action_dispatch/journey/backwards.rb b/actionpack/lib/action_dispatch/journey/backwards.rb deleted file mode 100644 index 3bd20fdf81..0000000000 --- a/actionpack/lib/action_dispatch/journey/backwards.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Rack # :nodoc: - Mount = ActionDispatch::Journey::Router - Mount::RouteSet = ActionDispatch::Journey::Router - Mount::RegexpWithNamedGroups = ActionDispatch::Journey::Path::Pattern -end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index f649588520..06cdce1724 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -16,9 +16,6 @@ module ActionDispatch class RoutingError < ::StandardError # :nodoc: end - # :nodoc: - VERSION = '2.0.0' - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 3477aa8b29..f2f3150b56 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/key_generator' require 'active_support/message_verifier' require 'active_support/json' +require 'rack/utils' module ActionDispatch class Request @@ -337,7 +338,7 @@ module ActionDispatch end def to_header - @cookies.map { |k,v| "#{k}=#{v}" }.join ';' + @cookies.map { |k,v| "#{escape(k)}=#{escape(v)}" }.join '; ' end def handle_options(options) #:nodoc: @@ -419,6 +420,10 @@ module ActionDispatch private + def escape(string) + ::Rack::Utils.escape(string) + end + def make_set_cookie_header(header) header = @set_cookies.inject(header) { |m, (k, v)| if write_cookie?(v) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index b55c937e0c..51a471fb23 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -156,15 +156,20 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << trace.join("\n ") - logger.fatal("#{message}\n\n") + logger.fatal " " + logger.fatal "#{exception.class} (#{exception.message}):" + log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) + logger.fatal " " + log_array logger, trace end end + def log_array(logger, array) + array.map { |line| logger.fatal line } + end + def logger(request) - request.logger || stderr_logger + request.logger || ActionView::Base.logger || stderr_logger end def stderr_logger diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 429a98f236..dec9c60ef2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -23,7 +23,7 @@ module ActionDispatch # goes a step further than signed cookies in that encrypted cookies cannot # be altered or read by users. This is the default starting in Rails 4. # - # If you have both secret_token and secret_key base set, your cookies will + # If you have both secret_token and secret_key_base set, your cookies will # be encrypted, and signed cookies generated by Rails 3 will be # transparently read and encrypted to provide a smooth upgrade path. # diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 79f9283f83..dcf800b215 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -159,7 +159,7 @@ module ActionDispatch # # controller 'geocode' do # get 'geocode/:postalcode' => :show, constraints: { - # postalcode: /# Postcode format + # postalcode: /# Postalcode format # \d{5} #Prefix # (-\d{4})? #Suffix # /x diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 846b5fa1fc..310e98f584 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -289,7 +289,7 @@ module ActionDispatch if last.permitted? args.pop.to_h else - raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" + raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" end end helper.call self, args, options diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index f91679593e..28be189f93 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -173,7 +173,7 @@ module ActionDispatch route_name) when ActionController::Parameters unless options.permitted? - raise ArgumentError.new("Generating an URL from non sanitized request parameters is insecure!") + raise ArgumentError.new("Generating a URL from non sanitized request parameters is insecure!") end route_name = options.delete :use_route _routes.url_for(options.to_h.symbolize_keys. diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 74c78dfa8e..754ac144cc 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -381,14 +381,14 @@ class CollectionCacheController < ActionController::Base render 'index' end - def index_explicit_render + def index_explicit_render_in_controller @customers = [Customer.new('david', 1)] - render partial: 'customers/customer', collection: @customers + render partial: 'customers/customer', collection: @customers, cached: true end def index_with_comment @customers = [Customer.new('david', 1)] - render partial: 'customers/commented_customer', collection: @customers, as: :customer + render partial: 'customers/commented_customer', collection: @customers, as: :customer, cached: true end end @@ -399,12 +399,13 @@ class AutomaticCollectionCacheTest < ActionController::TestCase @controller.perform_caching = true @controller.partial_rendered_times = 0 @controller.cache_store = ActiveSupport::Cache::MemoryStore.new - ActionView::PartialRenderer.collection_cache = @controller.cache_store + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new end def test_collection_fetches_cached_views get :index assert_equal 1, @controller.partial_rendered_times + assert_customer_cached 'david/1', 'david, 1' get :index assert_equal 1, @controller.partial_rendered_times @@ -412,13 +413,16 @@ class AutomaticCollectionCacheTest < ActionController::TestCase def test_preserves_order_when_reading_from_cache_plus_rendering get :index, params: { id: 2 } - get :index_ordered + assert_equal 1, @controller.partial_rendered_times + assert_select ':root', 'david, 2' + get :index_ordered + assert_equal 3, @controller.partial_rendered_times assert_select ':root', "david, 1\n david, 2\n david, 3" end def test_explicit_render_call_with_options - get :index_explicit_render + get :index_explicit_render_in_controller assert_select ':root', "david, 1" end @@ -430,6 +434,12 @@ class AutomaticCollectionCacheTest < ActionController::TestCase get :index_with_comment assert_equal 1, @controller.partial_rendered_times end + + private + def assert_customer_cached(key, content) + assert_match content, + ActionView::PartialRenderer.collection_cache.read("views/#{key}/7c228ab609f0baf0b1f2367469210937") + end end class FragmentCacheKeyTestController < CachingController diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index bd43ff7697..4ef5bed30d 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -129,9 +129,59 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_not @params[:person].values_at(:name).first.permitted? end - test "equality with another hash works" do + test "equality with a hash is deprecated" do hash1 = { foo: :bar } params1 = ActionController::Parameters.new(hash1) - assert(params1 == hash1) + assert_deprecated("will be removed in Rails 5.1") do + assert(params1 == hash1) + end + end + + test "is equal to Parameters instance with same params" do + params1 = ActionController::Parameters.new(a: 1, b: 2) + params2 = ActionController::Parameters.new(a: 1, b: 2) + assert(params1 == params2) + end + + test "is equal to Parameters instance with same permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + params2 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + assert(params1 == params2) + end + + test "is equal to Parameters instance with same different source params, but same permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + params2 = ActionController::Parameters.new(a: 1, c: 3).permit(:a) + assert(params1 == params2) + assert(params2 == params1) + end + + test 'is not equal to an unpermitted Parameters instance with same params' do + params1 = ActionController::Parameters.new(a: 1).permit(:a) + params2 = ActionController::Parameters.new(a: 1) + assert(params1 != params2) + assert(params2 != params1) + end + + test "is not equal to Parameters instance with different permitted params" do + params1 = ActionController::Parameters.new(a: 1, b: 2).permit(:a, :b) + params2 = ActionController::Parameters.new(a: 1, b: 2).permit(:a) + assert(params1 != params2) + assert(params2 != params1) + end + + test "equality with simple types works" do + assert(@params != 'Hello') + assert(@params != 42) + assert(@params != false) + end + + test "inspect shows both class name and parameters" do + assert_equal( + '<ActionController::Parameters {"person"=>{"age"=>"32", '\ + '"name"=>{"first"=>"David", "last"=>"Heinemeier Hansson"}, ' \ + '"addresses"=>[{"city"=>"Chicago", "state"=>"Illinois"}]}}>', + @params.inspect + ) end end diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index efaf8a96c3..c5bfb10b53 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -12,12 +12,6 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase ActionController::Parameters.always_permitted_parameters = %w( controller action ) end - test "shows deprecations warning on NEVER_UNPERMITTED_PARAMS" do - assert_deprecated do - ActionController::Parameters::NEVER_UNPERMITTED_PARAMS - end - end - test "returns super on missing constant other than NEVER_UNPERMITTED_PARAMS" do ActionController::Parameters.superclass.stub :const_missing, "super" do assert_equal "super", ActionController::Parameters::NON_EXISTING_CONSTANT diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 3299f2d9d0..96048e2868 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -27,6 +27,27 @@ class ParametersPermitTest < ActiveSupport::TestCase end end + def walk_permitted params + params.each do |k,v| + case v + when ActionController::Parameters + walk_permitted v + when Array + v.each { |x| walk_permitted v } + end + end + end + + test 'iteration should not impact permit' do + hash = {"foo"=>{"bar"=>{"0"=>{"baz"=>"hello", "zot"=>"1"}}}} + params = ActionController::Parameters.new(hash) + + walk_permitted params + + sanitized = params[:foo].permit(bar: [:baz]) + assert_equal({"0"=>{"baz"=>"hello"}}, sanitized[:bar].to_unsafe_h) + end + test 'if nothing is permitted, the hash becomes empty' do params = ActionController::Parameters.new(id: '1234') permitted = params.permit diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 0b184eace9..3ea03be74a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -310,7 +310,7 @@ class RedirectTest < ActionController::TestCase error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Generating an URL from non sanitized request parameters is insecure!", error.message + assert_equal "Generating a URL from non sanitized request parameters is insecure!", error.message end def test_redirect_to_with_block diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index c814d4ea54..60c6518c62 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -509,7 +509,7 @@ class EtagRenderTest < ActionController::TestCase begin File.write path, 'foo' - ActionView::Digestor.cache.clear + ActionView::LookupContext::DetailsKey.clear request.if_none_match = etag get :with_template diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index b9caddcdb7..0c1393548e 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -137,6 +137,10 @@ XML head :created, location: 'created resource' end + def render_cookie + render plain: cookies["foo"] + end + def delete_cookie cookies.delete("foo") render plain: 'ok' @@ -829,6 +833,12 @@ XML assert_equal 'bar', cookies['foo'] end + def test_cookies_should_be_escaped_properly + cookies['foo'] = '+' + get :render_cookie + assert_equal '+', @response.body + end + def test_should_detect_if_cookie_is_deleted cookies['foo'] = 'bar' get :delete_cookie diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 149e37bf3d..672b272590 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -49,7 +49,7 @@ class MimeTypeTest < ActiveSupport::TestCase test "parse application with trailing star" do accept = "application/*" - expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip]] + expect = [Mime[:html], Mime[:js], Mime[:xml], Mime[:rss], Mime[:atom], Mime[:yaml], Mime[:url_encoded_form], Mime[:json], Mime[:pdf], Mime[:zip], Mime[:gzip]] parsed = Mime::Type.parse(accept) assert_equal expect, parsed end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index a3992ad008..64801bff39 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -37,9 +37,9 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do + test "does not parse unregistered media types such as application/vnd.api+json" do assert_parses( - {"person" => {"name" => "David"}}, + {}, "{\"person\": {\"name\": \"David\"}}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } ) end @@ -143,13 +143,6 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end - test "parses json params for application/vnd.api+json" do - assert_parses( - {"user" => {"username" => "sikachu"}, "username" => "sikachu"}, - "{\"username\": \"sikachu\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } - ) - end - test "parses json with non-object JSON content" do assert_parses( {"user" => {"_json" => "string content" }, "_json" => "string content" }, diff --git a/actionpack/test/fixtures/collection_cache/index.html.erb b/actionpack/test/fixtures/collection_cache/index.html.erb index 521b1450df..853e501ab4 100644 --- a/actionpack/test/fixtures/collection_cache/index.html.erb +++ b/actionpack/test/fixtures/collection_cache/index.html.erb @@ -1 +1 @@ -<%= render @customers %>
\ No newline at end of file +<%= render partial: 'customers/customer', collection: @customers, cached: true %> |