Page MenuHomePhabricator

Drop empty inbound mail at the beginning of the receive workflow, not inside object handlers
ClosedPublic

Authored by epriestley on Jan 3 2019, 2:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 3:15 PM
Unknown Object (File)
Thu, Apr 11, 7:24 AM
Unknown Object (File)
Sat, Mar 30, 7:17 AM
Unknown Object (File)
Fri, Mar 29, 1:04 AM
Unknown Object (File)
Fri, Mar 22, 9:57 PM
Unknown Object (File)
Fri, Mar 22, 9:57 PM
Unknown Object (File)
Fri, Mar 22, 9:57 PM
Unknown Object (File)
Fri, Mar 22, 9:57 PM
Subscribers
None

Details

Summary

Ref T920. Ref T7477. We currently drop empty mail only once it reaches the ReplyHandler layer.

I think no plausible receiver can ever do anything useful with this kind of mail, so we can safely drop it earlier and simplify some of the logic. After T7477, we'd end up throwing multiple exceptions if you sent empty mail to several valid receivers.

(I also want to move away from APIs oriented around raw addresses in more specialized layers, and this is one of the few callsites for raw mail address information.)

This requires updating some unit tests to actually have message bodies, since they failed with this error before hitting the other errors otherwise.

Test Plan

Used bin/mail receive-test to send empty mail, got appropriate "err:empty" out of it.

Diff Detail

Repository
rP Phabricator
Branch
mfa7
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21471
Build 29244: Run Core Tests
Build 29243: arc lint + arc unit

Unit TestsFailed

TimeTest
244 msPhabricatorObjectMailReceiverTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at PhabricatorObjectMailReceiverTestCase.php:75). Expected: 'err:bad-hash' Actual: 'err:empty'
122 msPhabricatorObjectMailReceiverTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at PhabricatorObjectMailReceiverTestCase.php:23). Expected: 'err:no-public-mail' Actual: 'err:empty'
240 msPhabricatorObjectMailReceiverTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at PhabricatorObjectMailReceiverTestCase.php:53). Expected: 'err:not-found' Actual: 'err:empty'
235 msPhabricatorObjectMailReceiverTestCase::Unknown Unit Message ("")
Assertion failed, expected values to be equal (at PhabricatorObjectMailReceiverTestCase.php:64). Expected: 'err:bad-user' Actual: 'err:empty'
1 msAlmanacNamesTestCase::Unknown Unit Message ("")
30 assertions passed.
View Full Test Results (4 Failed · 367 Passed)

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 3 2019, 2:29 PM
Harbormaster failed remote builds in B21471: Diff 47618!
  • Fix more unit tests, including a "TODO" from May 2013!
This revision is now accepted and ready to land.Jan 3 2019, 8:35 PM
This revision was automatically updated to reflect the committed changes.