From f52e17f1c3b59f4301a84da5ed4f46a77363f29d Mon Sep 17 00:00:00 2001 From: Ashley Ellis Pierce Date: Mon, 15 Jan 2018 14:21:48 -0500 Subject: Move browser checking to its own class --- actionpack/lib/action_dispatch/system_test_case.rb | 1 + .../lib/action_dispatch/system_testing/browser.rb | 49 ++++++++++++++++++++++ .../lib/action_dispatch/system_testing/driver.rb | 29 ++----------- 3 files changed, 53 insertions(+), 26 deletions(-) create mode 100644 actionpack/lib/action_dispatch/system_testing/browser.rb (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 99d0c06751..81ddbc3567 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -6,6 +6,7 @@ require "capybara/dsl" require "capybara/minitest" require "action_controller" require "action_dispatch/system_testing/driver" +require "action_dispatch/system_testing/browser" require "action_dispatch/system_testing/server" require "action_dispatch/system_testing/test_helpers/screenshot_helper" require "action_dispatch/system_testing/test_helpers/setup_and_teardown" diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb new file mode 100644 index 0000000000..10e6888ab3 --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ActionDispatch + module SystemTesting + class Browser # :nodoc: + attr_reader :name + + def initialize(name) + @name = name + end + + def type + case name + when :headless_chrome + :chrome + when :headless_firefox + :firefox + else + name + end + end + + def options + case name + when :headless_chrome + headless_chrome_browser_options + when :headless_firefox + headless_firefox_browser_options + end + end + + private + def headless_chrome_browser_options + options = Selenium::WebDriver::Chrome::Options.new + options.args << "--headless" + options.args << "--disable-gpu" + + options + end + + def headless_firefox_browser_options + options = Selenium::WebDriver::Firefox::Options.new + options.args << "-headless" + + options + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 280989a146..5252ff6746 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -5,7 +5,7 @@ module ActionDispatch class Driver # :nodoc: def initialize(name, **options) @name = name - @browser = options[:using] + @browser = Browser.new(options[:using]) @screen_size = options[:screen_size] @options = options[:options] end @@ -32,34 +32,11 @@ module ActionDispatch end def browser_options - if @browser == :headless_chrome - browser_options = Selenium::WebDriver::Chrome::Options.new - browser_options.args << "--headless" - browser_options.args << "--disable-gpu" - - @options.merge(options: browser_options) - elsif @browser == :headless_firefox - browser_options = Selenium::WebDriver::Firefox::Options.new - browser_options.args << "-headless" - - @options.merge(options: browser_options) - else - @options - end - end - - def browser - if @browser == :headless_chrome - :chrome - elsif @browser == :headless_firefox - :firefox - else - @browser - end + @options.merge(options: @browser.options).compact end def register_selenium(app) - Capybara::Selenium::Driver.new(app, { browser: browser }.merge(browser_options)).tap do |driver| + Capybara::Selenium::Driver.new(app, { browser: @browser.type }.merge(browser_options)).tap do |driver| driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) end end -- cgit v1.2.3 From 5ac6ec54a673e493cddf6bf2eff5b98e52b3c268 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 18 Jan 2018 12:09:16 +0900 Subject: Enable autocorrect for `Lint/EndAlignment` cop ### Summary This PR changes .rubocop.yml. Regarding the code using `if ... else ... end`, I think the coding style that Rails expects is as follows. ```ruby var = if cond a else b end ``` However, the current .rubocop.yml setting does not offense for the following code. ```ruby var = if cond a else b end ``` I think that the above code expects offense to be warned. Moreover, the layout by autocorrect is unnatural. ```ruby var = if cond a else b end ``` This PR adds a setting to .rubocop.yml to make an offense warning and autocorrect as expected by the coding style. And this change also fixes `case ... when ... end` together. Also this PR itself is an example that arranges the layout using `rubocop -a`. ### Other Information Autocorrect of `Lint/EndAlignment` cop is `false` by default. https://github.com/bbatsov/rubocop/blob/v0.51.0/config/default.yml#L1443 This PR changes this value to `true`. Also this PR has changed it together as it is necessary to enable `Layout/ElseAlignment` cop to make this behavior. --- actionpack/lib/action_dispatch/http/url.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index f0344fd927..35ba44005a 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -274,7 +274,7 @@ module ActionDispatch def standard_port case protocol when "https://" then 443 - else 80 + else 80 end end -- cgit v1.2.3 From 403d0d8f9e69029dcfe5313d07dff7705141849e Mon Sep 17 00:00:00 2001 From: James Lovejoy Date: Fri, 19 Jan 2018 17:56:00 -0800 Subject: Fix typos. Improve text_helper documentation. [ci skip] --- actionpack/lib/action_dispatch/routing/mapper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d87a23a58c..31eb6104fe 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1573,7 +1573,7 @@ module ActionDispatch # Matches a URL pattern to one or more routes. # For more information, see match[rdoc-ref:Base#match]. # - # match 'path' => 'controller#action', via: patch + # match 'path' => 'controller#action', via: :patch # match 'path', to: 'controller#action', via: :post # match 'path', 'otherpath', on: :member, via: :get def match(path, *rest, &block) @@ -2082,9 +2082,9 @@ module ActionDispatch # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # - # In this instance the +params+ object comes from the context in which the the + # In this instance the +params+ object comes from the context in which the # block is executed, e.g. generating a URL inside a controller action or a view. - # If the block is executed where there isn't a params object such as this: + # If the block is executed where there isn't a +params+ object such as this: # # Rails.application.routes.url_helpers.browse_path # -- cgit v1.2.3 From fbf4e9562db9ba73428b9b397361aa886bc2c8e8 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Fri, 5 Jan 2018 18:24:39 +0200 Subject: Remove code duplication for `ActionController::Metal.action` --- actionpack/lib/action_controller/metal.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 457884ea08..f875aa5e6b 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -230,18 +230,16 @@ module ActionController # Returns a Rack endpoint for the given action name. def self.action(name) + app = lambda { |env| + req = ActionDispatch::Request.new(env) + res = make_response! req + new.dispatch(name, req, res) + } + if middleware_stack.any? - middleware_stack.build(name) do |env| - req = ActionDispatch::Request.new(env) - res = make_response! req - new.dispatch(name, req, res) - end + middleware_stack.build(name, app) else - lambda { |env| - req = ActionDispatch::Request.new(env) - res = make_response! req - new.dispatch(name, req, res) - } + app end end -- cgit v1.2.3 From c8bf6da46590bcf6bb4253bc4ba069a442c6b776 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Mon, 29 Jan 2018 10:43:30 -0800 Subject: ActionController::TestCase: fix #post documentation [ci skip] Fixes #31823. --- actionpack/lib/action_controller/test_case.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 4b408750a4..798d142755 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -256,7 +256,7 @@ module ActionController # # def test_create # json = {book: { title: "Love Hina" }}.to_json - # post :create, json + # post :create, body: json # end # # == Special instance variables -- cgit v1.2.3 From eea28f4103f0a55e50ce750582317110c988afcd Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Sun, 28 Jan 2018 12:53:11 -0500 Subject: Allow @ in X-Request-Id header It makes sense to be as strict as possible with headers from the outside world, but allowing @ to support Apache's mod_unique_id (see #31644) seems OK to me --- actionpack/lib/action_dispatch/middleware/request_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index 805d3f2148..da2871b551 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -30,7 +30,7 @@ module ActionDispatch private def make_request_id(request_id) if request_id.presence - request_id.gsub(/[^\w\-]/, "".freeze).first(255) + request_id.gsub(/[^\w\-@]/, "".freeze).first(255) else internal_request_id end -- cgit v1.2.3 From 1c383df324fdf0b68b3f54a649eb7d2a4f55bcb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 30 Jan 2018 18:51:17 -0500 Subject: Start Rails 6.0 development!!! :tada::tada::tada: --- actionpack/lib/action_pack/gem_version.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 97f4934b58..37969fcb57 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -7,10 +7,10 @@ module ActionPack end module VERSION - MAJOR = 5 - MINOR = 2 + MAJOR = 6 + MINOR = 0 TINY = 0 - PRE = "beta2" + PRE = "alpha" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end -- cgit v1.2.3 From 2587cadd4223f4abc1a94fdfb14cfd8c8d60e28b Mon Sep 17 00:00:00 2001 From: Igor Kasyanchuk Date: Wed, 31 Jan 2018 13:36:01 -0800 Subject: Consistent behavior for session and cookies with to_h and to_hash method --- actionpack/lib/action_dispatch/middleware/cookies.rb | 3 +++ actionpack/lib/action_dispatch/request/session.rb | 1 + 2 files changed, 4 insertions(+) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index ea4156c972..c45d947904 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -338,6 +338,9 @@ module ActionDispatch end alias :has_key? :key? + # Returns the cookies as Hash. + alias :to_hash :to_h + def update(other_hash) @cookies.update other_hash.stringify_keys self diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index d86d0b10c2..000847e193 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -130,6 +130,7 @@ module ActionDispatch load_for_read! @delegate.dup.delete_if { |_, v| v.nil? } end + alias :to_h :to_hash # Updates the session with given Hash. # -- cgit v1.2.3 From c92ea62792083af8130d9d24f70b9c8ea7badb0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 9 Feb 2018 13:51:20 -0500 Subject: Make sure assert_recognizes can still find routes mounted after engines Before, if the application defined after an engine this method would not recognize the route since it was not defined insdie the engine. --- actionpack/lib/action_dispatch/routing/route_set.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 9eff30fa53..d9d0c31165 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -855,7 +855,7 @@ module ActionDispatch recognize_path_with_request(req, path, extras) end - def recognize_path_with_request(req, path, extras) + def recognize_path_with_request(req, path, extras, raise_on_missing: true) @router.recognize(req) do |route, params| params.merge!(extras) params.each do |key, value| @@ -875,12 +875,14 @@ module ActionDispatch return req.path_parameters elsif app.matches?(req) && app.engine? - path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras) - return path_parameters + path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras, raise_on_missing: false) + return path_parameters if path_parameters end end - raise ActionController::RoutingError, "No route matches #{path.inspect}" + if raise_on_missing + raise ActionController::RoutingError, "No route matches #{path.inspect}" + end end end # :startdoc: -- cgit v1.2.3 From d0192e0c2daa03048a8c4d0d3b94763dbbfec4c1 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sun, 11 Feb 2018 02:19:41 +0900 Subject: Unused core_ext --- actionpack/lib/action_dispatch/routing/route_set.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index d9d0c31165..ff6998ae31 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -2,7 +2,6 @@ require "action_dispatch/journey" require "active_support/core_ext/object/to_query" -require "active_support/core_ext/hash/slice" require "active_support/core_ext/module/redefine_method" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/array/extract_options" -- cgit v1.2.3 From 24131d4a6a97621c1021231c650793ce29c4ed99 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 15 Feb 2018 15:03:20 +1100 Subject: PERF: dedupe scanned route fragments Per: https://bugs.ruby-lang.org/issues/13077 String @- will dedupe strings. This takes advantage of this by deduping route fragments that are full of duplication usually. For Discourse: Before: Total allocated: 207574305 bytes (2214916 objects) Total retained: 36470010 bytes (322194 objects) After Total allocated: 207556847 bytes (2214711 objects) Total retained: 36327973 bytes (318627 objects) <- object that GC can not collect So we save 3500 or so RVALUES this way, not the largest saving in the world, but worth it especially for large route files. --- actionpack/lib/action_dispatch/journey/scanner.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/scanner.rb b/actionpack/lib/action_dispatch/journey/scanner.rb index 4ae77903fa..5ed587c1b1 100644 --- a/actionpack/lib/action_dispatch/journey/scanner.rb +++ b/actionpack/lib/action_dispatch/journey/scanner.rb @@ -33,6 +33,13 @@ module ActionDispatch end private + + # takes advantage of String @- deduping capabilities in Ruby 2.5 upwards + # see: https://bugs.ruby-lang.org/issues/13077 + def dedup_scan(regex) + r = @ss.scan(regex) + r ? -r : nil + end def scan case @@ -47,15 +54,15 @@ module ActionDispatch [:OR, "|"] when @ss.skip(/\./) [:DOT, "."] - when text = @ss.scan(/:\w+/) + when text = dedup_scan(/:\w+/) [:SYMBOL, text] - when text = @ss.scan(/\*\w+/) + when text = dedup_scan(/\*\w+/) [:STAR, text] - when text = @ss.scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) + when text = dedup_scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) text.tr! "\\", "" [:LITERAL, text] # any char - when text = @ss.scan(/./) + when text = dedup_scan(/./) [:LITERAL, text] end end -- cgit v1.2.3 From f282f3758d31e8445d0854e2ae7a67f17cede3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 15 Feb 2018 15:43:29 -0500 Subject: Revert "Merge pull request #31999 from SamSaffron/patch-1" This reverts commit 9f65d2a08bc80a94bbb2c0b6e00957c7059aed25, reversing changes made to 966843732a607864b077b72b2a17168d4e3548cc. This broken a lot of tests. --- actionpack/lib/action_dispatch/journey/scanner.rb | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/scanner.rb b/actionpack/lib/action_dispatch/journey/scanner.rb index 5ed587c1b1..4ae77903fa 100644 --- a/actionpack/lib/action_dispatch/journey/scanner.rb +++ b/actionpack/lib/action_dispatch/journey/scanner.rb @@ -33,13 +33,6 @@ module ActionDispatch end private - - # takes advantage of String @- deduping capabilities in Ruby 2.5 upwards - # see: https://bugs.ruby-lang.org/issues/13077 - def dedup_scan(regex) - r = @ss.scan(regex) - r ? -r : nil - end def scan case @@ -54,15 +47,15 @@ module ActionDispatch [:OR, "|"] when @ss.skip(/\./) [:DOT, "."] - when text = dedup_scan(/:\w+/) + when text = @ss.scan(/:\w+/) [:SYMBOL, text] - when text = dedup_scan(/\*\w+/) + when text = @ss.scan(/\*\w+/) [:STAR, text] - when text = dedup_scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) + when text = @ss.scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) text.tr! "\\", "" [:LITERAL, text] # any char - when text = dedup_scan(/./) + when text = @ss.scan(/./) [:LITERAL, text] end end -- cgit v1.2.3 From 6781bd966df7f9cb89ed0beef679bf8feaf81610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 15 Feb 2018 17:34:18 -0500 Subject: Revert "Revert "Merge pull request #31999 from SamSaffron/patch-1"" This reverts commit f282f3758d31e8445d0854e2ae7a67f17cede3bc. --- actionpack/lib/action_dispatch/journey/scanner.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/scanner.rb b/actionpack/lib/action_dispatch/journey/scanner.rb index 4ae77903fa..5ed587c1b1 100644 --- a/actionpack/lib/action_dispatch/journey/scanner.rb +++ b/actionpack/lib/action_dispatch/journey/scanner.rb @@ -33,6 +33,13 @@ module ActionDispatch end private + + # takes advantage of String @- deduping capabilities in Ruby 2.5 upwards + # see: https://bugs.ruby-lang.org/issues/13077 + def dedup_scan(regex) + r = @ss.scan(regex) + r ? -r : nil + end def scan case @@ -47,15 +54,15 @@ module ActionDispatch [:OR, "|"] when @ss.skip(/\./) [:DOT, "."] - when text = @ss.scan(/:\w+/) + when text = dedup_scan(/:\w+/) [:SYMBOL, text] - when text = @ss.scan(/\*\w+/) + when text = dedup_scan(/\*\w+/) [:STAR, text] - when text = @ss.scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) + when text = dedup_scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) text.tr! "\\", "" [:LITERAL, text] # any char - when text = @ss.scan(/./) + when text = dedup_scan(/./) [:LITERAL, text] end end -- cgit v1.2.3 From 23c5558f37c2c55807e7603415214f2b4b7b22c1 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 16 Feb 2018 09:19:57 +1100 Subject: correct the dedup code --- actionpack/lib/action_dispatch/journey/scanner.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/scanner.rb b/actionpack/lib/action_dispatch/journey/scanner.rb index 5ed587c1b1..2a075862e9 100644 --- a/actionpack/lib/action_dispatch/journey/scanner.rb +++ b/actionpack/lib/action_dispatch/journey/scanner.rb @@ -33,7 +33,7 @@ module ActionDispatch end private - + # takes advantage of String @- deduping capabilities in Ruby 2.5 upwards # see: https://bugs.ruby-lang.org/issues/13077 def dedup_scan(regex) @@ -58,9 +58,9 @@ module ActionDispatch [:SYMBOL, text] when text = dedup_scan(/\*\w+/) [:STAR, text] - when text = dedup_scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) + when text = @ss.scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) text.tr! "\\", "" - [:LITERAL, text] + [:LITERAL, -text] # any char when text = dedup_scan(/./) [:LITERAL, text] -- cgit v1.2.3 From 98d4fc3e82ebfd535b05b7f6e3abb4b03595c2dd Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 16 Feb 2018 13:32:31 +1100 Subject: PERF: reduce retained objects in Journey Before: Total allocated: 209050523 bytes (2219202 objects) Total retained: 36580305 bytes (323462 objects) After: Total allocated: 209180253 bytes (2222455 objects) Total retained: 36515599 bytes (321850 objects) --- Modest saving of 1612 RVALUEs in the heap on Discourse boot The larger the route file the better the results. Saving will only be visible on Ruby 2.5 and up. --- actionpack/lib/action_dispatch/journey/nodes/node.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 08b931a3cd..6f12052713 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -32,7 +32,7 @@ module ActionDispatch end def name - left.tr "*:".freeze, "".freeze + -(left.tr "*:", "") end def type @@ -82,7 +82,7 @@ module ActionDispatch def initialize(left) super @regexp = DEFAULT_EXP - @name = left.tr "*:".freeze, "".freeze + @name = -(left.tr "*:", "") end def default_regexp? -- cgit v1.2.3 From 2c89d1dda4077b2a99ddcced59bdd7a9a21b39a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 16 Feb 2018 18:55:36 -0500 Subject: Write the code in a more natural way. --- actionpack/lib/action_dispatch/journey/nodes/node.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 6f12052713..32f632800c 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -32,7 +32,7 @@ module ActionDispatch end def name - -(left.tr "*:", "") + -left.tr("*:", "") end def type @@ -82,7 +82,7 @@ module ActionDispatch def initialize(left) super @regexp = DEFAULT_EXP - @name = -(left.tr "*:", "") + @name = -left.tr("*:", "") end def default_regexp? -- cgit v1.2.3 From 89bcca59e91fa9da941de890012872e8288e77b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 16 Feb 2018 19:28:30 -0500 Subject: Remove usage of strip_heredoc in the framework in favor of <<~ Some places we can't remove because Ruby still don't have a method equivalent to strip_heredoc to be called in an already existent string. --- .../lib/action_controller/metal/request_forgery_protection.rb | 3 +-- actionpack/lib/action_dispatch/routing/inspector.rb | 9 ++++----- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 0ab313e398..94092de96c 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -3,7 +3,6 @@ require "rack/session/abstract/id" require "action_controller/metal/exceptions" require "active_support/security_utils" -require "active_support/core_ext/string/strip" module ActionController #:nodoc: class InvalidAuthenticityToken < ActionControllerError #:nodoc: @@ -416,7 +415,7 @@ module ActionController #:nodoc: allow_forgery_protection end - NULL_ORIGIN_MESSAGE = <<-MSG.strip_heredoc + NULL_ORIGIN_MESSAGE = <<~MSG The browser returned a 'null' origin for a request with origin-based forgery protection turned on. This usually means you have the 'no-referrer' Referrer-Policy header enabled, or that you the request came from a site that refused to give its origin. This makes it impossible for Rails to verify the source of the requests. Likely the diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index a2205569b4..22336c59b6 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "delegate" -require "active_support/core_ext/string/strip" module ActionDispatch module Routing @@ -150,10 +149,10 @@ module ActionDispatch def no_routes(routes) @buffer << if routes.none? - <<-MESSAGE.strip_heredoc - You don't have any routes defined! + <<~MESSAGE + You don't have any routes defined! - Please add some routes in config/routes.rb. + Please add some routes in config/routes.rb. MESSAGE else "No routes were found for this controller" @@ -203,7 +202,7 @@ module ActionDispatch end def no_routes(*) - @buffer << <<-MESSAGE.strip_heredoc + @buffer << <<~MESSAGE

You don't have any routes defined!

  • Please add some routes in config/routes.rb.
    • diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 31eb6104fe..f3970d5445 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -611,7 +611,7 @@ module ActionDispatch end raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call) - raise ArgumentError, <<-MSG.strip_heredoc unless path + raise ArgumentError, <<~MSG unless path Must be called with mount point mount SomeRackApp, at: "some_route" -- cgit v1.2.3 From 94a27cb2b5c9b3db8e72d4cbef00ff040b30681d Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sat, 17 Feb 2018 01:26:36 +0200 Subject: Fix array routing constraints --- actionpack/lib/action_dispatch/journey/path/pattern.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 2d85a89a56..537f479ee5 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -90,7 +90,7 @@ module ActionDispatch return @separator_re unless @matchers.key?(node) re = @matchers[node] - "(#{re})" + "(#{Regexp.union(re)})" end def visit_GROUP(node) @@ -183,7 +183,7 @@ module ActionDispatch node = node.to_sym if @requirements.key?(node) - re = /#{@requirements[node]}|/ + re = /#{Regexp.union(@requirements[node])}|/ @offsets.push((re.match("").length - 1) + @offsets.last) else @offsets << @offsets.last -- cgit v1.2.3 From 1e526788e6b1d3f42f4d8fdca20e588d42838c80 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 16 Feb 2018 17:14:27 -0800 Subject: Rails 6 requires Ruby 2.3+ --- actionpack/lib/action_dispatch/http/mime_type.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index d2b2106845..1d3d241f66 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -279,13 +279,8 @@ module Mime def all?; false; end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. - protected - - attr_reader :string, :synonyms - private + attr_reader :string, :synonyms def to_ary; end def to_a; end -- cgit v1.2.3 From 56278a7a1e2efcf080259459f4f0ab40f29b1fca Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 17 Feb 2018 21:10:32 +0200 Subject: Partly revert 1e526788e6b1d3f42f4d8fdca20e588d42838c80 Some attr_readers should be `protected` instead of `private` See https://travis-ci.org/rails/rails/builds/342800276 --- actionpack/lib/action_dispatch/http/mime_type.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 1d3d241f66..295539281f 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -279,9 +279,12 @@ module Mime def all?; false; end - private + protected + attr_reader :string, :synonyms + private + def to_ary; end def to_a; end -- cgit v1.2.3 From d4eb0dc89ee6b476e2e10869dc282a96f956c6c7 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Feb 2018 13:02:18 -0800 Subject: Rails 6 requires Ruby 2.4.1+ Skipping over 2.4.0 to sidestep the `"symbol_from_string".to_sym.dup` bug. References #32028 --- actionpack/lib/action_controller/metal/strong_parameters.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index a56ac749f8..615c90c496 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/hash/indifferent_access" -require "active_support/core_ext/hash/transform_values" require "active_support/core_ext/array/wrap" require "active_support/core_ext/string/filters" require "active_support/core_ext/object/to_query" -- cgit v1.2.3 From 53d863d4bbfe279e00433ef3672b040e2e6ef267 Mon Sep 17 00:00:00 2001 From: Kohei Suzuki Date: Sun, 18 Feb 2018 21:36:59 +0900 Subject: Skip generating empty CSP header when no policy is configured `Rails.application.config.content_security_policy` is configured with no policies by default. In this case, Content-Security-Policy header should not be generated instead of generating the header with no directives. Firefox also warns "Content Security Policy: Couldn't process unknown directive ''". --- .../lib/action_dispatch/http/content_security_policy.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 4883e23d24..160c345361 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -21,7 +21,10 @@ module ActionDispatch #:nodoc: return response if policy_present?(headers) if policy = request.content_security_policy - headers[header_name(request)] = policy.build(request.controller_instance) + built_policy = policy.build(request.controller_instance) + if built_policy + headers[header_name(request)] = built_policy + end end response @@ -172,7 +175,12 @@ module ActionDispatch #:nodoc: end def build(context = nil) - build_directives(context).compact.join("; ") + ";" + built_directives = build_directives(context).compact + if built_directives.empty? + nil + else + built_directives.join("; ") + ";" + end end private -- cgit v1.2.3 From 52a1f1c226c2238e16d1a4d32faa8d1e6a36a26f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 19 Feb 2018 12:00:29 +0000 Subject: Revert "Merge pull request #32045 from eagletmt/skip-csp-header" This reverts commit 86f7c269073a3a9e6ddec9b957deaa2716f2627d, reversing changes made to 5ece2e4a4459065b5efd976aebd209bbf0cab89b. If a policy is set then we should generate it even if it's empty. However what is happening is that we're accidentally generating an empty policy when the initializer is commented out by default. --- .../lib/action_dispatch/http/content_security_policy.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 160c345361..4883e23d24 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -21,10 +21,7 @@ module ActionDispatch #:nodoc: return response if policy_present?(headers) if policy = request.content_security_policy - built_policy = policy.build(request.controller_instance) - if built_policy - headers[header_name(request)] = built_policy - end + headers[header_name(request)] = policy.build(request.controller_instance) end response @@ -175,12 +172,7 @@ module ActionDispatch #:nodoc: end def build(context = nil) - built_directives = build_directives(context).compact - if built_directives.empty? - nil - else - built_directives.join("; ") + ";" - end + build_directives(context).compact.join("; ") + ";" end private -- cgit v1.2.3 From d85283cc42b1a965944047a2f602153804126f77 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 19 Feb 2018 12:20:43 +0000 Subject: Remove trailing semi-colon from CSP Although the spec[1] is defined in such a way that a trailing semi-colon is valid it also doesn't allow a semi-colon by itself to indicate an empty policy. Therefore it's easier (and valid) just to omit it rather than to detect whether the policy is empty or not. [1]: https://www.w3.org/TR/CSP2/#policy-syntax --- actionpack/lib/action_dispatch/http/content_security_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 4883e23d24..ffac3b8d99 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -172,7 +172,7 @@ module ActionDispatch #:nodoc: end def build(context = nil) - build_directives(context).compact.join("; ") + ";" + build_directives(context).compact.join("; ") end private -- cgit v1.2.3 From 899e2dad03a319a9da86d8dd7b73de4ac9382ff7 Mon Sep 17 00:00:00 2001 From: utilum Date: Sat, 17 Feb 2018 13:25:42 +0100 Subject: Avoid method_redefined warnings in RouteSet::NamedRouteCollection Before: ``` ~/.rbenv/versions/2.5.0/bin/ruby -w -Itest -Ilib -I../activesupport/lib -I../actionpack/lib -I../actionview/lib -I../activemodel/lib test/application/routing_test.rb Run options: --seed 5851 .......~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:156: warning: method redefined; discarding old custom_path ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_path was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:162: warning: method redefined; discarding old custom_url ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_url was here ....~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:156: warning: method redefined; discarding old custom_path ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_path was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:162: warning: method redefined; discarding old custom_url ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_url was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:156: warning: method redefined; discarding old custom_path ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_path was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:162: warning: method redefined; discarding old custom_url ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_url was here ..........~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:156: warning: method redefined; discarding old custom_path ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_path was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:162: warning: method redefined; discarding old custom_url ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_url was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:156: warning: method redefined; discarding old custom_path ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_path was here ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:162: warning: method redefined; discarding old custom_url ~/code/rails/actionpack/lib/action_dispatch/routing/route_set.rb:321: warning: previous definition of custom_url was here ..... Finished in 13.233638s, 1.9647 runs/s, 5.8185 assertions/s. 26 runs, 77 assertions, 0 failures, 0 errors, 0 skips ``` After: ``` ~/.rbenv/versions/2.5.0/bin/ruby -w -Itest -Ilib -I../activesupport/lib -I../actionpack/lib -I../actionview/lib -I../activemodel/lib test/application/routing_test.rb Run options: --seed 38072 .......................... Finished in 12.009632s, 2.1649 runs/s, 6.4115 assertions/s. 26 runs, 77 assertions, 0 failures, 0 errors, 0 skips ``` --- actionpack/lib/action_dispatch/routing/route_set.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index ff6998ae31..a29a5a04ef 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -153,13 +153,13 @@ module ActionDispatch url_name = :"#{name}_url" @path_helpers_module.module_eval do - define_method(path_name) do |*args| + redefine_method(path_name) do |*args| helper.call(self, args, true) end end @url_helpers_module.module_eval do - define_method(url_name) do |*args| + redefine_method(url_name) do |*args| helper.call(self, args, false) end end -- cgit v1.2.3 From 31abee0341cb9d19f0234da7b42dddbabfcd1d4a Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 16 Feb 2018 13:21:48 +0000 Subject: Add support for automatic nonce generation for Rails UJS Because the UJS library creates a script tag to process responses it normally requires the script-src attribute of the content security policy to include 'unsafe-inline'. To work around this we generate a per-request nonce value that is embedded in a meta tag in a similar fashion to how CSRF protection embeds its token in a meta tag. The UJS library can then read the nonce value and set it on the dynamically generated script tag to enable it to execute without needing 'unsafe-inline' enabled. Nonce generation isn't 100% safe - if your script tag is including user generated content in someway then it may be possible to exploit an XSS vulnerability which can take advantage of the nonce. It is however an improvement on a blanket permission for inline scripts. It is also possible to use the nonce within your own script tags by using `nonce: true` to set the nonce value on the tag, e.g <%= javascript_tag nonce: true do %> alert('Hello, World!'); <% end %> Fixes #31689. --- .../metal/content_security_policy.rb | 18 ++++++++++++ .../http/content_security_policy.rb | 32 ++++++++++++++++++++++ 2 files changed, 50 insertions(+) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/metal/content_security_policy.rb b/actionpack/lib/action_controller/metal/content_security_policy.rb index 48a7109bea..95f2f3242d 100644 --- a/actionpack/lib/action_controller/metal/content_security_policy.rb +++ b/actionpack/lib/action_controller/metal/content_security_policy.rb @@ -5,6 +5,14 @@ module ActionController #:nodoc: # TODO: Documentation extend ActiveSupport::Concern + include AbstractController::Helpers + include AbstractController::Callbacks + + included do + helper_method :content_security_policy? + helper_method :content_security_policy_nonce + end + module ClassMethods def content_security_policy(**options, &block) before_action(options) do @@ -22,5 +30,15 @@ module ActionController #:nodoc: end end end + + private + + def content_security_policy? + request.content_security_policy + end + + def content_security_policy_nonce + request.content_security_policy_nonce + end end end diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index ffac3b8d99..a3407c9698 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -21,6 +21,12 @@ module ActionDispatch #:nodoc: return response if policy_present?(headers) if policy = request.content_security_policy + if policy.directives["script-src"] + if nonce = request.content_security_policy_nonce + policy.directives["script-src"] << "'nonce-#{nonce}'" + end + end + headers[header_name(request)] = policy.build(request.controller_instance) end @@ -51,6 +57,8 @@ module ActionDispatch #:nodoc: module Request POLICY = "action_dispatch.content_security_policy".freeze POLICY_REPORT_ONLY = "action_dispatch.content_security_policy_report_only".freeze + NONCE_GENERATOR = "action_dispatch.content_security_policy_nonce_generator".freeze + NONCE = "action_dispatch.content_security_policy_nonce".freeze def content_security_policy get_header(POLICY) @@ -67,6 +75,30 @@ module ActionDispatch #:nodoc: def content_security_policy_report_only=(value) set_header(POLICY_REPORT_ONLY, value) end + + def content_security_policy_nonce_generator + get_header(NONCE_GENERATOR) + end + + def content_security_policy_nonce_generator=(generator) + set_header(NONCE_GENERATOR, generator) + end + + def content_security_policy_nonce + if content_security_policy_nonce_generator + if nonce = get_header(NONCE) + nonce + else + set_header(NONCE, generate_content_security_policy_nonce) + end + end + end + + private + + def generate_content_security_policy_nonce + content_security_policy_nonce_generator.call(self) + end end MAPPINGS = { -- cgit v1.2.3