Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not automatically intercepting in Rails any longer #110

Closed
GUI opened this issue Jun 10, 2024 · 5 comments
Closed

Not automatically intercepting in Rails any longer #110

GUI opened this issue Jun 10, 2024 · 5 comments

Comments

@GUI
Copy link

GUI commented Jun 10, 2024

After upgrading from sanitize_email 2.0.4 to 2.0.6, it looks like e-mails have stopped being intercepted in a Rails 7.1 app by default. If I manually call Mail.register_interceptor(SanitizeEmail::Bleach), this does seem to fix the interception in the app, but I think something is maybe amiss with the new Engine integration for Rails 6+ in 2.0.6:

v2.0.4...v2.0.6#diff-c6ad5f4fb841393b5d4fee8dac9a3be490baf334851b4cae3adcb90c1096a4e2
v2.0.4...v2.0.6#diff-6b14c6f0901cbec9229126c0ae73f28b72421a354fc0aead148fa406f5f665ca

I think the issue is maybe related to the new EngineV6 calling:

Rails.application.config.action_mailer.register_interceptor(SanitizeEmail::Bleach)

I believe the issue is that register_interceptor may not actually be doing anything in this case. At least in the Rails 7.1 app I'm testing, Rails.application.config.action_mailer is actually an instance of ActiveSupport::OrderedOptions, so this is more like a hash that responds to method calls as hash lookups too. But that means that I don't think register_interceptor() is actually calling any type of underlying method. Instead, I think it's just looking for a :register_interceptor attribute on the hash of mailer config data, and returning nil since it doesn't exist.

So I think this means that calling Rails.application.config.action_mailer.register_interceptor(SanitizeEmail::Bleach) doesn't actually seem to ever call the underlying ActionMailer::Base.register_interceptor or Mail.register_interceptor methods. You can see how any method passed to action_mailer will not raise an exception, but I don't believe it's actually calling anything (I think it's just acting more like a hash accessor). This differs from how a real call to ActionMailer::Base.register_interceptor will react:

[1] pry(main)> Rails.application.config.action_mailer.register_interceptor(SanitizeEmail::Bleach)
=> nil
[2] pry(main)> Rails.application.config.action_mailer.register_interceptor
=> nil
[3] pry(main)> Rails.application.config.action_mailer.register_interceptor_foo(SanitizeEmail::Bleach)
=> nil
[4] pry(main)> ActionMailer::Base.register_interceptor
ArgumentError: wrong number of arguments (given 0, expected 1)
from /dev-cache/usr/local/bundle/gems/actionmailer-7.1.3.4/lib/action_mailer/base.rb:548:in `register_interceptor'

So I think this might mean that this Engine for Rails 6+ isn't actually setting the interceptor by default in version 2.0.6. It sounds like maybe a deprecation warning is the reason for the new approach in Rails 6+, but for whatever it's worth I don't actually see any deprecation warnings when using sanitize_email 2.0.4 under Rails 7.1 (but I realize I might not be exercising the right thing, since there's probably a lot of different possible app setups).

Thanks!

@pboling
Copy link
Owner

pboling commented Jun 18, 2024

Interesting. Thanks for the report. I'll get it fixed ASAP.

@pboling
Copy link
Owner

pboling commented Jun 18, 2024

Turns out part of the Rails documentation was bad.
rails/rails#42170

Fixing.

@pboling
Copy link
Owner

pboling commented Jun 18, 2024

Fix released in 2.0.7. Please give it a try @GUI

@anthonyklue
Copy link

We're still experiencing this issue in version 2.0.7 & rails 7.1.3.4

Changing the code in engine_v6.rb to

    ActiveSupport.on_load(:action_mailer) do
      ActionMailer::Base.register_interceptor(SanitizeEmail::Bleach)
    end

as suggested here appears to solve the problem

@pboling pboling reopened this Jul 7, 2024
@pboling
Copy link
Owner

pboling commented Jul 10, 2024

Fix is released in v2.0.8 @anthonyklue @GUI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants