Thank you for reporting! Did this work for you in Rails 4?

Also, can you create an executable test script using one of the templates here, that would demonstrate the problem you are seeing? This makes it easier for testing if the bug exists between Rails versions. :)

@maclover7 yes this worked fine in Rails 4 but was a controller test. This happened while I was upgrading to Rails 5 yesterday (from 4.2.6).

@maclover7 this is the test script, ran it locally and its failing against master. I also updated the Rails 5 test project I created to use Rails master to verify and same issue. I've included both the BugTest (using just Rack?) and then an ActiveDispatch::IntegrationTest as the params parsing is different but both still fail.

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'
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'
    resources :posts
  end
end

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

  def index
    puts "to_unsafe_h: #{params.to_unsafe_h}"
    puts "authentication_token: #{params[:authentication_token]}"
    raise if params[:authentication_token] != 'test'
    render json: {}
  end
end

require 'minitest/autorun'
require 'rack/test'
require 'rails/test_help'

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

  def test_does_not_raise_because_params_parsed_correctly
    get '/posts', params: { authentication_token: 'test' }, as: :json
    assert last_response.ok?
  end

  private
    def app
      Rails.application
    end
end

class PostsControllerTest < ActionDispatch::IntegrationTest

  test "should not raise (IntegrationTest)" do
    get posts_url, params: { authentication_token: 'test' }, as: :json
    assert_response :success
  end

  private
    def app
      Rails.application
    end

end

@maclover7 I've updated the script in my last comment. It now includes two tests on purpose. It didn't feel right to not have the ActionDispatch::IntegrationTest in there since that's what I'm using. What I thought was interesting though is both fail but parse params differently.

BugTest (using just Rack?)

Look careful at the output from the script. I did a puts after calling to_unsafe_h as I was curious why the params wasn't coming through and notice how its wrapped with a params key. That's unexpected.

I, [2016-08-03T22:18:44.245304 #10832]  INFO -- : Started GET "/posts?params[authentication_token]=test&as=json" for 127.0.0.1 at 2016-08-03 22:18:44 -0400
to_unsafe_h: {"params"=>{"authentication_token"=>"test"}, "as"=>"json", "controller"=>"posts", "action"=>"index"}
authentication_token: 
F, [2016-08-03T22:18:44.247069 #10832] FATAL -- :   

ActionDispatch::IntegrationTest

Then here this is what I was seeing when I upgraded. The params I gave aren't parsed correctly as can be seen in the second line below. Note that this does not happen on POST or other request methods. We include a payload with a JSON GET request and noticed this when upgrading to Rails 5, we already had this test in place and was passing on Rails 4.2.6.

I, [2016-08-03T22:18:44.236935 #10832]  INFO -- : Started GET "/posts.json?%7B%22authentication_token%22%3A%22test%22%7D" for 127.0.0.1 at 2016-08-03 22:18:44 -0400
to_unsafe_h: {"{\"authentication_token\":\"test\"}"=>nil, "controller"=>"posts", "action"=>"index", "format"=>"json"}
authentication_token: 
F, [2016-08-03T22:18:44.241388 #10832] FATAL -- :

@javierjulio are you actually passing JSON with a GET request in production? That would imply that the query parameters are JSON encoded, and I'm not sure if we ever supported that.

@tenderlove yes we are. It is supported and we've been using it for awhile on Rails 4.x. It is accepted to have a GET request with a JSON payload. I figured when I upgraded to Rails 5 and we had a failing test after much research the only conclusion I could come too is that perhaps something was wrong with how params were encoded for a GET request just in test mode. In production (meaning hitting through a real HTTP request, we haven't deployed our Rails 5 upgrade yet) we are able to do a GET request with a JSON payload. Also I've been following very closely the posts at BigBinary on Rails 5 and they had this one described the new as option and the example given is what I'm doing here, GET request with JSON payload. New to submitting bug reports but happy to provide any other info/help I can.

In production (meaning hitting through a real HTTP request, we haven't deployed our Rails 5 upgrade yet) we are able to do a GET request with a JSON payload.

Can you show me what the raw request / response look like (using something like curl -i so we can see the headers) with your production app?

New to submitting bug reports but happy to provide any other info/help I can.

No problem! This is a great bug report! 😄

@javierjulio Thank you for all of your details above! Looks like @tenderlove will be able to help you investigate this further. :)

@tenderlove @maclover7 sorry I made a mistake. 😞 I'm understanding now its non-standard to send JSON data with a GET request, which makes sense. Since I was upgrading I was trying hard to figure out what was wrong thinking it was a bug possibly in Rails 5 but the issue was a lot simpler than that.

I interpreted my failing controller test after upgrading to Rails 5 incorrectly. It was a GET request that meant to pass the authentication_token as a URL param, not a JSON payload (as you said, its not supported, makes sense). I realize now I made that mistake as I moved the controller test to an integration test. I was working on the deprecation for adding keyword params (love this change by the way). I should have asked one of our client developers to verify how the request is made but thought this was a bug since the server side test was passing before.

You think you know everything.. and then you don't haha. Thanks for following up @tenderlove and @maclover7! ❤️ I learned a lot about submitting bug reports. I know now better what to do for next time.

@javierjulio no problem! You did great on this bug report. Repro script is exactly what we needed. Thank you so much!

Hey @javierjulio Aaron and I looked at this and while maybe you don't need it anymore, we realized that Rack::MethodOverride wasn't getting used in integration tests and that if you were using a header to override we weren't respecting that.

I've pushed an update to the integration tests that respects the parameters sent with a GET if it's a JSON request. It uses Rack::MethodOverride to convert to at POST request.

Thanks for taking the time to write a bug report and help debug with us 😄

@eileencodes thanks! No problem, I'm just happy this was of some help to you and @tenderlove. ❤️

I might have another issue (need to replicate with a script and report separately) that started this whole hunt. I was confused with some of the docs on upgrading to Rails 5. Is ActionController::TestCase still supported in v5 and works the same as it was in v4? Meaning its not had its inheritance changed to be ActionDispatch::IntegrationTest? I know if you start a new Rails 5 app that controller tests are generated to use ActionDispatch::IntegrationTest.

This means what I came across would still be an issue in 5.0.0 with controller tests. I've noticed an ActionController::Parameters issue where a permitted array of hashes with the same keys are getting merged into one hash (!) in a controller test but not in an integration test. Once I switched to an integration test everything worked fine. I'll report that later today once I have time to replicate in a script. Now I know how to create these bug report scripts. Look out. 😏

Technically, ActionController::TestCase is deprecated in favor of ActionDispatch::IntegrationTest, but we just soft deprecated it (removed it from documentation and updated the generators) because Rails is still using it everywhere. But if there's a bug in it we should fix it, so please do open an new issue once you have a gist test case we can work with. Thanks!

That's what I thought. Perfect. Pretty sure this is a bug/issue. I have an executable script that replicates what I originally came across with both controller and integration tests to demonstrate the difference (since integration passes). Might be strong parameters related but unsure. I'll report that shortly. Thanks!