Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

'port' and 'host' querystring parameters don't get appended by the LinkRenderer #285

Closed
shivampatel opened this Issue Jan 12, 2013 · 4 comments

Comments

Projects
None yet
3 participants

I have a querystring which has 'host' and 'port' parameters. When LinkRenderer generates the URL for the pages, it omits 'port' and 'host' parameters from the querystring. Renaming 'port' to any other name works. This is the behavior seen in 3.0.0

When upgraded to 3.0.3 or 3.0.4, if there is a 'port' parameter in the initial URL, following is observed:
Initial URL: http://localhost:3000?blah=foo&port=80&blah1=bar
changes to: http://0.0.0.80?blah=foo&blah1=bar&page=2
in the links to other pages

Owner

mislav commented Jan 13, 2013

You'll have to help me understand the issue better. Let's just talk about 3.0.4, since that is the latest version.

will_paginate tries to preserve all parameters in the query string for the current request.

Can you tell me the relevant portion of your routes, what role do "host" and "port" params have in your app, and the URLs you've expected will_paginate to create vs. ones you got? You can anonymize the data if needed.

I wrote a small demo rails app to reproduce this, but it turns out that its even easier to reproduce.
The following steps can reproduce this issue:

  1. Use any application which uses will_paginate 3.0.4.
  2. At any page, append an extra querystring parameter '&port=80' in the URL and press enter.
  3. The pages from now on should preserve the port parameter. They don't.
  4. Change the parameter to '&port1=80' in the URL and press enter.
  5. Now, all other pages preserve the port1 parameter.
  6. At any page, append an extra querystring parameter '&host=foo' in the URL and press enter.
  7. All the other pages (links) now point to this URL:

So to summarize:
'port' querystring parameter doesn't get appended to the querystring of other links that will_paginate generates.
'host' parameter changes the URL, such that the domain name is replaced by the value of the host parameter.

If you can't reproduce, let me know - i'll pass you the code I am using.

This is an issue with the url_for method in Rails.

@template.url_for(url_params)

http://api.rubyonrails.org/v3.2.13/classes/ActionView/Helpers/UrlHelper.html#method-i-url_for

will_paginate should actually use :only_path => true when passing the params to url_for to prevent tampering with the host and port at all.

@mislav mislav added a commit that referenced this issue Sep 18, 2013

@mislav mislav prevent tampering with host, port, protocol
Prevents :host, :port, :protocol settings get inherited from GET query
parameters.

Fixes #285
c62c6f6
Owner

mislav commented Sep 18, 2013

@brookemckim Thanks!

I've prevented tampering with host/port/protocol coming from GET query params. The original issue isn't solved, and that is that query params "host", "port" and "protocol" won't get preserved. I tried to fix this but it's surprisingly difficult due to the fact that url_for accepts a params hash with mixed options, routing params and query params.

@novikserg novikserg added a commit to novikserg/will_paginate that referenced this issue Mar 8, 2014

@novikserg novikserg Preserve url-specific keywords in query string
Preserves :host, :port and :protocol query params and prevents them from
getting inherited in url_for helper.

Fixes #285
dbfa05a

@victormours victormours pushed a commit to tigerlily/will_paginate that referenced this issue May 5, 2014

@mislav mislav + Victor Mours prevent tampering with host, port, protocol
Prevents :host, :port, :protocol settings get inherited from GET query
parameters.

Fixes #285
301e83d

@2xmc 2xmc pushed a commit to 2xmc/will_paginate that referenced this issue May 13, 2014

2xmc prevent tampering with host, port, protocol
Prevents :host, :port, :protocol settings get inherited from GET query
parameters.

Fixes #285 (mislav)
217f7c8

@mislav mislav closed this in cca3616 Jun 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment