Commit Graph

3 Commits

Author SHA1 Message Date
b3881d42d0 DEV: Disallow the use of Rails.logger= in RSpec tests (#31920)
Setting Rails.logger after the application has been initialized does not
seem to be safe anymore and can lead to flaky tests. This commit
disallows the reassignment of `Rails.logger` going forward and updates
the affected test.

### Reviewer notes

Reassigning `Rails.logger` from within RSpec tests is causing tests
which uses `Rails.logger.broadcast_to(FakeLogger.new)` to flake.
Example:
https://github.com/discourse/discourse/actions/runs/13951116847/job/39050616967

```
  1) invalid requests handles NotFound with invalid json body
     Failure/Error: expect(fake_logger.errors).to have_attributes(size: 1)
     
       expected [] to have attributes {:size => 1} but had attributes {:size => 0}
       Diff:
       @@ -1 +1 @@
       -:size => 1,
       +:size => 0,
       
     # ./spec/integration/invalid_request_spec.rb:18:in `block (2 levels) in <main>'
     # ./spec/rails_helper.rb:619:in `block (3 levels) in <top (required)>'
     # /var/www/discourse/vendor/bundle/ruby/3.3.0/gems/benchmark-0.4.0/lib/benchmark.rb:304:in `measure'
     # ./spec/rails_helper.rb:619:in `block (2 levels) in <top (required)>'
     # ./spec/rails_helper.rb:580:in `block (3 levels) in <top (required)>'
     # /var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
     # /var/www/discourse/vendor/bundle/ruby/3.3.0/gems/timeout-0.4.3/lib/timeout.rb:192:in `timeout'
     # ./spec/rails_helper.rb:570:in `block (2 levels) in <top (required)>'
     # ./spec/rails_helper.rb:527:in `block (2 levels) in <top (required)>'
     # /var/www/discourse/vendor/bundle/ruby/3.3.0/gems/webmock-3.25.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
2025-03-21 08:48:38 +08:00
305039b1c3 DEV: ensure Rails application default headers are present in responses (#31619)
Follow up from https://github.com/discourse/discourse/pull/31559.

We expect some standard headers to be added from
`Rails.application.config.action_dispatch.default_headers` for
responses, however these were found to be removed in some error paths. 
For more detail on this behaviour, refer to https://github.com/discourse/discourse/pull/31619#issuecomment-2699644232.

This PR adds those headers back if they aren't there, with the caveats
that we don't add headers that are irrelevant for non-HTML responses,
and neither do we add X-Frame-Options which is intentionally removed for
embeddables.
2025-03-05 13:19:09 +08:00
0d90f6e3c3 FIX: cross origin opener policy should apply to public error responses (#31559)
In some error paths, headers that were set earlier can get overwritten
(e.g. `Cross-Origin-Opener-Policy`) by middleware such as
ActionDispatch::ShowExceptions.

This PR sets the `Cross-Origin-Opener-Policy` header to the value of the
SiteSetting `cross_origin_opener_policy_header` if it's missing and if
the response is for HTML.

In future, this DefaultHeaders middleware can be used to set other
default headers that relate to security or other purposes.

### Testing
<img width="631" alt="test"
src="https://github.com/user-attachments/assets/05106a40-2bc7-435d-91a2-4dd2a098f349"
/>
2025-03-03 17:04:24 +08:00