Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Rails 5 - controller spec example - passing params set to nil value gets their value set to blank string

I have this following Controller spec example passing in my API-only application based on Rails 4.2.0 and Ruby 2.2.1

  let!(:params) { { user_token: user_token } }

  context "- and optional address and contact details params value are received as a nil values -" do
    it "doesn't set the address and contact details and responds with 201 success", check: true do
      params.merge!(
        address_street: nil, address_other: nil, city: nil, state: nil,
        zip_code: nil, phone: nil)

      post :create, params

      expect(response).to have_http_status(201)

      saved_client_id = json_response["id"]
      saved_client = Client.find_by(id: saved_client_id)
      expect(saved_client.address_street).to be_nil
      expect(saved_client.address_other).to be_nil
      expect(saved_client.city).to be_nil
      expect(saved_client.state).to be_nil
      expect(saved_client.zip_code).to be_nil
      expect(saved_client.phone).to be_nil
    end
  end

However evaluating my application against Rails 5 (edge version) and Ruby 2.2.3 the same spec fails with following error:

  1) Api::V1::ClientsController POST #create when receives valid client details - and optional address and contact details params value are received as nil values - doesn't set the address and contact details and responds with 201 success
     Failure/Error: expect(saved_client.address_street).to be_nil
       expected: nil
            got: ""
     # ./spec/controllers/api/v1/clients_controller_spec.rb:352:in `block (5 levels) in <top (required)>'
     # ./spec/rails_helper.rb:61:in `block (3 levels) in <top (required)>'
     # /home/jignesh/.rvm/gems/ruby-2.2.3@myapp-on-rails-5/gems/database_cleaner-1.5.1/lib/database_cleaner/generic/base.rb:16:in `cleaning'
     # /home/jignesh/.rvm/gems/ruby-2.2.3@myapp-on-rails-5/gems/database_cleaner-1.5.1/lib/database_cleaner/base.rb:92:in `cleaning'
     # /home/jignesh/.rvm/gems/ruby-2.2.3@myapp-on-rails-5/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:86:in `block (2 levels) in cleaning'
     # /home/jignesh/.rvm/gems/ruby-2.2.3@myapp-on-rails-5/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:87:in `call'
     # /home/jignesh/.rvm/gems/ruby-2.2.3@myapp-on-rails-5/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:87:in `cleaning'
 # ./spec/rails_helper.rb:60:in `block (2 levels) in <top (required)>'

I did inspected the Rails source code at few points and found that the nil values are transformed to blank values before reaching the controller's targeted action logic.

This changed behavior is setting attributes to empty strings, when they are expected to be nil.

In my app's Gemfile (for using Rails 5) I have specified Rails using following code:

gem 'rails', git: 'https://github.com/rails/rails.git'

gem 'rack', :git => 'https://github.com/rack/rack.git'
gem 'arel', :git => 'https://github.com/rails/arel.git'

and in Gemfile.lock following can be seen (Gem and Dependencies portions truncated to cut it short):

GIT
  remote: git://github.com/capistrano/rbenv.git
  revision: 6f1216cfe0a6b4ac23ca4eaf8acf012e8165d247
  specs:
    capistrano-rbenv (2.0.3)
      capistrano (~> 3.1)
      sshkit (~> 1.3)

GIT
  remote: https://github.com/rack/rack.git
  revision: c393176b0edf3e5d06cabbb6eb9d9c7a07b2afa7
  specs:
    rack (2.0.0.alpha)
      json

GIT
  remote: https://github.com/rails/arel.git
  revision: 3c429c5d86e9e2201c2a35d934ca6a8911c18e69
  specs:
    arel (7.0.0.alpha)

GIT
  remote: https://github.com/rails/rails.git
  revision: 58df2f4b4abcce0b698c2540da215a565c24cbc9
  specs:
    actionmailer (5.0.0.alpha)
      actionpack (= 5.0.0.alpha)
      actionview (= 5.0.0.alpha)
      activejob (= 5.0.0.alpha)
      mail (~> 2.5, >= 2.5.4)
      rails-dom-testing (~> 1.0, >= 1.0.5)
    actionpack (5.0.0.alpha)
      actionview (= 5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      rack (~> 2.x)
      rack-test (~> 0.6.3)
      rails-dom-testing (~> 1.0, >= 1.0.5)
      rails-html-sanitizer (~> 1.0, >= 1.0.2)
    actionview (5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      builder (~> 3.1)
      erubis (~> 2.7.0)
      rails-dom-testing (~> 1.0, >= 1.0.5)
      rails-html-sanitizer (~> 1.0, >= 1.0.2)
    activejob (5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      globalid (>= 0.3.0)
    activemodel (5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      builder (~> 3.1)
    activerecord (5.0.0.alpha)
      activemodel (= 5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      arel (= 7.0.0.alpha)
    activesupport (5.0.0.alpha)
      concurrent-ruby (~> 1.0)
      i18n (~> 0.7)
      json (~> 1.7, >= 1.7.7)
      method_source
      minitest (~> 5.1)
      tzinfo (~> 1.1)
    rails (5.0.0.alpha)
      actionmailer (= 5.0.0.alpha)
      actionpack (= 5.0.0.alpha)
      actionview (= 5.0.0.alpha)
      activejob (= 5.0.0.alpha)
      activemodel (= 5.0.0.alpha)
      activerecord (= 5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      bundler (>= 1.3.0, < 2.0)
      railties (= 5.0.0.alpha)
      sprockets-rails (>= 2.0.0)
    railties (5.0.0.alpha)
      actionpack (= 5.0.0.alpha)
      activesupport (= 5.0.0.alpha)
      method_source
      rake (>= 0.8.7)
      thor (>= 0.18.1, < 2.0)
...
....

Can anybody please let me know what changed caused this? I guess it has something to do with change in Rails 5 or the latest Rack. Is this some-kind of bug which shall be fixed in the final release version or this is deliberate change.

like image 606
Jignesh Gohel Avatar asked Nov 23 '15 14:11

Jignesh Gohel


3 Answers

I have found the root cause of the mentioned behavior: In Rails 5 it is caused by the default CONTENT_TYPE header being set to 'application/x-www-form-urlencoded' by ActionController::TestRequest #assign_parameters method, however in Rails 4.2.0 this is not the case.

Detailed findings are given below on how I reached the conclusion:

In context of the params being passed in the spec example(shown in my question post) the execution flows in Rails 5 (and its Rack version) and Rails 4.2.0 (and its Rack version) goes in manner detailed below:

Rails 5

actionpack/lib/action_dispatch/http_request.rb#form_data? returns true

actionpack/lib/action_dispatch/http_request.rb#POST method looks like following:

# Override Rack's POST method to support indifferent access
def POST
  fetch_header("action_dispatch.request.request_parameters") do
    pr = parse_formatted_parameters(params_parsers) do |params|
      super || {}
    end
    self.request_parameters = Request::Utils.normalize_encode_params(pr)
  end
rescue ParamsParser::ParseError # one of the parse strategies blew up
  self.request_parameters = Request::Utils.normalize_encode_params(super || {})
  raise
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
  raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}")
end
alias :request_parameters :POST

That while trying to evaluate fetch_header("action_dispatch.request.request_parameters") runs the default value block which invokes super which makes the call goes to Rack's Request (/rack-c393176b0edf/lib/rack/request.rb) POST method. I have shown this method's code below with few debug statements I put:

rack/lib/rack/request.rb#POST

  # Returns the data received in the request body.
  #
  # This method support both application/x-www-form-urlencoded and
  # multipart/form-data.
  def POST
    puts ">>>>>>>>>>> DEBUG 2"
    if get_header(RACK_INPUT).nil?
      puts ">>>>>>>>>>> DEBUG 2.1"
      raise "Missing rack.input"
    elsif get_header(RACK_REQUEST_FORM_INPUT) == get_header(RACK_INPUT)
      puts ">>>>>>>>>>> DEBUG 2.2"
      get_header(RACK_REQUEST_FORM_HASH)
    elsif form_data? || parseable_data?
      puts ">>>>>>>>>>> DEBUG 2.3"
      unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart)
        form_vars = get_header(RACK_INPUT).read

        # Fix for Safari Ajax postings that always append \0
        # form_vars.sub!(/\0\z/, '') # performance replacement:
        form_vars.slice!(-1) if form_vars[-1] == ?\0

        set_header RACK_REQUEST_FORM_VARS, form_vars
        set_header RACK_REQUEST_FORM_HASH, parse_query(form_vars, '&')
        get_header(RACK_INPUT).rewind
      end
      set_header RACK_REQUEST_FORM_INPUT, get_header(RACK_INPUT)
      get_header RACK_REQUEST_FORM_HASH
    else
      puts ">>>>>>>>>>> DEBUG 2.4"
      {}
    end

With those debug statements the execution flow ended in ">>>>>>>>>>> DEBUG 2.3". There I also inspected get_header RACK_REQUEST_FORM_HASH and it printed

>>>>>>>>>>> get_header RACK_REQUEST_FORM_HASH: {"address_other"=>"", "address_street"=>"", "city"=>"", "client_residence_type_id"=>"", "name"=>"Test Client 1", "phone"=>"", "provider_id"=>"64", "state"=>"", "zip_code"=>""}

So its the parse_query(form_vars, '&') method which transforms the nil values to empty strings.

Rails 4.2.0

actionpack/lib/action_dispatch/http_request.rb#form_data? returns false

actionpack/lib/action_dispatch/http_request.rb#POST method looks like following:

# Override Rack's POST method to support indifferent access
def POST
  @env["action_dispatch.request.request_parameters"] ||= Utils.deep_munge(normalize_encode_params(super || {}))
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
  raise ActionController::BadRequest.new(:request, e)
end
alias :request_parameters :POST

That invokes super which makes the call goes to Rack's Request (rack-1.6.4/lib/rack/request.rb) POST method. I have shown this method's code below with few debug statements I put:

rack-1.6.4/lib/rack/request.rb#parseable_data? returns false

rack-1.6.4/lib/rack/request.rb#POST the flow ended in ">>>>>>>>>>> DEBUG 2.4"

def POST
  puts ">>>>>>>>>>> DEBUG 2"
  if @env["rack.input"].nil?
    puts ">>>>>>>>>>> DEBUG 2.1"
    raise "Missing rack.input"
  elsif @env["rack.request.form_input"].equal? @env["rack.input"]
    puts ">>>>>>>>>>> DEBUG 2.2"
    @env["rack.request.form_hash"]
  elsif form_data? || parseable_data?
    puts ">>>>>>>>>>> DEBUG 2.3"
    unless @env["rack.request.form_hash"] = parse_multipart(env)
      form_vars = @env["rack.input"].read

      # Fix for Safari Ajax postings that always append \0
      # form_vars.sub!(/\0\z/, '') # performance replacement:
      form_vars.slice!(-1) if form_vars[-1] == ?\0

      @env["rack.request.form_vars"] = form_vars
      @env["rack.request.form_hash"] = parse_query({ :query => form_vars, :separator => '&' })

      @env["rack.input"].rewind
    end
    @env["rack.request.form_input"] = @env["rack.input"]
    @env["rack.request.form_hash"]
  else
    puts ">>>>>>>>>>> DEBUG 2.4"
    {}
  end
end

This brings to my notice that in Rails 5 content_mime_type which is internally used by form_data? is set and hence the submitted params by the spec example gets parsed as form params.

However in Rails 4.2.0 the content_mime_type is not found set which doesn't cause the params submitted to be parsed as form_params.

Rails 4.2.0

The content_mime_type method is defined in ActionDispatch::Http::MimeNegotiation module

  def content_mime_type
    @env["action_dispatch.request.content_type"] ||= begin
      if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/
        Mime::Type.lookup($1.strip.downcase)
      else
        nil
      end
    end
  end

that returns nil

Rails 5

The content_mime_type method is defined in ActionDispatch::Http::MimeNegotiation module

  def content_mime_type
    fetch_header("action_dispatch.request.content_type") do |k|
      v = if get_header('CONTENT_TYPE') =~ /^([^,\;]*)/
        Mime::Type.lookup($1.strip.downcase)
      else
        nil
      end
      set_header k, v
    end
  end

In this case if get_header('CONTENT_TYPE') =~ /^([^,\;]*)/ is evaluated to true and hence Mime::Type.lookup($1.strip.downcase) is returned

Rails 4.2.0

The header CONTENT_TYPE doesn't get set by

actionpack/lib/action_controller/test_case.rb#def assign_parameters(routes, controller_path, action, parameters = {}) method

def assign_parameters(routes, controller_path, action, parameters = {})
  parameters = parameters.symbolize_keys.merge(:controller => controller_path, :action => action)
  extra_keys = routes.extra_keys(parameters)
  non_path_parameters = get? ? query_parameters : request_parameters
  parameters.each do |key, value|
    if value.is_a?(Array) && (value.frozen? || value.any?(&:frozen?))
      value = value.map{ |v| v.duplicable? ? v.dup : v }
    elsif value.is_a?(Hash) && (value.frozen? || value.any?{ |k,v| v.frozen? })
      value = Hash[value.map{ |k,v| [k, v.duplicable? ? v.dup : v] }]
    elsif value.frozen? && value.duplicable?
      value = value.dup
    end

    if extra_keys.include?(key)
      non_path_parameters[key] = value
    else
      if value.is_a?(Array)
        value = value.map(&:to_param)
      else
        value = value.to_param
      end

      path_parameters[key] = value
    end
  end

  # Clear the combined params hash in case it was already referenced.
  @env.delete("action_dispatch.request.parameters")

  # Clear the filter cache variables so they're not stale
  @filtered_parameters = @filtered_env = @filtered_path = nil

  params = self.request_parameters.dup
  %w(controller action only_path).each do |k|
    params.delete(k)
    params.delete(k.to_sym)
  end
  data = params.to_query

  @env['CONTENT_LENGTH'] = data.length.to_s
  @env['rack.input'] = StringIO.new(data)
end

Rails 5

The header CONTENT_TYPE gets set by

actionpack/lib/action_controller/test_case.rb#assign_parameters(routes, controller_path, action, parameters, generated_path, query_string_keys) method

def assign_parameters(routes, controller_path, action, parameters, generated_path, query_string_keys)
  non_path_parameters = {}
  path_parameters = {}

  parameters.each do |key, value|
    if query_string_keys.include?(key)
      non_path_parameters[key] = value
    else
      if value.is_a?(Array)
        value = value.map(&:to_param)
      else
        value = value.to_param
      end

      path_parameters[key] = value
    end
  end

  if get?
    if self.query_string.blank?
      self.query_string = non_path_parameters.to_query
    end
  else
    if ENCODER.should_multipart?(non_path_parameters)
      self.content_type = ENCODER.content_type
      data = ENCODER.build_multipart non_path_parameters
    else
      fetch_header('CONTENT_TYPE') do |k|
        set_header k, 'application/x-www-form-urlencoded'
      end

      case content_mime_type.to_sym
      when nil
        raise "Unknown Content-Type: #{content_type}"
      when :json
        data = ActiveSupport::JSON.encode(non_path_parameters)
      when :xml
        data = non_path_parameters.to_xml
      when :url_encoded_form
        data = non_path_parameters.to_query
      else
        @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters }
        data = non_path_parameters.to_query
      end
    end

    set_header 'CONTENT_LENGTH', data.length.to_s
    set_header 'rack.input', StringIO.new(data)
  end

  fetch_header("PATH_INFO") do |k|
    set_header k, generated_path
  end
  path_parameters[:controller] = controller_path
  path_parameters[:action] = action

  self.path_parameters = path_parameters
end

As can be seen for POST request following code gets executed which sets the CONTENT_TYPE header to a default value 'application/x-www-form-urlencoded'

      fetch_header('CONTENT_TYPE') do |k|
        set_header k, 'application/x-www-form-urlencoded'
      end

Thanks.

like image 135
Jignesh Gohel Avatar answered Sep 20 '22 13:09

Jignesh Gohel


It looks like this issue is known but not fixed yet. There is a workaround mentioned in this issue: https://github.com/rspec/rspec-rails/issues/1655

I've tested and used this in my rspec controller test and it sends the data across properly:

before { request.env['CONTENT_TYPE'] = 'application/json' }

like image 25
violetaria Avatar answered Sep 21 '22 13:09

violetaria


A variant of violetaria's answer is to add as: :json to your requests, e.g.

post :create, params: {…}, as: :json
like image 27
Arnaud Avatar answered Sep 22 '22 13:09

Arnaud