From bfad9a7170e695c98de074e6ca0fd8a51d81e78a Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 29 Aug 2024 14:22:42 +0800 Subject: [PATCH] DEV: Gracefully handle an array of IDs passed to Topics#show (#28631) We're seeing a lot of log noise coming from unhandled exceptions stemming from requests to TopicsController#show where id is passed in as an array. In the implementation of the method, we assume that if id is present it will be a string. This is because one of the routes to this action uses :id as a URL fragment, and so must be a string. However, there are other routes that go to this endpoint as well. Some of them don't have this URL fragment, so you can pass an arbitrary id query parameter. Instead of a downstream unhandled exception, we raise a Discourse::InvalidParameters upfront. --- app/controllers/topics_controller.rb | 4 ++++ spec/requests/topics_controller_spec.rb | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index b5d910a8259..aca86dd57c0 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -51,6 +51,10 @@ class TopicsController < ApplicationController end def show + if params[:id].is_a?(Array) + raise Discourse::InvalidParameters.new("Show only accepts a single ID") + end + flash["referer"] ||= request.referer[0..255] if request.referer # TODO: We'd like to migrate the wordpress feed to another url. This keeps up backwards diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 66af5a01982..94eed15ed3b 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2322,6 +2322,12 @@ RSpec.describe TopicsController do expect(response).to redirect_to(topic.relative_url) end + it "does not raise an unhandled exception when receiving an array of IDs" do + get "/t/#{topic.id}/summary?id[]=a,b" + + expect(response.status).to eq(400) + end + it "keeps the post_number parameter around when redirecting" do get "/t/#{topic.slug}", params: { post_number: 42 } expect(response).to redirect_to(topic.relative_url + "/42")