From fac71da605efc98740177d080ff5811ba7005a9d Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 3 Jan 2020 17:01:38 +0300 Subject: [PATCH] FIX: Don't give error 500 when invalid date param is given to admin reports (#8658) Providing invalid dates as the end_date or start_date param causes a 500 error and creates noise in the logs. This will handle the error and returns a proper 400 response to the client with a message that explains what the problem is. --- app/controllers/admin/reports_controller.rb | 8 ++++++-- spec/requests/admin/reports_controller_spec.rb | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 20b97a6f255..864512c451e 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -88,8 +88,12 @@ class Admin::ReportsController < Admin::AdminController private def parse_params(report_params) - start_date = (report_params[:start_date].present? ? Time.parse(report_params[:start_date]).to_date : 1.days.ago).beginning_of_day - end_date = (report_params[:end_date].present? ? Time.parse(report_params[:end_date]).to_date : start_date + 30.days).end_of_day + begin + start_date = (report_params[:start_date].present? ? Time.parse(report_params[:start_date]).to_date : 1.days.ago).beginning_of_day + end_date = (report_params[:end_date].present? ? Time.parse(report_params[:end_date]).to_date : start_date + 30.days).end_of_day + rescue ArgumentError => e + raise Discourse::InvalidParameters.new(e.message) + end facets = nil if Array === report_params[:facets] diff --git a/spec/requests/admin/reports_controller_spec.rb b/spec/requests/admin/reports_controller_spec.rb index fa1bf1f751e..1f3208746e5 100644 --- a/spec/requests/admin/reports_controller_spec.rb +++ b/spec/requests/admin/reports_controller_spec.rb @@ -47,6 +47,24 @@ describe Admin::ReportsController do expect(JSON.parse(response.body)["reports"][1]["type"]).to eq("not_found") end end + + context "invalid start or end dates" do + it "doesn't return 500 error" do + get "/admin/reports/bulk.json", params: { + reports: { + topics: { limit: 10, start_date: "2015-0-1" } + } + } + expect(response.status).to eq(400) + + get "/admin/reports/bulk.json", params: { + reports: { + topics: { limit: 10, end_date: "2015-0-1" } + } + } + expect(response.status).to eq(400) + end + end end end