Use of ParameterFilter changes request.filtered_parameters class to Hash

[leonelgalan]

Steps to reproduce

By default all Rails apps are created with an initializer called filter_parameter_logging.rb and its default contents are:

Rails.application.config.filter_parameters += [:password]

This is enough to change the class of request.filtered_parameters from ActiveSupport::HashWithIndifferentAccess to Hash. Omitting this line (defaults to []) returns a duplicate of parameters which is a ActiveSupport::HashWithIndifferentAccess.

Expected behavior

ParameterFilter should return the a hash with the same class than the parameters it is filtering.

Actual behavior

When filtering, ParameterFilter creates a new copy of the hash but looses its indifferent access property.

System configuration

Rails version: 5.0.1

Ruby version: 2.3.1

Failing Test

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rails', github: 'rails/rails', branch: '5-0-stable'
  gem 'arel', github: 'rails/arel', branch: '7-1-stable'
  gem 'byebug'
end

require 'action_controller/railtie'

class TestApp < Rails::Application
  config.root = File.dirname(__FILE__)
  config.session_store :cookie_store, key: 'cookie_store_key'
  secrets.secret_token    = 'secret_token'
  secrets.secret_key_base = 'secret_key_base'

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get '/' => 'test#index'
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: 'Home'
  end
end

require 'minitest/autorun'
require 'rack/test'

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_returns_success
    get '/'
    assert last_response.ok?
  end

  def test_parameter_filter_class
    [
      [{'foo'=>'bar'}.with_indifferent_access, %w'bar'],
      [{'bar'=>'foo'}.with_indifferent_access, []]
    ].each do |before_filter, filter_words|
      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
      assert_instance_of ActiveSupport::HashWithIndifferentAccess,
                         parameter_filter.filter(before_filter)
    end
  end

  private
    def app
      Rails.application
    end
end

  1) Failure:
BugTest#test_parameter_filter_class [failing_test.rb:56]:
Expected {"foo"=>"bar"} to be an instance of ActiveSupport::HashWithIndifferentAccess, not Hash.

[leonelgalan]

Possible Solutions

I think the problem originates on https://github.com/rails/rails/blob/5-0-stable/actionpack/lib/action_dispatch/http/parameter_filter.rb#L55: filtered_params = {}. Instead of initializing a simple Hash we could initialize a hash matching original_params class or a ActiveSupport::HashWithIndifferentAccess every time.

[maclover7]

+1 to changing it to be original_params.class.new or similar; can you open up a PR to do this?

[leonelgalan]

Thanks for your feedback, that's my preference as well. I'm on it! I'll the PR To this Issue.