From 5bef4c64f0f77cce4bfe6a201454e39a6fc993be Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 28 Jul 2017 17:26:41 +0900 Subject: [PATCH 01/12] Remove redundant `unless current_adapter?(:OracleAdapter)` --- activerecord/test/cases/comment_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 9ab6607668..f2ec5d6518 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -119,8 +119,6 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.integer\s+"rating",\s+precision: 38,\s+comment: "I am running out of imagination"], output else assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output - end - unless current_adapter?(:OracleAdapter) assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output end From d76ed9e47e8209811914eb6e2e55c232e2795471 Mon Sep 17 00:00:00 2001 From: Boris Slobodin Date: Mon, 31 Jul 2017 11:10:59 -0700 Subject: [PATCH 02/12] fix typo in assert_changes error message --- activesupport/lib/active_support/testing/assertions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index f6366bfd39..e2bc51ff7a 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -157,7 +157,7 @@ module ActiveSupport after = exp.call if to == UNTRACKED - error = "#{expression.inspect} didn't changed" + error = "#{expression.inspect} didn't change" error = "#{message}.\n#{error}" if message assert_not_equal before, after, error else From 84f5d2b9babb70747f9193a2743b987c2ab11894 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 1 Aug 2017 06:19:04 +0900 Subject: [PATCH 03/12] Add backticks around method [ci skip] And make reference to `Relation`. --- activerecord/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index aa581ed1e6..8181d67816 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,4 @@ -* When using #or, extract the common conditions and put them before the OR condition. +* When using `Relation#or`, extract the common conditions and put them before the OR condition. *Maxime Handfield Lapointe* From ff657e73f0b61a7e224a9f158467ed2ec915bd9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 31 Jul 2017 17:42:27 -0400 Subject: [PATCH 04/12] Talk about bytes not characters [ci skip] Closes #30012 --- activemodel/lib/active_model/secure_password.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 197f7f20b9..86f051f5ce 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -4,8 +4,8 @@ module ActiveModel module SecurePassword extend ActiveSupport::Concern - # BCrypt hash function can handle maximum 72 characters, and if we pass - # password of length more than 72 characters it ignores extra characters. + # BCrypt hash function can handle maximum 72 bytes, and if we pass + # password of length more than 72 bytes it ignores extra characters. # Hence need to put a restriction on password length. MAX_PASSWORD_LENGTH_ALLOWED = 72 @@ -20,7 +20,7 @@ module ActiveModel # # The following validations are added automatically: # * Password must be present on creation - # * Password length should be less than or equal to 72 characters + # * Password length should be less than or equal to 72 bytes # * Confirmation of password (using a +password_confirmation+ attribute) # # If password confirmation validation is not needed, simply leave out the From c2e3397230615b990c275c789ab313fd00c4004f Mon Sep 17 00:00:00 2001 From: roberts1000 Date: Mon, 31 Jul 2017 17:29:50 -0400 Subject: [PATCH 05/12] Fix rubocop style issues in yarn and spring.rb templates" --- railties/lib/rails/generators/rails/app/templates/bin/yarn | 2 +- .../lib/rails/generators/rails/app/templates/config/spring.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/templates/bin/yarn b/railties/lib/rails/generators/rails/app/templates/bin/yarn index 44f75c22a4..c2f9b6768a 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/yarn +++ b/railties/lib/rails/generators/rails/app/templates/bin/yarn @@ -1,7 +1,7 @@ VENDOR_PATH = File.expand_path('..', __dir__) Dir.chdir(VENDOR_PATH) do begin - exec "yarnpkg #{ARGV.join(" ")}" + exec "yarnpkg #{ARGV.join(' ')}" rescue Errno::ENOENT $stderr.puts "Yarn executable was not detected in the system." $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" diff --git a/railties/lib/rails/generators/rails/app/templates/config/spring.rb b/railties/lib/rails/generators/rails/app/templates/config/spring.rb index c9119b40c0..9fa7863f99 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/spring.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/spring.rb @@ -1,6 +1,6 @@ -%w( +%w[ .ruby-version .rbenv-vars tmp/restart.txt tmp/caching-dev.txt -).each { |path| Spring.watch(path) } +].each { |path| Spring.watch(path) } From 75f87fa99e3c07bfec2719cc24bc1699cd818a70 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Tue, 1 Aug 2017 07:40:01 +0900 Subject: [PATCH 06/12] Remove unnecessary `doc` directory deletion Since 553b695, `doc` directory is not created in application. --- railties/lib/rails/generators/rails/plugin/plugin_generator.rb | 1 - railties/test/generators/plugin_generator_test.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index a1209e4624..61c54b4222 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -120,7 +120,6 @@ task default: :test def test_dummy_clean inside dummy_path do remove_file "db/seeds.rb" - remove_file "doc" remove_file "Gemfile" remove_file "lib/tasks" remove_file "public/robots.txt" diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index e8372dea47..1fa40f74b9 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -491,7 +491,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_no_file "test/dummy/public/robots.txt" assert_no_file "test/dummy/README.md" assert_no_directory "test/dummy/lib/tasks" - assert_no_directory "test/dummy/doc" assert_no_directory "test/dummy/test" assert_no_directory "test/dummy/vendor" assert_no_directory "test/dummy/.git" From 12f3d07439f07d7af0bd8bc4057f5ba174fc05a9 Mon Sep 17 00:00:00 2001 From: Yoshiyuki Hirano Date: Tue, 1 Aug 2017 23:53:57 +0900 Subject: [PATCH 07/12] [ci skip] Fix rails_command comments --- railties/lib/rails/generators/actions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 5cf0985050..2792d7636f 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -216,9 +216,9 @@ module Rails # Runs the supplied rake task (invoked with 'rails ...') # - # rails("db:migrate") - # rails("db:migrate", env: "production") - # rails("gems:install", sudo: true) + # rails_command("db:migrate") + # rails_command("db:migrate", env: "production") + # rails_command("gems:install", sudo: true) def rails_command(command, options = {}) execute_command :rails, command, options end From 92209356c310caabda8665d6369d3b1e4a1800d1 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 1 Aug 2017 13:02:53 -0400 Subject: [PATCH 08/12] Path parameters should default to UTF8 This commit changes the behavior such the path_params now default to UTF8 just like regular parameters. This also changes the behavior such that if a path parameter contains invalid UTF8 it returns a 400 bad request. Previously the behavior was to encode the path params as binary but that's not the same as query params. So this commit makes path params behave the same as query params. It's important to test with a path that's encoded as binary because that's how paths are encoded from the socket. The test that was altered was changed to make the behavior for bad encoding the same as query params. We want to treat path params the same as query params. The params in the test are invalid UTF8 so they should return a bad request. Fixes #29669 *Eileen M. Uchitelle, Aaron Patterson, & Tsukuru Tanimichi* --- .../lib/action_dispatch/http/parameters.rb | 14 ++++++++------ actionpack/lib/action_dispatch/http/request.rb | 9 ++++++--- actionpack/lib/action_dispatch/journey/router.rb | 7 ++++++- actionpack/test/controller/routing_test.rb | 16 ++++++++++++++++ actionpack/test/dispatch/routing_test.rb | 12 ++++++++---- 5 files changed, 44 insertions(+), 14 deletions(-) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index d80d1d714c..ae875eb830 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -57,7 +57,7 @@ module ActionDispatch query_parameters.dup end params.merge!(path_parameters) - params = set_binary_encoding(params) + params = set_binary_encoding(params, params[:controller], params[:action]) set_header("action_dispatch.request.parameters", params) params end @@ -66,6 +66,7 @@ module ActionDispatch def path_parameters=(parameters) #:nodoc: delete_header("action_dispatch.request.parameters") + parameters = set_binary_encoding(parameters, parameters[:controller], parameters[:action]) # If any of the path parameters has an invalid encoding then # raise since it's likely to trigger errors further on. Request::Utils.check_param_encoding(parameters) @@ -85,9 +86,10 @@ module ActionDispatch private - def set_binary_encoding(params) - action = params[:action] - if binary_params_for?(action) + def set_binary_encoding(params, controller, action) + return params unless controller && controller.valid_encoding? + + if binary_params_for?(controller, action) ActionDispatch::Request::Utils.each_param_value(params) do |param| param.force_encoding ::Encoding::ASCII_8BIT end @@ -95,8 +97,8 @@ module ActionDispatch params end - def binary_params_for?(action) - controller_class.binary_params_for?(action) + def binary_params_for?(controller, action) + controller_class_for(controller).binary_params_for?(action) rescue NameError false end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 94b0d4880c..0b38ab7afb 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -76,10 +76,13 @@ module ActionDispatch def controller_class params = path_parameters + params[:action] ||= "index" + controller_class_for(params[:controller]) + end - if params.key?(:controller) - controller_param = params[:controller].underscore - params[:action] ||= "index" + def controller_class_for(name) + if name + controller_param = name.underscore const_name = "#{controller_param.camelize}Controller" ActiveSupport::Dependencies.constantize(const_name) else diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 809bfcbe8a..9987a9bfa1 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -43,6 +43,10 @@ module ActionDispatch req.path_info = "/" + req.path_info unless req.path_info.start_with? "/" end + parameters = route.defaults.merge parameters.transform_values { |val| + val.dup.force_encoding(::Encoding::UTF_8) + } + req.path_parameters = set_params.merge parameters status, headers, body = route.app.serve(req) @@ -67,6 +71,7 @@ module ActionDispatch rails_req.path_info = match.post_match.sub(/^([^\/])/, '/\1') end + parameters = route.defaults.merge parameters yield(route, parameters) end end @@ -119,7 +124,7 @@ module ActionDispatch routes.map! { |r| match_data = r.path.match(req.path_info) - path_parameters = r.defaults.dup + path_parameters = {} match_data.names.zip(match_data.captures) { |name, val| path_parameters[name.to_sym] = Utils.unescape_uri(val) if val } diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index fefb84e095..f09051b306 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -96,6 +96,22 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal({ "artist" => "journey", "song" => "faithfully" }, hash) end + def test_id_encoding + rs.draw do + get "/journey/:id", to: lambda { |env| + param = ActionDispatch::Request.new(env).path_parameters + resp = ActiveSupport::JSON.encode param + [200, {}, [resp]] + } + end + + # The encoding of the URL in production is *binary*, so we add a + # .b here. + hash = ActiveSupport::JSON.decode get(URI("http://example.org/journey/%E5%A4%AA%E9%83%8E".b)) + assert_equal({ "id" => "太郎" }, hash) + assert_equal ::Encoding::UTF_8, hash["id"].encoding + end + def test_id_with_dash rs.draw do get "/journey/:id", to: lambda { |env| diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1af0edc1d3..2bb814e5f4 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4416,6 +4416,10 @@ end class TestInvalidUrls < ActionDispatch::IntegrationTest class FooController < ActionController::Base + def self.binary_params_for?(action) + action == "show" + end + def show render plain: "foo#show" end @@ -4437,19 +4441,19 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest end get "/%E2%EF%BF%BD%A6" - assert_response :not_found + assert_response :bad_request get "/foo/%E2%EF%BF%BD%A6" - assert_response :not_found + assert_response :bad_request get "/foo/show/%E2%EF%BF%BD%A6" assert_response :ok get "/bar/%E2%EF%BF%BD%A6" - assert_response :redirect + assert_response :bad_request get "/foobar/%E2%EF%BF%BD%A6" - assert_response :ok + assert_response :bad_request end end end From e31a1629d4b6a82e334735ff4a78016db24f32f5 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Tue, 1 Aug 2017 12:41:35 -0500 Subject: [PATCH 09/12] Update working_with_javascript_in_rails.md --- guides/source/working_with_javascript_in_rails.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index ed27752a06..9ae86c7943 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -151,7 +151,7 @@ Because of Unobtrusive JavaScript, the Rails "Ajax helpers" are actually in two parts: the JavaScript half and the Ruby half. Unless you have disabled the Asset Pipeline, -[rails-ujs](https://github.com/rails/rails/blob/master/actionview/app/assets/javascripts/rails-ujs.coffee) +[rails-ujs](https://github.com/rails/rails-ujs/tree/master) provides the JavaScript half, and the regular Ruby view helpers add appropriate tags to your DOM. From 789f3be02048cf33c14c8a98c6fe91d0d7b157eb Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 1 Aug 2017 15:10:51 -0400 Subject: [PATCH 10/12] Clarify route encoding test Since this test changed in 9220935 I noticed that it really doesn't make sense anymore. I split the tests into 2 groups to explain what each one does. First these routes should throw a `bad_request` when the encoding isn't valid. We're expecting UTF8 encoding and passing binary, that should be a bad request. For the second test we are setting the `show` route to set `self.binary_params_for?` for that route which will convert the parameters and return a `:ok` instead of a `:bad_request`. --- actionpack/test/dispatch/routing_test.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 2bb814e5f4..446b65a9b9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4425,17 +4425,15 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest end end - test "invalid UTF-8 encoding is treated as ASCII 8BIT encode" do + test "invalid UTF-8 encoding returns a bad request" do with_routing do |set| set.draw do get "/bar/:id", to: redirect("/foo/show/%{id}") - get "/foo/show(/:id)", to: "test_invalid_urls/foo#show" ok = lambda { |env| [200, { "Content-Type" => "text/plain" }, []] } get "/foobar/:id", to: ok ActiveSupport::Deprecation.silence do - get "/foo(/:action(/:id))", controller: "test_invalid_urls/foo" get "/:controller(/:action(/:id))" end end @@ -4446,9 +4444,6 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest get "/foo/%E2%EF%BF%BD%A6" assert_response :bad_request - get "/foo/show/%E2%EF%BF%BD%A6" - assert_response :ok - get "/bar/%E2%EF%BF%BD%A6" assert_response :bad_request @@ -4456,6 +4451,17 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest assert_response :bad_request end end + + test "params encoded with binary_params_for? are treated as ASCII 8bit" do + with_routing do |set| + set.draw do + get "/foo/show(/:id)", to: "test_invalid_urls/foo#show" + end + + get "/foo/show/%E2%EF%BF%BD%A6" + assert_response :ok + end + end end class TestOptionalRootSegments < ActionDispatch::IntegrationTest From dd439bfbda2a39b305b13f00c40d07c6a8c46d8c Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Mon, 31 Jul 2017 14:59:11 +0200 Subject: [PATCH 11/12] Use duktape gem as default JS engine on Windows-MINGW and MS-Visual-C builds The fallback javascript engine on Windows is Windows Script Host (JScript). However this engine isn't able to process the default assets, because it supports ES3 only but the coffeescript compiler requires ES5. Fixes #30014 --- railties/lib/rails/generators/app_base.rb | 2 ++ railties/test/generators/app_generator_test.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 760cb2f81e..4899a63917 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -348,6 +348,8 @@ module Rails comment = "See https://github.com/rails/execjs#readme for more supported runtimes" if defined?(JRUBY_VERSION) GemfileEntry.version "therubyrhino", nil, comment + elsif RUBY_PLATFORM =~ /mingw|mswin/ + GemfileEntry.version "duktape", nil, comment else GemfileEntry.new "mini_racer", nil, comment, { platforms: :ruby }, true end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index ffdee3a6b5..284c200e35 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -521,6 +521,8 @@ class AppGeneratorTest < Rails::Generators::TestCase run_generator if defined?(JRUBY_VERSION) assert_gem "therubyrhino" + elsif RUBY_PLATFORM =~ /mingw|mswin/ + assert_gem "duktape" else assert_file "Gemfile", /# gem 'mini_racer', platforms: :ruby/ end From 38a79124424da5c34a2275cfd45c828ac4a089b2 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Tue, 1 Aug 2017 14:53:17 -0500 Subject: [PATCH 12/12] Update working_with_javascript_in_rails.md --- guides/source/working_with_javascript_in_rails.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 9ae86c7943..304ac97b32 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -151,7 +151,7 @@ Because of Unobtrusive JavaScript, the Rails "Ajax helpers" are actually in two parts: the JavaScript half and the Ruby half. Unless you have disabled the Asset Pipeline, -[rails-ujs](https://github.com/rails/rails-ujs/tree/master) +[rails-ujs](https://github.com/rails/rails/tree/master/actionview/app/assets/javascripts) provides the JavaScript half, and the regular Ruby view helpers add appropriate tags to your DOM.