Rails: ignore InvalidPage exception in production

[graudeejs]

in our logs we found this problem

ArgumentError: invalid value for Integer(): 'xoforvfmy' in "[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/page_number.rb" on line 16.

Some web crawler requested page=xoforvfmy

I tried to find best place to fix this, but failed.

relavent backtrace:

[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/page_number.rb:16→ Integer
[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/page_number.rb:16→ initialize
[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/page_number.rb:54→ new
[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/page_number.rb:54→ PageNumber
[GEM_ROOT]/gems/will_paginate-3.0.2/lib/will_paginate/active_record.rb:142→ page

[quainjn]

I've been getting the same error periodically for the last couple weeks.

ArgumentError: invalid value for Integer(): "vvxvpgwxofhmvf"

(and: "jqhscznljnhlxr", "rixhuvnazokxf", "rksoidrehnh", "lbnbyiqvta")

[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/page_number.rb:16:in 'Integer'
[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/page_number.rb:16:in 'initialize'
[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/page_number.rb:54:in 'new'
[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/page_number.rb:54:in 'PageNumber'
[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/active_record.rb:142:in 'page'
[GEM_ROOT]/gems/will_paginate-3.0.3/lib/will_paginate/active_record.rb:133:in 'paginate'

[hmaack]

best practice would be to check if your params id is a number and greater than 0, I also changed this in my fork and made a pull-reques

params[:page] = 'foobar'
posts = Post.where(:published => true).page(params[:page])
#posts.current_page = 1

# set default_page if necessary 
posts = Post.where(:published => true).page(params[:page], :default_page => 10)
#posts.current_page = 10
# this raises no exception, it trys to return the default_page

have a look at #276

[mislav]

The exception is deliberate. However, in production will_paginate configures Rails to swallow these exceptions and return 404s. You shouldn't be seening those in the exception tracker.

If you see these exceptions in production, tell me your Rails version and output of rake middleware

[kaleemullah]

@mislav I'm seeing this exception in production also.
Rails Version: 3.1.10

Exception:

invalid value for Integer(): "6/"
will_paginate (3.0.3) lib/will_paginate/page_number.rb:16:in Integer' will_paginate (3.0.3) lib/will_paginate/page_number.rb:16:ininitialize'
will_paginate (3.0.3) lib/will_paginate/page_number.rb:54:in new' will_paginate (3.0.3) lib/will_paginate/page_number.rb:54:inPageNumber'
will_paginate (3.0.3) lib/will_paginate/active_record.rb:142:in page' will_paginate (3.0.3) lib/will_paginate/active_record.rb:133:inpaginate'

[mislav]

OK, my bad. will_paginate just configures Rails to display a 404 for that exception in production, but the exception is still thrown internally, and exception trackers will log it.

I'll have to rethink this for future versions

[j15e]

Hi there,

I got this issue in production on website with a lot of traffic (always get strange wrong queries).

I think a quick & clean solution can be to add a rails route constraint to avoid hitting your controller with wrong values (and yeah otherwise it should raise, but maybe a custom exception would be nicer to catch it & render a 404. Oh, seems like it now the case, nvm.).

Anyway, here it goes for my solution on the routes level :

# lib/will_paginate_constraint.rb
class WillPaginateConstraint
  def matches?(request)
    request.params[:page].nil? || /^\d+$/ =~ request.params[:page]
  end
end

# config/routes.rb
match '/', :action => :index, :controller => :publications,
  :constraints => WillPaginateConstraint.new

[ericboehs]

Any update on this? It's been a todo for a year and a half. I'm still seeing this logged in our production exception logger. I'd ignore it in my exception tracker but it's an exception coming from ruby and not will paginate.

[sintro]

What if I want to handle the InvalidPage exception manually in my ApplicationController? As I can assume now, it is handled by middleware and outputs the 404 static page, without any possibility to override this behavior (without monkeypatching of gem`s source).

[thebravoman]

After duplicating this in #626 and thinking about a PR I am wondering how have you approached it:

Is rails constraints the preferred approach to filter the params or adding logic to ApplicationController or should it be the will_paginate that understands about pagination?

[mislav]

It's an interesting problem; one I always felt is better solved in the app itself.

1. will_paginate throws an exception on invalid input. By itself, that behavior should not be surprising for any library;
2. Exception trackers log every exception in the app. So far so good;
3. will_paginate exceptions get logged for garbage input for ?page=... parameter. This itself is not very useful, but I do not know how to avoid it. I'm not sure I want to default to page=1 for garbage input.

If someone has ideas/experience addressing this, please share!

[thebravoman]

Other params are checked in the model.
It makes sense to check all params in the model.

ActionModel allows the validation of params in the model. The controllers only check that they exist.
will_paginate also validates the params in the model. We are calling .paginate(page: params[:page]) the same way we are calling .new(name: params[:name]).

But will_paginate does half the job - only validates stricktly, but does not allow the configuration of the validation the same way ActiveModel allows.
I think it will be ok for will_paginate to throw an error by default.
But it is ok for a library working with the model and accepting params from the controller to allow for the validation of these params. One thing that would happen is that the controllers will become library neutral. Also it will be consistent with how other params are validated - eg in the model.

[denzelem]

In Rails applications I see often (simplified) code like this:

class UsersController < ApplicationController
  def index
    @users = User.paginate(page: params[:page])
  end
end

Since the .paginate Method is mixed into ActiveRecord directly there is no obvious clean way to intercept this chain. I've seen people monkey patching the paginate [1] method or sanitizing the page param [2].

IMHO raising any kind of error in a scope will not improve the quality of code compared to [1] and [2]:

class UsersController < ApplicationController
  def index
    @users = User.paginate(page: params[:page]).order(:created_at)
  rescue ArgumentError
    @users = User.order(:created_at)
  end
end

And there are more params that might be used directly in the controller e.g. per_page and total_entries that share the same underlying issue.

I will drop some thoughts here, how this could be addressed:

• Cast integer attributes and set them to nil if not a number
• Add a config to allow cast integer attributes, which defaults to .to_i for backward compatibility
• Extend potential public facing methods e.g. .paginate to have some extra flag attribute to cast / sanitize numbers

I'm happy to see if we can find an option @mislav likes to add to will_paginate.

---

[1]

module WillPaginateActiveRecordPaginationExtension
  def paginate(options)
    options[:page] = Integer(options[:page], exception: false)

    super
  end
end

WillPaginate::ActiveRecord::Pagination.prepend(WillPaginateActiveRecordPaginationExtension)

[2]

class ApplicationController < ActionController::Base
  private
  
  def sanitized_page
    Integer(params[:page], exception: false)
  end
end

class UsersController < ApplicationController
  def index
    @users = User.paginate(page: sanitized_page)
  end
end