From 837e40dcac023319a0bfc38240761d4352b73b99 Mon Sep 17 00:00:00 2001 From: Dave Gynn Date: Sat, 26 Dec 2015 22:25:27 -0800 Subject: restore ability to pass extra options to cache stores The `cache` helper methods should pass any extra options to the cache store. For example :expires_in would be a valid option if memcache was the cache store. The change in commit da16745 broke the ability to pass any options other than :skip_digest and :virtual_path. This PR restores that functionality and adds a test for it. --- actionpack/test/controller/caching_test.rb | 12 ++++++++++++ .../functional_caching/fragment_cached_with_options.html.erb | 3 +++ 2 files changed, 15 insertions(+) create mode 100644 actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb (limited to 'actionpack') diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index d19b3810c2..74c78dfa8e 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -172,6 +172,9 @@ class FunctionalCachingController < CachingController def fragment_cached_without_digest end + + def fragment_cached_with_options + end end class FunctionalFragmentCachingTest < ActionController::TestCase @@ -215,6 +218,15 @@ CACHED assert_equal "

ERB

", @store.read("views/nodigest") end + def test_fragment_caching_with_options + get :fragment_cached_with_options + assert_response :success + expected_body = "\n

ERB

\n\n" + + assert_equal expected_body, @response.body + assert_equal "

ERB

", @store.read("views/with_options") + end + def test_render_inline_before_fragment_caching get :inline_fragment_cached assert_response :success diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb new file mode 100644 index 0000000000..6865df9b7e --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_with_options.html.erb @@ -0,0 +1,3 @@ + +<%= cache 'with_options', :skip_digest => true, :expires_in => 1.minute do %>

ERB

<% end %> + -- cgit v1.2.3 From 52bb2d36d3141dcd8217221065d9b5fa2b12deba Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sat, 19 Sep 2015 00:11:18 +0200 Subject: Add `as` to encode a request as a specific mime type. Turns ``` post articles_path(format: :json), params: { article: { name: 'Ahoy!' } }.to_json, headers: { 'Content-Type' => 'application/json' } ``` into ``` post articles_path, params: { article: { name: 'Ahoy!' } }, as: :json ``` --- .../lib/action_dispatch/testing/integration.rb | 83 ++++++++++++++++++++-- actionpack/test/controller/integration_test.rb | 43 +++++++++++ 2 files changed, 122 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 711ca10419..cade3bf6b3 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -321,7 +321,9 @@ module ActionDispatch end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil, xhr: false) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) + request_encoder = RequestEncoder.encoder(as) + if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -330,14 +332,17 @@ module ActionDispatch url_host += ":#{location.port}" if default != location.port host! url_host end - path = location.query ? "#{location.path}?#{location.query}" : location.path + path = request_encoder.append_format_to location.path + path = location.query ? "#{path}?#{location.query}" : path + else + path = request_encoder.append_format_to path end hostname, port = host.split(':') request_env = { :method => method, - :params => params, + :params => request_encoder.encode_params(params), "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -347,7 +352,7 @@ module ActionDispatch "REQUEST_URI" => path, "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, - "CONTENT_TYPE" => "application/x-www-form-urlencoded", + "CONTENT_TYPE" => request_encoder.content_type, "HTTP_ACCEPT" => accept } @@ -387,6 +392,48 @@ module ActionDispatch def build_full_uri(path, env) "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + + class RequestEncoder # :nodoc: + @encoders = {} + + def initialize(mime_name, param_encoder, url_encoded_form = false) + @mime = Mime[mime_name] + + unless @mime + raise ArgumentError, "Can't register a request encoder for " \ + "unregistered MIME Type: #{mime_name}. See `Mime::Type.register`." + end + + @url_encoded_form = url_encoded_form + @path_format = ".#{@mime.symbol}" unless @url_encoded_form + @param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc + end + + def append_format_to(path) + path << @path_format unless @url_encoded_form + path + end + + def content_type + @mime.to_s + end + + def encode_params(params) + @param_encoder.call(params) + end + + def self.encoder(name) + @encoders[name] || WWWFormEncoder + end + + def self.register_encoder(mime_name, ¶m_encoder) + @encoders[mime_name] = new(mime_name, param_encoder) + end + + register_encoder :json + + WWWFormEncoder = new(:url_encoded_form, -> params { params }, true) + end end module Runner @@ -643,6 +690,30 @@ module ActionDispatch # end # end # + # You can also test your JSON API easily by setting what the request should + # be encoded as: + # + # require 'test_helper' + # + # class ApiTest < ActionDispatch::IntegrationTest + # test "creates articles" do + # assert_difference -> { Article.count } do + # post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + # end + # + # assert_response :success + # end + # end + # + # The `as` option sets the format to JSON, sets the content type to + # 'application/json' and encodes the parameters as JSON. + # + # For any custom MIME Types you've registered, you can even add your own encoders with: + # + # ActionDispatch::IntegrationTest.register_encoder :wibble do |params| + # params.to_wibble + # end + # # Consult the Rails Testing Guide for more. class IntegrationTest < ActiveSupport::TestCase @@ -671,5 +742,9 @@ module ActionDispatch def document_root_element html_document.root end + + def self.register_encoder(*args, ¶m_encoder) + Integration::Session::RequestEncoder.register_encoder(*args, ¶m_encoder) + end end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index d0a1d1285f..296bc1baad 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -1126,3 +1126,46 @@ class IntegrationRequestsWithSessionSetup < ActionDispatch::IntegrationTest assert_equal({"user_name"=>"david"}, cookies.to_hash) end end + +class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def foos + render plain: 'ok' + end + end + + def test_encoding_as_json + assert_encoded_as :json, content_type: 'application/json' + end + + def test_encoding_as_without_mime_registration + assert_raise ArgumentError do + ActionDispatch::IntegrationTest.register_encoder :wibble + end + end + + def test_registering_custom_encoder + Mime::Type.register 'text/wibble', :wibble + + ActionDispatch::IntegrationTest.register_encoder(:wibble, &:itself) + + assert_encoded_as :wibble, content_type: 'text/wibble', + parsed_parameters: Hash.new # Unregistered MIME Type can't be parsed + ensure + Mime::Type.unregister :wibble + end + + private + def assert_encoded_as(format, content_type:, parsed_parameters: { 'foo' => 'fighters' }) + with_routing do |routes| + routes.draw { post ':action' => FooController } + + post '/foos', params: { foo: 'fighters' }, as: format + + assert_response :success + assert_match "foos.#{format}", request.path + assert_equal content_type, request.content_type + assert_equal parsed_parameters, request.request_parameters + end + end +end -- cgit v1.2.3 From 0361d8449ff1c18da041df4b7dfe648abf0f1887 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 17:06:31 -0800 Subject: clear view path cache between tests The cache for `render file:` seems to also be used in the case of `render(string)`. If one is supposed to be a hit and the other is supposed to be a miss, and they both reference the same file, then the cache could return incorrect values. This commit clears the cache between runs so that we get non-cached behavior. --- actionpack/test/controller/render_test.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 3f569230c2..db73de6010 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -253,6 +253,11 @@ end class ExpiresInRenderTest < ActionController::TestCase tests TestController + def setup + super + ActionController::Base.view_paths.paths.each(&:clear_cache) + end + def test_dynamic_render_with_file # This is extremely bad, but should be possible to do. assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) -- cgit v1.2.3 From 534b12afb5cd0240128c3552394004f15b18520c Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Mon, 25 Jan 2016 15:21:11 -0500 Subject: Fix undefined error for `ActionController::Parameters` --- actionpack/lib/abstract_controller/rendering.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 63fd76d9b7..841a4c07ad 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -82,13 +82,10 @@ module AbstractController # render :file => "foo/bar". # :api: plugin def _normalize_args(action=nil, options={}) - case action - when ActionController::Parameters - unless action.permitted? - raise ArgumentError, "render parameters are not permitted" - end + if action.respond_to?(:permitted?) && action.permitted? + raise ArgumentError, "render parameters are not permitted" action - when Hash + elsif action.is_a?(Hash) action else options -- cgit v1.2.3 From 00285e7cf75c96553719072a27c27e4ab7d25b40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 18:00:05 -0800 Subject: fix permitted? conditional for `render` calls --- actionpack/lib/abstract_controller/rendering.rb | 9 ++++++--- actionpack/test/controller/render_test.rb | 11 +++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 841a4c07ad..e765d73ce4 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -82,9 +82,12 @@ module AbstractController # render :file => "foo/bar". # :api: plugin def _normalize_args(action=nil, options={}) - if action.respond_to?(:permitted?) && action.permitted? - raise ArgumentError, "render parameters are not permitted" - action + if action.respond_to?(:permitted?) + if action.permitted? + action + else + raise ArgumentError, "render parameters are not permitted" + end elsif action.is_a?(Hash) action else diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index db73de6010..f205b96ce8 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -66,6 +66,10 @@ class TestController < ActionController::Base render params[:id] # => String, AC:Params end + def dynamic_render_permit + render params[:id].permit(:file) + end + def dynamic_render_with_file # This is extremely bad, but should be possible to do. file = params[:id] # => String, AC:Params @@ -273,6 +277,13 @@ class ExpiresInRenderTest < ActionController::TestCase end end + def test_permitted_dynamic_render_file_hash + assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) + response = get :dynamic_render_permit, { id: { file: '../\\../test/abstract_unit.rb' } } + assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), + response.body + end + def test_dynamic_render_file_hash assert_raises ArgumentError do get :dynamic_render, params: { id: { file: '../\\../test/abstract_unit.rb' } } -- cgit v1.2.3 From 3844854af109fb9eee75c90bacf8bf87eb2bf968 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 26 Jan 2016 18:01:24 -0800 Subject: add a skip for failing test --- actionpack/test/controller/render_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index f205b96ce8..ea789b6121 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -278,8 +278,9 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_permitted_dynamic_render_file_hash + skip "FIXME: this test passes on 4-2-stable but not master. Why?" assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) - response = get :dynamic_render_permit, { id: { file: '../\\../test/abstract_unit.rb' } } + response = get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } } assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')), response.body end -- cgit v1.2.3 From 5262bf544abacb10828abcfeb046d8146bef4c14 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Wed, 27 Jan 2016 18:19:15 +0900 Subject: doc typo [ci skip] --- actionpack/test/controller/render_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index ea789b6121..d1b9586533 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -63,7 +63,7 @@ class TestController < ActionController::Base end def dynamic_render - render params[:id] # => String, AC:Params + render params[:id] # => String, AC::Params end def dynamic_render_permit @@ -72,7 +72,7 @@ class TestController < ActionController::Base def dynamic_render_with_file # This is extremely bad, but should be possible to do. - file = params[:id] # => String, AC:Params + file = params[:id] # => String, AC::Params render file: file end -- cgit v1.2.3 From 385e0a3311960032fa149f8650d73fd483dc5f75 Mon Sep 17 00:00:00 2001 From: Marek Date: Wed, 27 Jan 2016 18:41:48 +0000 Subject: Fix typo in strong params hash deprecation message and remove unecessary spaces in string interpolation. --- actionpack/lib/action_controller/metal/strong_parameters.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index ba03430930..d3382ef296 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -587,11 +587,11 @@ module ActionController def method_missing(method_sym, *args, &block) if @parameters.respond_to?(method_sym) message = <<-DEPRECATE.squish - Method #{ method_sym } is deprecated and will be removed in Rails 5.1, + Method #{method_sym} is deprecated and will be removed in Rails 5.1, as `ActionController::Parameters` no longer inherits from hash. Using this deprecated behavior exposes potential security problems. If you continue to use this method you may be creating - a security vulunerability in your app that can be exploited. Instead, + a security vulnerability in your app that can be exploited. Instead, consider using one of these documented methods which are not deprecated: http://api.rubyonrails.org/v#{ActionPack.version}/classes/ActionController/Parameters.html DEPRECATE -- cgit v1.2.3 From 542b2e9c9b7e24f5337fd529a36f9d89f727e189 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 27 Jan 2016 17:46:06 -0800 Subject: change `@text_xml_idx` to an lvar and cache it on the stack this eliminates the ivar lookup and also eliminates the `||=` conditional that happens every time we called the `text_xml_idx` method. --- actionpack/lib/action_dispatch/http/mime_type.rb | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 6abbf9c754..a90c691ff7 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -116,13 +116,18 @@ module Mime def assort! sort! + text_xml_idx = index('text/xml') + # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two - exchange_xml_items if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list + app_xml.q = [text_xml(text_xml_idx).q, app_xml.q].max # set the q value to the max of the two + if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list + exchange_xml_items(text_xml_idx) + @app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx + end delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - text_xml.name = Mime[:xml].to_s + text_xml(text_xml_idx).name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml @@ -146,25 +151,18 @@ module Mime end private - def text_xml_idx - @text_xml_idx ||= index('text/xml') - end - def app_xml_idx @app_xml_idx ||= index(Mime[:xml].to_s) end - def text_xml - self[text_xml_idx] - end + alias :text_xml :[] def app_xml self[app_xml_idx] end - def exchange_xml_items - self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml - @app_xml_idx, @text_xml_idx = text_xml_idx, app_xml_idx + def exchange_xml_items(text_xml_idx) + self[app_xml_idx], self[text_xml_idx] = text_xml(text_xml_idx), app_xml end end -- cgit v1.2.3 From f7f59c2eb732e9cb26060058e0ccac7dc74f324c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 27 Jan 2016 17:50:16 -0800 Subject: change `@app_xml_idx` to an lvar and cache it on the stack same strategy as `@text_xml_idx`: cache it on the stack to avoid ivar lookups and the `||=` call. --- actionpack/lib/action_dispatch/http/mime_type.rb | 26 +++++++++--------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index a90c691ff7..d953030139 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -117,13 +117,14 @@ module Mime sort! text_xml_idx = index('text/xml') + app_xml_idx = index(Mime[:xml].to_s) # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml.q = [text_xml(text_xml_idx).q, app_xml.q].max # set the q value to the max of the two + app_xml(app_xml_idx).q = [text_xml(text_xml_idx).q, app_xml(app_xml_idx).q].max # set the q value to the max of the two if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - exchange_xml_items(text_xml_idx) - @app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx + exchange_xml_items(text_xml_idx, app_xml_idx) + app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx @@ -136,11 +137,11 @@ module Mime while idx < length type = self[idx] - break if type.q < app_xml.q + break if type.q < app_xml(app_xml_idx).q if type.name.ends_with? '+xml' - self[app_xml_idx], self[idx] = self[idx], app_xml - @app_xml_idx = idx + self[app_xml_idx], self[idx] = self[idx], app_xml(app_xml_idx) + app_xml_idx = idx end idx += 1 end @@ -151,18 +152,11 @@ module Mime end private - def app_xml_idx - @app_xml_idx ||= index(Mime[:xml].to_s) - end - alias :text_xml :[] + alias :app_xml :[] - def app_xml - self[app_xml_idx] - end - - def exchange_xml_items(text_xml_idx) - self[app_xml_idx], self[text_xml_idx] = text_xml(text_xml_idx), app_xml + def exchange_xml_items(text_xml_idx, app_xml_idx) + self[app_xml_idx], self[text_xml_idx] = text_xml(text_xml_idx), app_xml(app_xml_idx) end end -- cgit v1.2.3 From 7fc79bc7907ea53aead3ff8461daedaacbf1cd71 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 27 Jan 2016 17:58:26 -0800 Subject: remove useless private methods This commit refactors the private methods that were just aliases to [] to just directly use [] and cache the return values on the stack. --- actionpack/lib/action_dispatch/http/mime_type.rb | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index d953030139..4a57e04c3a 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -121,26 +121,30 @@ module Mime # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml(app_xml_idx).q = [text_xml(text_xml_idx).q, app_xml(app_xml_idx).q].max # set the q value to the max of the two + app_xml = self[app_xml_idx] + text_xml = self[text_xml_idx] + + app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - exchange_xml_items(text_xml_idx, app_xml_idx) + self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - text_xml(text_xml_idx).name = Mime[:xml].to_s + self[text_xml_idx].name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml if app_xml_idx + app_xml = self[app_xml_idx] idx = app_xml_idx while idx < length type = self[idx] - break if type.q < app_xml(app_xml_idx).q + break if type.q < app_xml.q if type.name.ends_with? '+xml' - self[app_xml_idx], self[idx] = self[idx], app_xml(app_xml_idx) + self[app_xml_idx], self[idx] = self[idx], app_xml app_xml_idx = idx end idx += 1 @@ -150,14 +154,6 @@ module Mime map! { |i| Mime::Type.lookup(i.name) }.uniq! to_a end - - private - alias :text_xml :[] - alias :app_xml :[] - - def exchange_xml_items(text_xml_idx, app_xml_idx) - self[app_xml_idx], self[text_xml_idx] = text_xml(text_xml_idx), app_xml(app_xml_idx) - end end class << self -- cgit v1.2.3 From a447252ac4c97b06df271c04ddd7530014dd8c86 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 27 Jan 2016 18:05:15 -0800 Subject: remove == from AcceptItem Remove nonsense definition of == from `AcceptItem`. The definition only compared names and not `q` values or even object identity. The only use was in the `assort!` method that really just wanted the index of the item given the item's name. Instead we just change the caller to use `index` with the block form. --- actionpack/lib/action_dispatch/http/mime_type.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 4a57e04c3a..463b5fe405 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -106,18 +106,14 @@ module Mime result = @index <=> item.index if result == 0 result end - - def ==(item) - @name == item.to_s - end end class AcceptList < Array #:nodoc: def assort! sort! - text_xml_idx = index('text/xml') - app_xml_idx = index(Mime[:xml].to_s) + text_xml_idx = find_item_by_name self, 'text/xml' + app_xml_idx = find_item_by_name self, Mime[:xml].to_s # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx @@ -154,6 +150,11 @@ module Mime map! { |i| Mime::Type.lookup(i.name) }.uniq! to_a end + + private + def find_item_by_name(array, name) + array.index { |item| item.name == name } + end end class << self -- cgit v1.2.3 From 020d6fda29c1f04818dbf467764fc8ac16b7042f Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 28 Jan 2016 14:18:01 -0500 Subject: Regression test for rendering file from absolute path Test that we are not allowing you to grab a file with an absolute path outside of your application directory. This is dangerous because it could be used to retrieve files from the server like `/etc/passwd`. --- actionpack/test/controller/render_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index d1b9586533..2e1a687513 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -270,6 +270,17 @@ class ExpiresInRenderTest < ActionController::TestCase response.body end + def test_dynamic_render_with_absolute_path + file = Tempfile.new + file.write "secrets!" + file.flush + assert_raises ActionView::MissingTemplate do + response = get :dynamic_render, params: { id: file.path } + end + ensure + file.unlink + end + def test_dynamic_render assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')) assert_raises ActionView::MissingTemplate do -- cgit v1.2.3 From 5181d5c283430762ea444d26e72890501a6584ff Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 28 Jan 2016 15:22:02 -0500 Subject: Run `file.close` before unlinking for travis This works on OSX but for some reason travis is throwing a ``` 1) Error: ExpiresInRenderTest#test_dynamic_render_with_absolute_path: NoMethodError: undefined method `unlink' for nil:NilClass ``` Looking at other tests in Railties the file has a name and we close it before unlinking, so I'm going to try that. --- actionpack/test/controller/render_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 2e1a687513..9430ab8db8 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -271,13 +271,14 @@ class ExpiresInRenderTest < ActionController::TestCase end def test_dynamic_render_with_absolute_path - file = Tempfile.new + file = Tempfile.new('name') file.write "secrets!" file.flush assert_raises ActionView::MissingTemplate do response = get :dynamic_render, params: { id: file.path } end ensure + file.close file.unlink end -- cgit v1.2.3 From f8c9ef8401ab11db5f64fc1e13e8b1fe5a0803ce Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 28 Jan 2016 14:14:17 -0800 Subject: convert AcceptList to a regular class we never use this custom array outside the mime type `parse` method. We can reduce the interaction to just a regular array, so we should use that instead (IOW, there was nothing special about AcceptList so we should remove it). --- actionpack/lib/action_dispatch/http/mime_type.rb | 39 ++++++++++++------------ 1 file changed, 19 insertions(+), 20 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 463b5fe405..065d7dd355 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -108,51 +108,50 @@ module Mime end end - class AcceptList < Array #:nodoc: - def assort! - sort! + class AcceptList #:nodoc: + def self.sort!(list) + list.sort! - text_xml_idx = find_item_by_name self, 'text/xml' - app_xml_idx = find_item_by_name self, Mime[:xml].to_s + text_xml_idx = find_item_by_name list, 'text/xml' + app_xml_idx = find_item_by_name list, Mime[:xml].to_s # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml = self[app_xml_idx] - text_xml = self[text_xml_idx] + app_xml = list[app_xml_idx] + text_xml = list[text_xml_idx] app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml + list[app_xml_idx], list[text_xml_idx] = text_xml, app_xml app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end - delete_at(text_xml_idx) # delete text_xml from the list + list.delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - self[text_xml_idx].name = Mime[:xml].to_s + list[text_xml_idx].name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml if app_xml_idx - app_xml = self[app_xml_idx] + app_xml = list[app_xml_idx] idx = app_xml_idx - while idx < length - type = self[idx] + while idx < list.length + type = list[idx] break if type.q < app_xml.q if type.name.ends_with? '+xml' - self[app_xml_idx], self[idx] = self[idx], app_xml + list[app_xml_idx], list[idx] = list[idx], app_xml app_xml_idx = idx end idx += 1 end end - map! { |i| Mime::Type.lookup(i.name) }.uniq! - to_a + list.map! { |i| Mime::Type.lookup(i.name) }.uniq! + list end - private - def find_item_by_name(array, name) + def self.find_item_by_name(array, name) array.index { |item| item.name == name } end end @@ -198,7 +197,7 @@ module Mime accept_header = accept_header.split(PARAMETER_SEPARATOR_REGEXP).first parse_trailing_star(accept_header) || [Mime::Type.lookup(accept_header)].compact else - list, index = AcceptList.new, 0 + list, index = [], 0 accept_header.split(',').each do |header| params, q = header.split(PARAMETER_SEPARATOR_REGEXP) if params.present? @@ -212,7 +211,7 @@ module Mime end end end - list.assort! + AcceptList.sort! list end end -- cgit v1.2.3 From c082a7aae4a41b52e58c7dea43e7e9c6c41d11ad Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 28 Jan 2016 16:08:03 -0800 Subject: speed up accept header parsing a bit. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept header is taken from what Safari on El Capitan sends: ```ruby require 'benchmark/ips' require 'action_dispatch/http/mime_type' require 'active_support/all' accept = 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' Benchmark.ips do |x| x.report "omg" do Mime::Type.parse(accept) end end ``` Before: ``` [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 3.181k i/100ms ------------------------------------------------- omg 35.062k (±12.8%) i/s - 174.955k [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 3.153k i/100ms ------------------------------------------------- omg 33.724k (±12.4%) i/s - 167.109k [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 3.575k i/100ms ------------------------------------------------- omg 37.251k (±10.4%) i/s - 185.900k ``` After: ``` [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 3.365k i/100ms ------------------------------------------------- omg 40.069k (±16.1%) i/s - 198.535k [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 4.168k i/100ms ------------------------------------------------- omg 47.596k (± 7.7%) i/s - 237.576k [aaron@TC actionpack (master)]$ be ruby ../x.rb Calculating ------------------------------------- omg 4.282k i/100ms ------------------------------------------------- omg 43.626k (±17.7%) i/s - 209.818k ``` --- actionpack/lib/action_dispatch/http/mime_type.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 065d7dd355..4672ea7199 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,3 +1,5 @@ +# -*- frozen-string-literal: true -*- + require 'singleton' require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/starts_ends_with' @@ -157,7 +159,7 @@ module Mime end class << self - TRAILING_STAR_REGEXP = /(text|application)\/\*/ + TRAILING_STAR_REGEXP = /^(text|application)\/\*/ PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/ def register_callback(&block) @@ -200,15 +202,16 @@ module Mime list, index = [], 0 accept_header.split(',').each do |header| params, q = header.split(PARAMETER_SEPARATOR_REGEXP) - if params.present? - params.strip! - params = parse_trailing_star(params) || [params] + next unless params + params.strip! + next if params.empty? + + params = parse_trailing_star(params) || [params] - params.each do |m| - list << AcceptItem.new(index, m.to_s, q) - index += 1 - end + params.each do |m| + list << AcceptItem.new(index, m.to_s, q) + index += 1 end end AcceptList.sort! list -- cgit v1.2.3 From 349f187f58491f2c2ef929ec0b46d78d4e888e85 Mon Sep 17 00:00:00 2001 From: Tawan Sierek Date: Fri, 29 Jan 2016 21:01:33 +0100 Subject: Add additional documentation on Headers#[] [ci skip] Issue #16519 covers confusion potentially caused by how HTTP headers, that contain underscores in their names, are retrieved through `ActionDispatch::Http::Headers#[]`. This confusion has its origin in how a CGI maps HTTP header names to variable names. Even though underscores in header names are rarely encountered, they are valid according to RFC822 [1]. Nonetheless CGI like variable names, as requested by the Rack specfication, will only contain underscores and therefore the original header name cannot be recovered after the Rack server passed on the environemnt hash. Please, see also the disscussion on StackOverflow [2], which also links to an explaination in the nginx documentation [3]. [1] http://www.ietf.org/rfc/rfc822.txt [2] http://stackoverflow.com/questions/22856136/why-underscores-are-forbidden-in-http-header-names [3] https://www.nginx.com/resources/wiki/start/topics/tutorials/config_pitfalls/#missing-disappearing-http-headers --- actionpack/lib/action_dispatch/http/headers.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 12f81dc1a5..8e899174c6 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -2,9 +2,23 @@ module ActionDispatch module Http # Provides access to the request's HTTP headers from the environment. # - # env = { "CONTENT_TYPE" => "text/plain" } + # env = { "CONTENT_TYPE" => "text/plain", "HTTP_USER_AGENT" => "curl/7.43.0" } # headers = ActionDispatch::Http::Headers.new(env) # headers["Content-Type"] # => "text/plain" + # headers["User-Agent"] # => "curl/7/43/0" + # + # Also note that when headers are mapped to CGI-like variables by the Rack + # server, both dashes and underscores are converted to underscores. This + # ambiguity cannot be resolved at this stage anymore. Both underscores and + # dashes have to be interpreted as if they were originally sent as dashes. + # + # # GET / HTTP/1.1 + # # ... + # # User-Agent: curl/7.43.0 + # # X_Custom_Header: token + # + # headers["X_Custom_Header"] # => nil + # headers["X-Custom-Header"] # => "token" class Headers CGI_VARIABLES = Set.new(%W[ AUTH_TYPE -- cgit v1.2.3 From 647a04cdbcb4e3abccb9487cdbc7366b4d923ea4 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sat, 30 Jan 2016 13:40:12 +0900 Subject: remove unused variable from render test This removes the following warning. ``` rails/actionpack/test/controller/render_test.rb:278: warning: assigned but unused variable - response ``` --- actionpack/test/controller/render_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 9430ab8db8..c814d4ea54 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -275,7 +275,7 @@ class ExpiresInRenderTest < ActionController::TestCase file.write "secrets!" file.flush assert_raises ActionView::MissingTemplate do - response = get :dynamic_render, params: { id: file.path } + get :dynamic_render, params: { id: file.path } end ensure file.close -- cgit v1.2.3 From a027a750d0a3790965de9faa78c3eeb7f79f517e Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sat, 30 Jan 2016 13:50:22 +0900 Subject: remove unused require `with_indifferent_access` had been used in `assigns` method, but has been removed in ca83436. --- actionpack/lib/action_dispatch/testing/test_process.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index eca0439909..1ecd7d14a7 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -1,6 +1,5 @@ require 'action_dispatch/middleware/cookies' require 'action_dispatch/middleware/flash' -require 'active_support/core_ext/hash/indifferent_access' module ActionDispatch module TestProcess -- cgit v1.2.3 From c4d85dfbc71043e2a746acd310e32f4f04db801a Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 26 Jan 2016 19:15:09 -0500 Subject: Handle response_body= when body is nil There are some cases when the `body` in `response_body=` can be set to nil. One of those cases is in `actionpack-action_caching` which I found while upgrading it for Rails 5. It's not possible to run `body.each` on a `nil` body so we have to return after we run `response.reset_body!`. --- actionpack/lib/action_controller/metal.rb | 1 + actionpack/test/controller/new_base/bare_metal_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index f6a93a8940..1641d01c30 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -174,6 +174,7 @@ module ActionController def response_body=(body) 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 diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index c226fa57ee..ee3c498b1c 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -40,6 +40,22 @@ module BareMetalTest end end + class BareEmptyController < ActionController::Metal + def index + self.response_body = nil + end + end + + class BareEmptyTest < ActiveSupport::TestCase + test "response body is nil" do + controller = BareEmptyController.new + controller.set_request!(ActionDispatch::Request.empty) + controller.set_response!(BareController.make_response!(controller.request)) + controller.index + assert_equal nil, controller.response_body + end + end + class HeadController < ActionController::Metal include ActionController::Head -- cgit v1.2.3 From d6f2000a67cc63aa67414c75ce77de671824ec52 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Mon, 1 Feb 2016 04:31:03 +1030 Subject: Wrangle the asset build into something that sounds more general --- actionpack/Rakefile | 3 +++ 1 file changed, 3 insertions(+) (limited to 'actionpack') diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 601263bfac..37a269cffd 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -5,6 +5,9 @@ test_files = Dir.glob('test/**/*_test.rb') desc "Default Task" task :default => :test +task :package +task "package:clean" + # Run the unit tests Rake::TestTask.new do |t| t.libs << 'test' -- cgit v1.2.3 From c3639458af5e307191ff9d5674f8144caf6a2833 Mon Sep 17 00:00:00 2001 From: Pierre Schambacher Date: Mon, 1 Feb 2016 11:28:25 +0000 Subject: Duplicate assert_generates options before modifying it --- actionpack/lib/action_dispatch/testing/assertions/routing.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 78ef860548..e7af27463c 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -86,6 +86,7 @@ module ActionDispatch end # Load routes.rb if it hasn't been loaded. + options = options.clone generated_path, query_string_keys = @routes.generate_extras(options, defaults) found_extras = options.reject { |k, _| ! query_string_keys.include? k } -- cgit v1.2.3 From b3427c662e337c2a3aa8d944b0f012641e67a4bd Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Wed, 20 Jan 2016 19:58:25 -0500 Subject: Add documentation for #17573 Fixes some parts of #23148. [ci skip] --- actionpack/lib/action_dispatch/http/cache.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 0f7898a3f8..4bd727c14e 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -80,6 +80,14 @@ module ActionDispatch set_header DATE, utc_time.httpdate end + # This method allows you to set the ETag for cached content, which + # will be returned to the end user. + # + # By default, Action Dispatch sets all ETags to be weak. + # This ensures that if the content changes only semantically, + # the whole page doesn't have to be regenerated from scratch + # by the web server. With strong ETags, pages are compared + # byte by byte, and are regenerated only if they are not exactly equal. def etag=(etag) key = ActiveSupport::Cache.expand_cache_key(etag) super %(W/"#{Digest::MD5.hexdigest(key)}") -- cgit v1.2.3 From 8a436fdd98c63cc0a7a6d2c642c18d33421dc6ad Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 25 Jan 2016 01:32:44 +0530 Subject: Add options for rake routes task Add two options: `-c` and `-g`. `-g` option returns the urls name, verb and path fields that match the pattern. `-c` option returns the urls for specific controller. Fixes #18902, and Fixes #20420 [Anton Davydov & Vipul A M] --- actionpack/CHANGELOG.md | 10 +++++++ actionpack/lib/action_dispatch/routing.rb | 3 +- .../lib/action_dispatch/routing/inspector.rb | 34 ++++++++++++---------- actionpack/test/dispatch/routing/inspector_test.rb | 34 +++++++++++++++++----- 4 files changed, 58 insertions(+), 23 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index f6ffe45490..b2d9288eca 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options + to `bin/rake routes`. These options return the url `name`, `verb` and + `path` field that match the pattern or match a specific controller. + + Deprecate `CONTROLLER` env variable in `bin/rake routes`. + + See #18902. + + *Anton Davydov* & *Vipul A M* + * Response etags to always be weak: Prefixes 'W/' to value returned by `ActionDispatch::Http::Cache::Response#etag=`, such that etags set in `fresh_when` and `stale?` are weak. diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index d00b2c3eb5..6cde5b2900 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -239,7 +239,8 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with CONTROLLER=x. + # Target specific controllers by prefixing the command with --controller option + # - or its -c shorthand. # module Routing extend ActiveSupport::Autoload diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 69e6dd5215..1ca2a3b683 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -60,12 +60,11 @@ module ActionDispatch end def format(formatter, filter = nil) - routes_to_display = filter_routes(filter) - + filter_options = normalize_filter(filter) + routes_to_display = filter_routes(filter_options) routes = collect_routes(routes_to_display) - if routes.none? - formatter.no_routes(collect_routes(@routes), filter) + formatter.no_routes(collect_routes(@routes)) return formatter.result end @@ -82,10 +81,21 @@ module ActionDispatch private - def filter_routes(filter) - if filter - filter_name = filter.underscore.sub(/_controller$/, '') - @routes.select { |route| route.defaults[:controller] == filter_name } + def normalize_filter(filter) + if filter.is_a?(Hash) && filter[:controller] + {controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/} + elsif filter.is_a?(String) + {controller: /#{filter}/, action: /#{filter}/} + else + nil + end + end + + def filter_routes(filter_options) + if filter_options + @routes.select do |route| + filter_options.any? { |default, filter| route.defaults[default] =~ filter } + end else @routes end @@ -137,7 +147,7 @@ module ActionDispatch @buffer << draw_header(routes) end - def no_routes(routes, filter) + def no_routes(routes) @buffer << if routes.none? <<-MESSAGE.strip_heredoc @@ -145,8 +155,6 @@ module ActionDispatch Please add some routes in config/routes.rb. MESSAGE - elsif missing_controller?(filter) - "The controller #{filter} does not exist!" else "No routes were found for this controller" end @@ -154,10 +162,6 @@ module ActionDispatch end private - def missing_controller?(controller_name) - [ controller_name.camelize, "#{controller_name.camelize}Controller" ].none?(&:safe_constantize) - end - def draw_section(routes) header_lengths = ['Prefix', 'Verb', 'URI Pattern'].map(&:length) name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 7382c267c7..f72a87b994 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -17,10 +17,10 @@ module ActionDispatch @set = ActionDispatch::Routing::RouteSet.new end - def draw(options = {}, &block) + def draw(options = nil, &block) @set.draw(&block) inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) - inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options[:filter]).split("\n") + inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options).split("\n") end def test_displaying_routes_for_engines @@ -297,7 +297,7 @@ module ActionDispatch end def test_routes_can_be_filtered - output = draw(filter: 'posts') do + output = draw('posts') do resources :articles resources :posts end @@ -313,6 +313,26 @@ module ActionDispatch " DELETE /posts/:id(.:format) posts#destroy"], output end + def test_routes_can_be_filtered_with_namespaced_controllers + output = draw('admin/posts') do + resources :articles + namespace :admin do + resources :posts + end + end + + assert_equal [" Prefix Verb URI Pattern Controller#Action", + " admin_posts GET /admin/posts(.:format) admin/posts#index", + " POST /admin/posts(.:format) admin/posts#create", + " new_admin_post GET /admin/posts/new(.:format) admin/posts#new", + "edit_admin_post GET /admin/posts/:id/edit(.:format) admin/posts#edit", + " admin_post GET /admin/posts/:id(.:format) admin/posts#show", + " PATCH /admin/posts/:id(.:format) admin/posts#update", + " PUT /admin/posts/:id(.:format) admin/posts#update", + " DELETE /admin/posts/:id(.:format) admin/posts#destroy"], output + end + + def test_regression_route_with_controller_regexp output = draw do get ':controller(/:action)', controller: /api\/[^\/]+/, format: false @@ -336,18 +356,18 @@ module ActionDispatch end def test_routes_with_undefined_filter - output = draw(:filter => 'Rails::MissingController') do + output = draw(controller: 'Rails::MissingController') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end assert_equal [ - "The controller Rails::MissingController does not exist!", + "No routes were found for this controller", "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." ], output end def test_no_routes_matched_filter - output = draw(:filter => 'rails/dummy') do + output = draw('rails/dummy') do get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ end @@ -358,7 +378,7 @@ module ActionDispatch end def test_no_routes_were_defined - output = draw(:filter => 'Rails::DummyController') { } + output = draw('Rails::DummyController') {} assert_equal [ "You don't have any routes defined!", -- cgit v1.2.3 From 5966b801ced62bdfcf1c8189bc068911db90ac13 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Mon, 1 Feb 2016 22:11:52 +0100 Subject: Simplify filter normalization. Assume the filter is a string, if it wasn't a hash and isn't nil. Remove needless else and rely on Ruby's default nil return. Add spaces within hash braces. --- actionpack/lib/action_dispatch/routing/inspector.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 1ca2a3b683..115ec98ea9 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -83,11 +83,9 @@ module ActionDispatch def normalize_filter(filter) if filter.is_a?(Hash) && filter[:controller] - {controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/} - elsif filter.is_a?(String) - {controller: /#{filter}/, action: /#{filter}/} - else - nil + { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + elsif filter + { controller: /#{filter}/, action: /#{filter}/ } end end -- cgit v1.2.3 From 84c3738c146c28790eeca99c1a20b2c6ba20e548 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Mon, 1 Feb 2016 22:15:21 +0100 Subject: Converge on filter. Some places were saying filter, while others said filter_options, spare the ambiguity and use filter throughout. This inlines a needless local variable and clarifies a route filter consists of defaults and values to match against. --- actionpack/lib/action_dispatch/routing/inspector.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 115ec98ea9..b806ee015b 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -60,8 +60,7 @@ module ActionDispatch end def format(formatter, filter = nil) - filter_options = normalize_filter(filter) - routes_to_display = filter_routes(filter_options) + routes_to_display = filter_routes(normalize_filter(filter)) routes = collect_routes(routes_to_display) if routes.none? formatter.no_routes(collect_routes(@routes)) @@ -89,10 +88,10 @@ module ActionDispatch end end - def filter_routes(filter_options) - if filter_options + def filter_routes(filter) + if filter @routes.select do |route| - filter_options.any? { |default, filter| route.defaults[default] =~ filter } + filter.any? { |default, value| route.defaults[default] =~ value } end else @routes -- cgit v1.2.3 From 49f6ce63f33b7817bcbd0cdf5f8881b63f40d9c9 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 1 Feb 2016 14:27:38 -0700 Subject: Preparing for Rails 5.0.0.beta2 --- actionpack/CHANGELOG.md | 7 ++++++- actionpack/lib/action_pack/gem_version.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index b2d9288eca..2aa78d20c0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +## Rails 5.0.0.beta2 (February 01, 2016) ## + +* No changes. + + * Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options to `bin/rake routes`. These options return the url `name`, `verb` and `path` field that match the pattern or match a specific controller. @@ -7,7 +12,7 @@ See #18902. *Anton Davydov* & *Vipul A M* - + * Response etags to always be weak: Prefixes 'W/' to value returned by `ActionDispatch::Http::Cache::Response#etag=`, such that etags set in `fresh_when` and `stale?` are weak. diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 3a1410d492..778c5482d3 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta1.1" + PRE = "beta2" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end -- cgit v1.2.3 From 60b040e362086fa11f86d35938d515145241174e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 1 Feb 2016 19:57:31 -0200 Subject: Add some Action Cable CHANGELOG entries And improve changelongs. [ci skip] --- actionpack/CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 2aa78d20c0..710751ff1b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,8 +1,5 @@ ## Rails 5.0.0.beta2 (February 01, 2016) ## -* No changes. - - * Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options to `bin/rake routes`. These options return the url `name`, `verb` and `path` field that match the pattern or match a specific controller. -- cgit v1.2.3 From c4ac23bfa68d07b58e0ff32dfca1fee2b3283542 Mon Sep 17 00:00:00 2001 From: Kang-Kyu Lee Date: Mon, 1 Feb 2016 14:21:34 -0800 Subject: Update CHANGELOG.md fix indentation to show it as code --- actionpack/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 710751ff1b..809b735deb 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -183,11 +183,11 @@ * Accessing mime types via constants like `Mime::HTML` is deprecated. Please change code like this: - Mime::HTML + Mime::HTML To this: - Mime[:html] + Mime[:html] This change is so that Rails will not manage a list of constants, and fixes an issue where if a type isn't registered you could possibly get the wrong -- cgit v1.2.3 From 522099a13ffea611dfb37d4d22da62eb8cb81c12 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 2 Feb 2016 18:55:02 -0700 Subject: Sleep well, sweet prince Prototype, you have served us well. But you are no longer how we make an XMLHttpRequest. RIP --- actionpack/lib/action_dispatch/testing/integration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 711ca10419..6f51accee7 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -71,7 +71,7 @@ module ActionDispatch end # Performs an XMLHttpRequest request with the given parameters, mirroring - # a request from the Prototype library. + # an AJAX request made from JavaScript. # # The request_method is +:get+, +:post+, +:patch+, +:put+, +:delete+ or # +:head+; the parameters are +nil+, a hash, or a url-encoded or multipart -- cgit v1.2.3 From 7ca7c0ef289595d79ed1a59af9873f4fcc29918e Mon Sep 17 00:00:00 2001 From: Prathamesh Sonpatki Date: Wed, 3 Feb 2016 13:33:31 +0530 Subject: Put some space for non-assets requests in development mode - Fixes #23428. --- actionpack/lib/action_controller/log_subscriber.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 4c9f14e409..d1d6acac26 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -26,6 +26,8 @@ module ActionController end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" message << " (#{additions.join(" | ".freeze)})" unless additions.blank? + message << "\n\n" if Rails.env.development? + message end end -- cgit v1.2.3 From a640da454fdb9cd8806a1b6bd98c2da93f1b53b9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 5 Feb 2016 11:42:06 -0800 Subject: disable controller / view thread spawning in tests Tests can (and do) access the database from the main thread. In this case they were starting a transaction, then making a request. The request would create a new thread, which would allocate a new database connection. Since the main thread started a transaction that contains data that the new thread wants to see, the new thread would not see it due to data visibility from transactions. Spawning the new thread in production is fine because middleware should not be doing database manipulation similar to the test harness. Before 603fe20c it was possible to set the database connection id based on a thread local, but 603fe20c changes the connection lookup code to never look at the "connection id" but only at the thread object itself. Without that indirection, we can't force threads to use the same connection pool as another thread. Fixes #23483 --- actionpack/lib/action_controller/metal/live.rb | 15 +++++++++++++-- actionpack/lib/action_controller/test_case.rb | 10 ++++++++++ actionpack/test/controller/live_stream_test.rb | 9 ++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index e3c540bf5f..acc4507b2d 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -237,9 +237,8 @@ module ActionController # This processes the action in a child thread. It lets us return the # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client - Thread.new { + new_controller_thread { t2 = Thread.current - t2.abort_on_exception = true # Since we're processing the view in a different thread, copy the # thread locals from the main thread to the child thread. :'( @@ -270,6 +269,18 @@ module ActionController raise error if error end + # Spawn a new thread to serve up the controller in. This is to get + # around the fact that Rack isn't based around IOs and we need to use + # a thread to stream data from the response bodies. Nobody should call + # this method except in Rails internals. Seriously! + def new_controller_thread # :nodoc: + Thread.new { + t2 = Thread.current + t2.abort_on_exception = true + yield + } + end + def log_error(exception) logger = ActionController::Base.logger return unless logger diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index b43bb9dc17..ac45b2e363 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -12,6 +12,16 @@ module ActionController include Testing::Functional end + module Live + # Disable controller / rendering threads in tests. User tests can access + # the database on the main thread, so they could open a txn, then the + # controller thread will open a new connection and try to access data + # that's only visible to the main thread's txn. This is the problem in #23483 + def new_controller_thread # :nodoc: + yield + end + end + # ActionController::TestCase will be deprecated and moved to a gem in Rails 5.1. # Please use ActionDispatch::IntegrationTest going forward. class TestRequest < ActionDispatch::TestRequest #:nodoc: diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 2ef9734269..0c3884cd38 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -152,7 +152,6 @@ module ActionController def thread_locals tc.assert_equal 'aaron', Thread.current[:setting] - tc.assert_not_equal Thread.current.object_id, Thread.current[:originating_thread] response.headers['Content-Type'] = 'text/event-stream' %w{ hello world }.each do |word| @@ -261,6 +260,14 @@ module ActionController end end + def setup + super + + def @controller.new_controller_thread + Thread.new { yield } + end + end + def test_set_cookie get :set_cookie assert_equal({'hello' => 'world'}, @response.cookies) -- cgit v1.2.3 From 38b5af6595338cb2212980062d9aaf51241878cc Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 5 Feb 2016 13:43:56 -0800 Subject: add missing require --- actionpack/lib/action_dispatch/http/response.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 14f86c7c07..fa4c54701a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/module/attribute_accessors' require 'action_dispatch/http/filter_redirect' +require 'action_dispatch/http/cache' require 'monitor' module ActionDispatch # :nodoc: -- cgit v1.2.3 From 4e4bcae0841b81438ce3b6c627eebc7536566bf5 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sat, 6 Feb 2016 22:28:59 +0100 Subject: Avoid coupling Action Pack to Railties. Referencing Rails.env without checking if it's defined couples us to Railties. Fix by avoiding the line breaks if we don't have an env check to rely on. --- actionpack/lib/action_controller/log_subscriber.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index d1d6acac26..450a04779f 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -26,7 +26,7 @@ module ActionController end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" message << " (#{additions.join(" | ".freeze)})" unless additions.blank? - message << "\n\n" if Rails.env.development? + message << "\n\n" if defined?(Rails.env) && Rails.env.development? message end -- cgit v1.2.3 From 3e4a69e52d8c8f0335e0ddbb46fe21009b962334 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Sun, 7 Feb 2016 08:24:57 +1030 Subject: Hand off the interlock to the new thread in AC::Live Most importantly, the original request thread must yield its share lock while waiting for the live thread to commit -- otherwise a request's base and live threads can deadlock against each other. --- actionpack/lib/action_controller/metal/live.rb | 51 ++++++++++++++------------ 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index acc4507b2d..fc20e7a421 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -238,34 +238,39 @@ module ActionController # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client new_controller_thread { - t2 = Thread.current - - # Since we're processing the view in a different thread, copy the - # thread locals from the main thread to the child thread. :'( - locals.each { |k,v| t2[k] = v } - - begin - super(name) - rescue => e - if @_response.committed? - begin - @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html - @_response.stream.call_on_error - rescue => exception - log_error(exception) - ensure - log_error(e) - @_response.stream.close + ActiveSupport::Dependencies.interlock.running do + t2 = Thread.current + + # Since we're processing the view in a different thread, copy the + # thread locals from the main thread to the child thread. :'( + locals.each { |k,v| t2[k] = v } + + begin + super(name) + rescue => e + if @_response.committed? + begin + @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html + @_response.stream.call_on_error + rescue => exception + log_error(exception) + ensure + log_error(e) + @_response.stream.close + end + else + error = e end - else - error = e + ensure + @_response.commit! end - ensure - @_response.commit! end } - @_response.await_commit + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + @_response.await_commit + end + raise error if error end -- cgit v1.2.3 From 73d1975810cf83351f3c72e3aaa2ea43a64d08b9 Mon Sep 17 00:00:00 2001 From: Scott Bronson Date: Sat, 6 Feb 2016 18:24:10 -0800 Subject: fix 'method redefined' warnings --- actionpack/lib/action_controller/test_case.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index ac45b2e363..0c4b661214 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -17,6 +17,7 @@ module ActionController # the database on the main thread, so they could open a txn, then the # controller thread will open a new connection and try to access data # that's only visible to the main thread's txn. This is the problem in #23483 + remove_method :new_controller_thread def new_controller_thread # :nodoc: yield end -- cgit v1.2.3 From 7e35cb2987e146de29d50f97a43caef61e7588af Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 7 Feb 2016 15:34:13 +0100 Subject: Add SVG as a default mime type --- actionpack/CHANGELOG.md | 4 ++++ actionpack/lib/action_dispatch/http/mime_types.rb | 1 + 2 files changed, 5 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 809b735deb..93e598e493 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add image/svg+xml as a default mime type. + + *DHH* + ## Rails 5.0.0.beta2 (February 01, 2016) ## * Add `-g` and `-c` (short for _grep_ and _controller_ respectively) options diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 87715205d9..8356d1a238 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -14,6 +14,7 @@ Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg) Mime::Type.register "image/gif", :gif, [], %w(gif) Mime::Type.register "image/bmp", :bmp, [], %w(bmp) Mime::Type.register "image/tiff", :tiff, [], %w(tif tiff) +Mime::Type.register "image/svg+xml", :svg Mime::Type.register "video/mpeg", :mpeg, [], %w(mpg mpeg mpe) -- cgit v1.2.3 From ec82c13dd47e386de8928f3cdd24eef33b8f835b Mon Sep 17 00:00:00 2001 From: Karim El-Husseiny Date: Mon, 8 Feb 2016 17:04:31 +0200 Subject: Update rails-html-sanitizer version to v1.0.3 rails-html-sanitizer 1.0.2 is vulnerable: https://groups.google.com/d/msg/rubyonrails-security/uh--W4TDwmI/m_CVZtdbFQAJ --- actionpack/actionpack.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 28d8bc3091..f2d08dc6ca 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |s| s.add_dependency 'rack', '~> 2.x' s.add_dependency 'rack-test', '~> 0.6.3' - s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.2' + s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.3' s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5' s.add_dependency 'actionview', version -- cgit v1.2.3 From 9b5ae716db601a8a34a4227c9c4ef2ec9cf90f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 8 Feb 2016 14:09:29 -0200 Subject: Revert "Merge pull request #23562 from Azzurrio/patch-1" This reverts commit 8c3cca5e113213958469b1cec8aa9a664535251a, reversing changes made to 9dcf67c4da35b165301865d9721da1d552f7e03f. Reason: https://github.com/rails/rails/pull/23562#issuecomment-181442569 --- actionpack/actionpack.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index f2d08dc6ca..28d8bc3091 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |s| s.add_dependency 'rack', '~> 2.x' s.add_dependency 'rack-test', '~> 0.6.3' - s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.3' + s.add_dependency 'rails-html-sanitizer', '~> 1.0', '>= 1.0.2' s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5' s.add_dependency 'actionview', version -- cgit v1.2.3 From 02c3867882d6d23b10df262a6db5f937ca69fb53 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 8 Feb 2016 15:40:10 -0800 Subject: speed up string xor operation and reduce object allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` [aaron@TC rails (master)]$ cat xor.rb a = "\x14b\"\xB4P8\x05\x8D\xC74\xC3\xEC}\xFDf\x8E!h\xCF^\xBF\xA5%\xC6\xF0\xA9\xF9x\x04\xFA\xF1\x82" b = "O.\xF7\x01\xA9D\xA3\xE1D\x7FU\x85\xFC\x8Ak\e\x04\x8A\x97\x91\xD01\x02\xA4G\x1EIf:Y\x0F@" def xor_byte_strings(s1, s2) s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') end def xor_byte_strings2(s1, s2) s2_bytes = s2.bytes s1.bytes.map.with_index { |c1, i| c1 ^ s2_bytes[i] }.pack('c*') end require 'benchmark/ips' require 'allocation_tracer' Benchmark.ips do |x| x.report 'xor_byte_strings' do xor_byte_strings a, b end x.report 'xor_byte_strings2' do xor_byte_strings2 a, b end end ObjectSpace::AllocationTracer.setup(%i{type}) result = ObjectSpace::AllocationTracer.trace do xor_byte_strings a, b end p :xor_byte_strings => result ObjectSpace::AllocationTracer.clear result = ObjectSpace::AllocationTracer.trace do xor_byte_strings2 a, b end p :xor_byte_strings2 => result [aaron@TC rails (master)]$ ruby -I~/git/allocation_tracer/lib xor.rb Calculating ------------------------------------- xor_byte_strings 10.087k i/100ms xor_byte_strings2 11.339k i/100ms ------------------------------------------------- xor_byte_strings 108.386k (± 5.8%) i/s - 544.698k xor_byte_strings2 122.239k (± 3.0%) i/s - 612.306k {:xor_byte_strings=>{[:T_ARRAY]=>[38, 0, 0, 0, 0, 0], [:T_STRING]=>[2, 0, 0, 0, 0, 0]}} {:xor_byte_strings2=>{[:T_ARRAY]=>[3, 0, 0, 0, 0, 0], [:T_DATA]=>[1, 0, 0, 0, 0, 0], [:T_IMEMO]=>[2, 0, 0, 0, 0, 0], [:T_STRING]=>[2, 0, 0, 0, 0, 0]}} ``` --- actionpack/lib/action_controller/metal/request_forgery_protection.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 91b3403ad5..6586985ff5 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -378,7 +378,8 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) - s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') + s2_bytes = s2.bytes + s1.bytes.map.with_index { |c1, i| c1 ^ s2_bytes[i] }.pack('c*') end # The form's authenticity parameter. Override to provide your own. -- cgit v1.2.3 From d5c6babd9e139deed5d43c35a41b2af5d73376ba Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 9 Feb 2016 12:12:05 -0800 Subject: AC::Request#format always returns a value, so we do not need to try --- actionpack/lib/action_controller/metal/instrumentation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 3dbf34eb2a..f66e3efffe 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,7 +19,7 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :format => request.format.ref, :method => request.request_method, :path => (request.fullpath rescue "unknown") } -- cgit v1.2.3 From b2a9047558e20e35681eae8d47ad7a95c3e1d19e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 9 Feb 2016 13:34:44 -0800 Subject: Request#fullpath should not raise an exception, so remove the rescue --- actionpack/lib/action_controller/metal/instrumentation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index f66e3efffe..bf74b39ac4 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -21,7 +21,7 @@ module ActionController :params => request.filtered_parameters, :format => request.format.ref, :method => request.request_method, - :path => (request.fullpath rescue "unknown") + :path => request.fullpath } ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) -- cgit v1.2.3 From b5eb2423b6e431ba53e3836d58449e7e810096b4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 9 Feb 2016 13:48:03 -0800 Subject: `log_process_action` will return an array, so use `empty?` We don't need to use active support in this case because we know the type that will be returned. --- actionpack/lib/action_controller/log_subscriber.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 450a04779f..a0917b4fdb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -25,7 +25,7 @@ module ActionController status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" - message << " (#{additions.join(" | ".freeze)})" unless additions.blank? + message << " (#{additions.join(" | ".freeze)})" unless additions.empty? message << "\n\n" if defined?(Rails.env) && Rails.env.development? message -- cgit v1.2.3