Skip to content

Guard email receiver from empty From and To fields

Quang-Minh Nguyen requested to merge qmnguyen0711/364326-nomethoderror-undefined-method-find-for-nil-nilclass into master

What does this MR do and why?

For #364326 (closed)

We noticed the following exception when watching the mailroom logs and Sentry. This exception is raised when processing incoming emails and service desk emails. As a result, we would loose user emails.

NoMethodError: undefined method `find' for nil:NilClass

NoMethodError: undefined method `find' for nil:NilClass
  lib/gitlab/email/receiver.rb:81:in `key_from_to_header'
    mail.to.find do |address|
  lib/gitlab/email/receiver.rb:76:in `block in mail_key'
    key_from_to_header || key_from_additional_headers
  lib/gitlab/utils/strong_memoize.rb:30:in `strong_memoize'
    instance_variable_set(key, yield)
  lib/gitlab/email/receiver.rb:75:in `mail_key'
    strong_memoize(:mail_key) do
  lib/gitlab/email/receiver.rb:62:in `find_handler'
    Handler.for(mail, mail_key)
...
(97 additional frame(s) were not displayed)

The root cause of this exception is due to To field of an email could be blank. It's a surprise that email RFC allows this absence. A To field could be blank when CC or BCC fields are set. So, one scenario leading this bug is when a user forward an email, and bcc GitLab's incoming email or service desk email. Please see the steps of reproduction in this issue.

How to set up and validate locally

This MR attaches some tests to very this case particularly.

Before

Screen_Shot_2022-06-04_at_16.26.19

Screen_Shot_2022-06-04_at_16.31.29

After

Both of those tests pass.

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