Skip to content

Prevent frozen modification in email encoding patch

What does this MR do and why?

For #364619 (closed)

FrozenError: can't modify frozen String: ""
  config/initializers/mail_encoding_patch.rb:62:in `force_encoding'
    decoded = decoded.force_encoding(charset)
  config/initializers/mail_encoding_patch.rb:62:in `encoded'
    decoded = decoded.force_encoding(charset)
  config/initializers/mail_encoding_patch.rb:80:in `encoded'
    buffer << body.encoded(content_transfer_encoding, charset)
  lib/gitlab/email/failure_handler.rb:39:in `handle'
    EmailRejectionMailer.rejection(reason, receiver.mail.encoded, can_retry).deliver_later
  app/workers/email_receiver_worker.rb:93:in `handle_failure'
    Gitlab::Email::FailureHandler.handle(receiver, error)

image Source: https://log.gprd.gitlab.net/goto/148ae0d0-e6f3-11ec-8656-f5f2137823ba

We noticed a wave of FrozenError when fixing some other exceptions related to incoming emails and service desk emails. This exception occurs inside failure handler. It prevents users from receiving rejection notification when their emails are rejected. It's not a data loss incident, because the emails are rejected anyway. Instead, it adds some annoyances to users when they are not informed about those failures to try again later.

After investigation, we found that the root cause of this exception comes from our money patch on top of Mail::Message. Before sending email rejection emails, we modify the email's body to nil, then translated to a frozen empty string by the Mail gem. Our patch modifies that string, leading to that this issue. The full investigation can be found here: #364619 (closed)

How to set up and validate locally

I added one test to cover this case. Before the fix, the test raise this exception. Afterward, the test is green.

Screen_Shot_2022-06-10_at_17.10.36

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports