Validation of params should restrict to max int (#6331)

* FIX: Validation of params should restrict to max int

* FIX: Send status 400 when "page" param isn't between 1 and max int
This commit is contained in:
Gerhard Schlager 2018-09-03 06:45:32 +02:00 committed by Sam
parent 747c9bb47f
commit f33433bf9e
3 changed files with 29 additions and 13 deletions

View File

@ -367,7 +367,6 @@ class ListController < ApplicationController
def build_topic_list_options def build_topic_list_options
options = {} options = {}
params[:page] = params[:page].to_i rescue 1
params[:tags] = [params[:tag_id].parameterize] if params[:tag_id].present? && guardian.can_tag_pms? params[:tags] = [params[:tag_id].parameterize] if params[:tag_id].present? && guardian.can_tag_pms?
TopicQuery.public_valid_options.each do |key| TopicQuery.public_valid_options.each do |key|

View File

@ -9,6 +9,7 @@ require_dependency 'topic_query_sql'
require_dependency 'avatar_lookup' require_dependency 'avatar_lookup'
class TopicQuery class TopicQuery
PG_MAX_INT ||= 2147483647
def self.validators def self.validators
@validators ||= begin @validators ||= begin
@ -17,8 +18,12 @@ class TopicQuery
Integer === x || (String === x && x.match?(/^-?[0-9]+$/)) Integer === x || (String === x && x.match?(/^-?[0-9]+$/))
end end
zero_or_more = lambda do |x| zero_up_to_max_int = lambda do |x|
int.call(x) && x.to_i >= 0 int.call(x) && x.to_i.between?(0, PG_MAX_INT)
end
one_up_to_max_int = lambda do |x|
int.call(x) && x.to_i.between?(1, PG_MAX_INT)
end end
array_int_or_int = lambda do |x| array_int_or_int = lambda do |x|
@ -28,8 +33,9 @@ class TopicQuery
end end
{ {
max_posts: zero_or_more, max_posts: zero_up_to_max_int,
min_posts: zero_or_more, min_posts: zero_up_to_max_int,
page: one_up_to_max_int,
exclude_category_ids: array_int_or_int exclude_category_ids: array_int_or_int
} }
end end

View File

@ -11,11 +11,6 @@ RSpec.describe ListController do
end end
describe '#index' do describe '#index' do
it "doesn't throw an error with a negative page" do
get "/#{Discourse.anonymous_filters[1]}", params: { page: -1024 }
expect(response.status).to eq(200)
end
it "does not return a 500 for invalid input" do it "does not return a 500 for invalid input" do
get "/latest?min_posts=bob" get "/latest?min_posts=bob"
expect(response.status).to eq(400) expect(response.status).to eq(400)
@ -28,6 +23,21 @@ RSpec.describe ListController do
get "/latest?exclude_category_ids[]=bob" get "/latest?exclude_category_ids[]=bob"
expect(response.status).to eq(400) expect(response.status).to eq(400)
get "/latest?max_posts=1111111111111111111111111111111111111111"
expect(response.status).to eq(400)
get "/latest?page=-1"
expect(response.status).to eq(400)
get "/latest?page=0"
expect(response.status).to eq(400)
get "/latest?page=2147483648"
expect(response.status).to eq(400)
get "/latest?page=1111111111111111111111111111111111111111"
expect(response.status).to eq(400)
end end
it "returns 200 for legit requests" do it "returns 200 for legit requests" do
@ -42,10 +52,11 @@ RSpec.describe ListController do
get "/latest.json?min_posts=0" get "/latest.json?min_posts=0"
expect(response.status).to eq(200) expect(response.status).to eq(200)
end
it "doesn't throw an error with page params as an array" do get "/latest?page=1"
get "/#{Discourse.anonymous_filters[1]}", params: { page: ['7'] } expect(response.status).to eq(200)
get "/latest.json?page=2147483647"
expect(response.status).to eq(200) expect(response.status).to eq(200)
end end