From ca9ac310028d0009edf1fbf657541d2dea327535 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 22 Apr 2017 18:29:05 +0900 Subject: `respond_to_missing?` should be private Follow up of 03d3f036. Some of `respond_to?` were replaced to `respond_to_missing?` in 03d3f036. But the visibility is still public. It should be private. --- actionpack/lib/action_dispatch/http/mime_type.rb | 8 ++++---- actionpack/lib/action_dispatch/routing/routes_proxy.rb | 5 +++-- actionpack/lib/action_dispatch/testing/integration.rb | 11 ++++++----- 3 files changed, 13 insertions(+), 11 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 0cf010f59e..74a3afab44 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -326,11 +326,11 @@ module Mime def ref; end - def respond_to_missing?(method, include_private = false) - method.to_s.ends_with? "?" - end - private + def respond_to_missing?(method, _) + method.to_s.ends_with? "?" + end + def method_missing(method, *args) false if method.to_s.ends_with? "?" end diff --git a/actionpack/lib/action_dispatch/routing/routes_proxy.rb b/actionpack/lib/action_dispatch/routing/routes_proxy.rb index ee847eaeed..c1423f770f 100644 --- a/actionpack/lib/action_dispatch/routing/routes_proxy.rb +++ b/actionpack/lib/action_dispatch/routing/routes_proxy.rb @@ -19,7 +19,8 @@ module ActionDispatch end end - def respond_to_missing?(method, include_private = false) + private + def respond_to_missing?(method, _) super || @helpers.respond_to?(method) end @@ -32,7 +33,7 @@ module ActionDispatch @helpers.#{method}(*args) end RUBY - send(method, *args) + public_send(method, *args) else super end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 2e2db98ad6..2416c58817 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -385,14 +385,15 @@ module ActionDispatch integration_session.default_url_options = options end - def respond_to_missing?(method, include_private = false) - integration_session.respond_to?(method, include_private) || super + private + def respond_to_missing?(method, _) + integration_session.respond_to?(method) || super end # Delegate unhandled messages to the current session instance. - def method_missing(sym, *args, &block) - if integration_session.respond_to?(sym) - integration_session.__send__(sym, *args, &block).tap do + def method_missing(method, *args, &block) + if integration_session.respond_to?(method) + integration_session.public_send(method, *args, &block).tap do copy_session_variables! end else -- cgit v1.2.3 From e06f68fdb2132623d27a26ae58213b09ecb308ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 26 Apr 2017 19:32:49 -0700 Subject: Do not try to encoding the parameters when the controller is not defined When you have a route that points to an nonexistent controller we raise an exception. This exception was being caught by the DebugExceptions middleware in development, but when trying to render the error page, we are reading the request format[[1][]]. To determine the request format we are reading the format parameters[[2][]], and to be able to read the parameters we need to encode them[[3][]]. This was raising another exception that to encode the parameter we try to load the controller to determine if we need to encode the parameters are binary[[4][]]. This new exception inside the DebugExceptions middleware makes Rails to render a generic error page. To avoid this new exception now we only encode the parameters when the controller can be loaded. Fixes #28892 [1]: https://github.com/rails/rails/blob/f52cdaac6336f99d13622ff9bda556a3124a4121/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L80 [2]: https://github.com/rails/rails/blob/f52cdaac6336f99d13622ff9bda556a3124a4121/actionpack/lib/action_dispatch/http/mime_negotiation.rb#L63 [3]: https://github.com/rails/rails/blob/f52cdaac6336f99d13622ff9bda556a3124a4121/actionpack/lib/action_dispatch/http/parameters.rb#L58 [4]: https://github.com/rails/rails/blob/f52cdaac6336f99d13622ff9bda556a3124a4121/actionpack/lib/action_dispatch/http/parameters.rb#L88 --- actionpack/lib/action_dispatch/http/parameters.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 79a2ef965b..7c585dbe68 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -85,7 +85,7 @@ module ActionDispatch def set_binary_encoding(params) action = params[:action] - if controller_class.binary_params_for?(action) + if binary_params_for?(action) ActionDispatch::Request::Utils.each_param_value(params) do |param| param.force_encoding ::Encoding::ASCII_8BIT end @@ -93,6 +93,12 @@ module ActionDispatch params end + def binary_params_for?(action) + controller_class.binary_params_for?(action) + rescue NameError + false + end + def parse_formatted_parameters(parsers) return yield if content_length.zero? || content_mime_type.nil? -- cgit v1.2.3 From 89389428b5d04e14f09009ac9af593d7c3da709f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 30 Apr 2017 02:41:44 +0900 Subject: Cleanup CHANGELOGs [ci skip] * Remove trailing spaces. * Add backticks around method and command. * Fix indentation. --- actionpack/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 51cec82801..cc908dcc43 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -10,4 +10,5 @@ *Julian Nadeau* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md) for previous changes. -- cgit v1.2.3 From 0d0015c98ad9af08f812a692ca30a70e66a75159 Mon Sep 17 00:00:00 2001 From: dixpac Date: Sun, 30 Apr 2017 15:10:21 +0200 Subject: Add docs for Router::Utils.unescape_uri method --- actionpack/lib/action_dispatch/journey/router/utils.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index ffcd875b4d..f937597c1b 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -84,6 +84,10 @@ module ActionDispatch ENCODER.escape_fragment(fragment.to_s) end + # Replaces any escaped sequences with their unescaped representations + # + # uri = "/topics?title=Ruby%20on%20Rails" + # unescape_uri(uri) #=> "/topics?title=Ruby on Rails" def self.unescape_uri(uri) ENCODER.unescape_uri(uri) end -- cgit v1.2.3 From da70168715c02980986279d60e3c6631cfb164d2 Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Sun, 30 Apr 2017 10:43:51 -0400 Subject: Add period [ci skip] --- actionpack/lib/action_dispatch/journey/router/utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index f937597c1b..e6353f0142 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -84,7 +84,7 @@ module ActionDispatch ENCODER.escape_fragment(fragment.to_s) end - # Replaces any escaped sequences with their unescaped representations + # Replaces any escaped sequences with their unescaped representations. # # uri = "/topics?title=Ruby%20on%20Rails" # unescape_uri(uri) #=> "/topics?title=Ruby on Rails" -- cgit v1.2.3 From 7cb71c5ce37b7bc599de851d44eaa6a948daec03 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 May 2017 01:11:14 +0900 Subject: Remove `:doc:` in `:nodoc:` class [ci skip] The `:doc:` was added in bc478158 but originally `UriEncoder` is a `:nodoc:` class. --- actionpack/lib/action_dispatch/journey/router/utils.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index e6353f0142..e624b4c80d 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -59,11 +59,11 @@ module ActionDispatch end private - def escape(component, pattern) # :doc: + def escape(component, pattern) component.gsub(pattern) { |unsafe| percent_encode(unsafe) }.force_encoding(US_ASCII) end - def percent_encode(unsafe) # :doc: + def percent_encode(unsafe) safe = EMPTY.dup unsafe.each_byte { |b| safe << DEC2HEX[b] } safe -- cgit v1.2.3 From b201474756a2ee493406ad0cb49f49c6873bdc28 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 7 May 2017 04:08:58 +0900 Subject: Should escape meta characters in regexp --- actionpack/test/dispatch/request_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 2f9228a62d..28cbde028d 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -110,8 +110,8 @@ class RequestIP < BaseRequestTest request.remote_ip } assert_match(/IP spoofing attack/, e.message) - assert_match(/HTTP_X_FORWARDED_FOR="1.1.1.1"/, e.message) - assert_match(/HTTP_CLIENT_IP="2.2.2.2"/, e.message) + assert_match(/HTTP_X_FORWARDED_FOR="1\.1\.1\.1"/, e.message) + assert_match(/HTTP_CLIENT_IP="2\.2\.2\.2"/, e.message) end test "remote ip with spoof detection disabled" do -- cgit v1.2.3 From fb0fae9747ceec065e75867ea83afb713a5b449b Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Wed, 10 May 2017 23:48:46 +0100 Subject: Pass block in ActionController::Parameters#delete In order to fully support the same interface as `Hash#delete`, we need to pass the block through to the underlying method, not just the key. This used to work correctly, but it regressed when `ActionController::Parameters` stopped inheriting from `Hash` in 5.0. --- .../action_controller/metal/strong_parameters.rb | 4 ++-- .../test/controller/parameters/mutators_test.rb | 21 +++++++++++++++++++++ 2 files changed, 23 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 7864f9decd..20330b5091 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -666,8 +666,8 @@ module ActionController # to key. If the key is not found, returns the default value. If the # optional code block is given and the key is not found, pass in the key # and return the result of block. - def delete(key) - convert_value_to_parameters(@parameters.delete(key)) + def delete(key, &block) + convert_value_to_parameters(@parameters.delete(key, &block)) end # Returns a new instance of ActionController::Parameters with only diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index e61bbdbe13..2c36f488c6 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -25,6 +25,27 @@ class ParametersMutatorsTest < ActiveSupport::TestCase assert_not @params.delete(:person).permitted? end + test "delete returns the value when the key is present" do + assert_equal "32", @params[:person].delete(:age) + end + + test "delete removes the entry when the key present" do + @params[:person].delete(:age) + assert_not @params[:person].key?(:age) + end + + test "delete returns nil when the key is not present" do + assert_equal nil, @params[:person].delete(:first_name) + end + + test "delete returns the value of the given block when the key is not present" do + assert_equal "David", @params[:person].delete(:first_name) { "David" } + end + + test "delete yields the key to the given block when the key is not present" do + assert_equal "first_name: David", @params[:person].delete(:first_name) { |k| "#{k}: David" } + end + test "delete_if retains permitted status" do @params.permit! assert @params.delete_if { |k| k == "person" }.permitted? -- cgit v1.2.3 From 8607c25ba7810573733d9b37d0015154ba059f5e Mon Sep 17 00:00:00 2001 From: eileencodes Date: Fri, 12 May 2017 14:12:40 -0400 Subject: Maintain original encoding from path When the path info is read from the socket it's encoded as ASCII 8BIT. The unescape method changes the encoding to UTF8 but it should maintain the encoding of the string that's passed in. This causes parameters to be force encoded to UTF8 when we don't actually know what the encoding of the parameter should be. --- actionpack/lib/action_dispatch/journey/router/utils.rb | 2 ++ actionpack/test/journey/router/utils_test.rb | 5 +++++ 2 files changed, 7 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index e624b4c80d..6d400f3364 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -13,11 +13,13 @@ module ActionDispatch # normalize_path("") # => "/" # normalize_path("/%ab") # => "/%AB" def self.normalize_path(path) + encoding = path.encoding path = "/#{path}" path.squeeze!("/".freeze) path.sub!(%r{/+\Z}, "".freeze) path.gsub!(/(%[a-f0-9]{2})/) { $1.upcase } path = "/" if path == "".freeze + path.force_encoding(encoding) path end diff --git a/actionpack/test/journey/router/utils_test.rb b/actionpack/test/journey/router/utils_test.rb index b77bf6628a..74277a4325 100644 --- a/actionpack/test/journey/router/utils_test.rb +++ b/actionpack/test/journey/router/utils_test.rb @@ -31,6 +31,11 @@ module ActionDispatch def test_normalize_path_uppercase assert_equal "/foo%AAbar%AAbaz", Utils.normalize_path("/foo%aabar%aabaz") end + + def test_normalize_path_maintains_string_encoding + path = "/foo%AAbar%AAbaz".b + assert_equal Encoding::ASCII_8BIT, Utils.normalize_path(path).encoding + end end end end -- cgit v1.2.3 From e605921614c286ab2d6b4cafb655230a3d9b5fee Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Tue, 16 May 2017 07:45:41 +0900 Subject: Fix `TestInvalidUrls` with rack 2.0.3 Currently, raise `BadRequest` if params encoding is invalid. https://github.com/rails/rails/blob/5-1-stable/actionpack/lib/action_dispatch/http/parameters.rb#L64..L74 https://github.com/rails/rails/blob/5-1-stable/actionpack/lib/action_dispatch/request/utils.rb#L26..L39 However, env values are ensure encoded in ASCII 8 BIT at rack 2.0.3. https://github.com/rack/rack/commit/68db9aa99e3e2775a58621f658b2a7a0f67db459 Therefore, even if specify an invalid urls, it will not cause an error. --- actionpack/test/dispatch/routing_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index d64917e0d3..32cd78e492 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4419,7 +4419,7 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest end end - test "invalid UTF-8 encoding returns a 400 Bad Request" do + test "invalid UTF-8 encoding is treated as ASCII 8BIT encode" do with_routing do |set| set.draw do get "/bar/:id", to: redirect("/foo/show/%{id}") @@ -4435,19 +4435,19 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest end get "/%E2%EF%BF%BD%A6" - assert_response :bad_request + assert_response :not_found get "/foo/%E2%EF%BF%BD%A6" - assert_response :bad_request + assert_response :not_found get "/foo/show/%E2%EF%BF%BD%A6" - assert_response :bad_request + assert_response :ok get "/bar/%E2%EF%BF%BD%A6" - assert_response :bad_request + assert_response :redirect get "/foobar/%E2%EF%BF%BD%A6" - assert_response :bad_request + assert_response :ok end end end -- cgit v1.2.3 From bfbbb1207930e7ebe56d4a99abd53b2aa66e0b6e Mon Sep 17 00:00:00 2001 From: sepehr500 Date: Fri, 12 May 2017 14:43:34 -0400 Subject: Fixed string being modified in place causing frozen string errors in Ruby 2.3 --- actionpack/lib/action_dispatch/http/upload.rb | 8 ++++++-- actionpack/test/dispatch/uploaded_file_test.rb | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 61ba052e45..225272d66e 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -27,14 +27,18 @@ module ActionDispatch @tempfile = hash[:tempfile] raise(ArgumentError, ":tempfile is required") unless @tempfile - @original_filename = hash[:filename] - if @original_filename + if hash[:filename] + @original_filename = hash[:filename].dup + begin @original_filename.encode!(Encoding::UTF_8) rescue EncodingError @original_filename.force_encoding(Encoding::UTF_8) end + else + @original_filename = nil end + @content_type = hash[:type] @headers = hash[:head] end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 51680216e4..0074d2a314 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -13,6 +13,12 @@ module ActionDispatch assert_equal "foo", uf.original_filename end + def test_filename_is_different_object + file_str = "foo" + uf = Http::UploadedFile.new(filename: file_str, tempfile: Object.new) + assert_not_equal file_str.object_id , uf.original_filename.object_id + end + def test_filename_should_be_in_utf_8 uf = Http::UploadedFile.new(filename: "foo", tempfile: Object.new) assert_equal "UTF-8", uf.original_filename.encoding.to_s -- cgit v1.2.3