From 797ab30d9524ace142fbf39f70492568b9fa5533 Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Tue, 2 Apr 2024 15:35:44 -0500 Subject: [PATCH] DEV: Modifier to add params to TopicsController redirect url (#26470) --- app/controllers/topics_controller.rb | 14 ++++++++++++- spec/requests/topics_controller_spec.rb | 28 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 9b90d47818e..087dc15a6ef 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -1255,7 +1255,19 @@ class TopicsController < ApplicationController raise(SiteSetting.detailed_404 ? ex : Discourse::NotFound) end - opts = params.slice(:page, :print, :filter_top_level_replies, :preview_theme_id) + # Allow plugins to append allowed query parameters, so they aren't scrubbed on redirect to proper topic URL + additional_allowed_query_parameters = + DiscoursePluginRegistry.apply_modifier( + :redirect_to_correct_topic_additional_query_parameters, + [], + ) + + opts = + params.slice( + *%i[page print filter_top_level_replies preview_theme_id].concat( + additional_allowed_query_parameters, + ), + ) opts.delete(:page) if params[:page] == 0 url = topic.relative_url diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 1cd6db32c1a..9463d33151b 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2317,6 +2317,12 @@ RSpec.describe TopicsController do expect(response).to redirect_to("#{topic.relative_url}/42?page=1") end + it "scrubs invalid query parameters when redirecting" do + get "/t/#{topic.slug}", params: { silly_param: "hehe" } + + expect(response).to redirect_to(topic.relative_url) + end + it "returns 404 when an invalid slug is given and no id" do get "/t/nope-nope.json" @@ -2386,6 +2392,28 @@ RSpec.describe TopicsController do expect(second_request_queries.count).to eq(first_request_queries.count) end + context "with registered redirect_to_correct_topic_additional_query_parameters" do + let(:modifier_block) { Proc.new { |allowed_params| allowed_params << :silly_param } } + + it "retains the permitted query param when redirecting" do + plugin_instance = Plugin::Instance.new + plugin_instance.register_modifier( + :redirect_to_correct_topic_additional_query_parameters, + &modifier_block + ) + + get "/t/#{topic.slug}", params: { silly_param: "hehe" } + + expect(response).to redirect_to("#{topic.relative_url}?silly_param=hehe") + ensure + DiscoursePluginRegistry.unregister_modifier( + plugin_instance, + :redirect_to_correct_topic_additional_query_parameters, + &modifier_block + ) + end + end + context "when a topic with nil slug exists" do before do nil_slug_topic = Fabricate(:topic)