From b64a58071d4430b155acdc7cff96bfad53ee34de Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 23 Feb 2024 07:51:51 +0800 Subject: [PATCH] DEV: Ensure that `BlockRequestsMiddleware` cookie is always set (#25826) Why this change? This reverts 725561cf4bddeda0a5cf2f68ed7b76e0c9deead8 as it did not address the root cause of the problem even though it fixed the failing tests we were seeing when running `bundle exec rspec --tag ~type:multisite --order random:776 spec/system/admin_customize_form_templates_spec.rb spec/system/admin_sidebar_navigation_spec.rb spec/system/admin_site_setting_search_spec.rb spec/system/composer/dont_feed_the_trolls_popup_spec.rb spec/system/composer/review_media_unless_trust_level_spec.rb spec/system/create_account_spec.rb spec/system/editing_sidebar_tags_navigation_spec.rb spec/system/email_change_spec.rb spec/system/emojis/emoji_deny_list_spec.rb spec/system/group_activity_spec.rb spec/system/hashtag_autocomplete_spec.rb spec/system/network_disconnected_spec.rb spec/system/post_menu_spec.rb spec/system/post_small_action_spec.rb spec/system/tags_intersection_spec.rb spec/system/topic_list_focus_spec.rb spec/system/topic_page_spec.rb spec/system/user_page/user_profile_info_panel_spec.rb spec/system/viewing_group_members_spec.rb spec/system/viewing_navigation_menu_preferences_spec.rb`. The root cause here is that `before_action`s added to a controller is order dependent. As such, some requests were not setting the cookie because the `before_action` callback was not even hit as a prior `before_action` callbacks has raised an error such as the `check_xhr` `before_action` callback. To resolve the problem, we need to add the `prepend: true` option in our monkey patch of `ApplicationController` to ensure that the `before_action` callback which we have added is always run first. This change also makes a couple of changes: 1. Improve the response body when a request is blocked by the `BlockRequestsMiddleware` middleware so that it makes debugging easier. 2. Only set the cookies for non-xhr HTML format requests. Setting it for other formats is kind of pointless. --- config/initializers/200-first_middlewares.rb | 10 ++++++++-- spec/rails_helper.rb | 5 +++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/config/initializers/200-first_middlewares.rb b/config/initializers/200-first_middlewares.rb index e06b7b9f3d1..ed3e2dccf2a 100644 --- a/config/initializers/200-first_middlewares.rb +++ b/config/initializers/200-first_middlewares.rb @@ -58,12 +58,18 @@ if Rails.env.test? ( @@block_requests || ( - request.cookies[RSPEC_CURRENT_EXAMPLE_COOKIE_STRING].present? && + self.class.current_example_location.present? && self.class.current_example_location != request.cookies[RSPEC_CURRENT_EXAMPLE_COOKIE_STRING] ) ) - [503, { "Content-Type" => "text/plain" }, ["Blocked by BlockRequestsMiddleware"]] + [ + 503, + { "Content-Type" => "text/plain" }, + [ + "Blocked by BlockRequestsMiddleware for requests initiated by #{request.cookies[RSPEC_CURRENT_EXAMPLE_COOKIE_STRING]} when running #{self.class.current_example_location}", + ], + ] else @app.call(env) end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index af8f3d93dac..6bd575fe359 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -593,8 +593,9 @@ RSpec.configure do |config| ActiveRecord::Base.connection.schema_cache.add(table) end - ApplicationController.before_action do - if BlockRequestsMiddleware.current_example_location + ApplicationController.before_action(prepend: true) do + if BlockRequestsMiddleware.current_example_location && !request.xhr? && + request.format == "html" cookies[ BlockRequestsMiddleware::RSPEC_CURRENT_EXAMPLE_COOKIE_STRING ] = BlockRequestsMiddleware.current_example_location