mislav/will_paginate

Using GET parameters even if the request was not GET

malteschmitz opened this issue · 3 comments

The LinkRenderer for ActionView merges existing GET params into the parameters for a link using the protected method merge_get_params which checks if the request is a GET requests and then takes all params from the request:

def merge_get_params(url_params)
  if @template.respond_to? :request and @template.request and @template.request.get?
    symbolized_update(url_params, @template.params, GET_PARAMS_BLACKLIST)
  end
  url_params
end

This works well if the request is a GET request, but in case of a non-GET request with GET parameters this fails and completely ignores the GET parameters. I suggest the following change which does not check the requests method, but instead considers only the query_parameters instead of all params:

def merge_get_params(url_params)
  if @template.respond_to? :request and @template.request
    symbolized_update(url_params, @template.request.query_parameters, GET_PARAMS_BLACKLIST)
  end
  url_params
end

Thank you for the suggestion! That's not a bad idea. I just hope that @template.request.query_parameters is sufficiently equivalent to @template.params for GET requests so that this change doesn't cause any backwards-compatibility issues.

I just hope that @template.request.query_parameters is sufficiently equivalent to @template.params for GET requests so that this change doesn't cause any backwards-compatibility issues.

As far as I understand things, the @template.params refers to the method parameters (or more precise its alias params) in Rails parameters.rb and is basically defined as request_parameters.merge(query_parameters) which are both defined in request.rb where they override Rack's corresponding methods. Those are what we call with @template.request.query_parameters. Despite many changes this overall structures seems to have been quite stable throughout the years. It was already present in parameters.rb's earliest version in January 2010, where the file was created du to some reorganisation, so this probably has not changed since Rails 2.

I used this answer on StackOverflow explaining Rail's query and post parameters.

@malteschmitz Thank you for doing the research! I haven't had time to work on this library much lately, so a PR to address this would be much appreciated