Skip to content

Set MailRoom's postback request content type to text/plain

Quang-Minh Nguyen requested to merge qmnguyen0711/skip-rails-parameter-parsing into master

What does this MR do and why?

For https://gitlab.com/gitlab-org/gitlab/-/issues/362068

When investigating gitlab-com/gl-infra/production#7029 (closed), we found out that MailRoom content was delivered as application/x-www-form-urlencoded content type. There are some problems with this configuration:

  • This content type is not technically correct. The content is not encoded as form url encoded.
  • Performance issue. Rails tries to parse the body to put to params inside the controller. At a later stage, the mailroom endpoint reads the email content directly from request body and ignores this params.
  • Rails automatically logs params keys and values. This behavior is a potential security risk.

This content type is a configuration that MailRoom gem does not set by default: https://github.com/tpitale/mail_room/blob/14e5bccac93493ad3e84d56140ee172974365dcf/lib/mail_room/delivery/postback.rb#L98. As a result, the content-type header defaults to Faraday's default content type. We should submit an upstream patch to fix this issue permanently. However, it would take several iterations until the patch lands in our production. Therefore, I updated the configuration set inside Rails application and CNG helm chart. We'll submit a patch later when we have some spare time.

Screenshots or screen recordings

Before

  • Sending a request to the internal endpoint with authentication disabled:
 curl -X POST \
  -d @spec/fixtures/emails/commands_in_reply.eml \
  -v \
  "http://localhost:3000/api/v4/internal/mail_room/incoming_email"
  • The logs shows params key and value:
{"time":"2022-05-23T03:51:50.000Z","severity":"INFO","duration_s":0.0,"db_duration_s":0.0,"view_duration_s":0.0,"status":400,"method":"POST","path":"/api/v4/internal/mail_room/incoming_email","params":[{"key":"Return-Path: <alan@adventuretime.ooo>\nReceived: from iceking.adventuretime.ooo (","value":{"unix socket":{") by iceking (Cyrus v2.2.13-Debian-2.2.13-19 squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400\nReceived: from mail-ie0-x234.google.com (mail-ie0-x234.google.com ":{"IPv6:2607:f8b0:4001:c03::234":{") by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version":"TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400\nReceived: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming email-test-project_id-issue-@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700\nReceived: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700\nDate: Thu, 13 Jun 2013 17:03:48 -0400\nFrom: Jake the Dog <alan@adventuretime.ooo>\nTo: incoming email-test-project_id-issue-@appmail.adventuretime.ooo\nMessage-ID: <CAH_Wr rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>\nIn-Reply-To: <CADkmRc rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>\nSubject: The message subject! @all\nMime-Version: 1.0\nContent-Type: text/plain;\n charset=ISO-8859-1\nContent-Transfer-Encoding: 7bit\nX-Sieve: CMU Sieve 2.2\nX-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,\n 13 Jun 2013 14:03:48 -0700 (PDT)\nX-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1\n\nService desk reply!\n\n/label ~label2\n"}}}}}]}
  • Putting a breakpoint inside the controller, the params variable has two keys, one is half-baked params
[3] pry(main)> params
=> {"Return-Path: <alan@adventuretime.ooo>\nReceived: from iceking.adventuretime.ooo ("=>"...", "mailroom_type"=>"incoming_email"}

After

  • Sending a request to the internal endpoint with authentication disabled, with/without content type
curl -X POST \
  -d @spec/fixtures/emails/commands_in_reply.eml \
  -H Content-Type:text/plain \
  -v \
  "http://localhost:3000/api/v4/internal/mail_room/incoming_email"

curl -X POST \
  -d @spec/fixtures/emails/commands_in_reply.eml \
  -v \
  "http://localhost:3000/api/v4/internal/mail_room/incoming_email"
  • The logs show no params value:
{"time":"2022-05-23T03:51:53.000Z","severity":"INFO","duration_s":0.0,"db_duration_s":0.0,"view_duration_s":0.0,"status":200,"method":"POST","path":"/api/v4/internal/mail_room/incoming_email","params":[],"host":"www.example.com","remote_ip":"127.0.0.1","ua":null,"route":"/api/:version/internal/mail_room/*mailbox_type"}
  • Putting a breakpoint inside the controller, the params variable has only one value:
[3] pry(main)> params
=> {"mailroom_type"=>"incoming_email"}

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