See T13065 and rP1e2bc7775bc4.
Intended to cover the gap left by D25899.
The actual amount of impacted installations should
be 0, but the migration makes sure all pastes have
a mail key regardless.
Details
Details
- Reviewers
aklapper - Group Reviewers
O1: Blessed Committers - Commits
- rP9c388c65f628: Remove the onboard "mailKey" from Paste
- Create a paste
- Check SELECT * FROM phabricator_paste.paste; and SELECT * FROM phabricator_metamta.metamta_mailproperties; on the database
- Run ./bin/storage upgrade
- Check that the mailKeys have been moved to mailProperties in the phabricator_metamta.metamta_mailproperties table and the mailKey column is gone from the phabricator_paste.paste table
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- migrate-paste-to-mailkey-table
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 1753 Build 1753: arc lint + arc unit
Event Timeline
Comment Actions
You know better than me the context. Is there a possibility that an installation is already without that column?
If you think yes, maybe DROP COLUMN IF EXISTS
| resources/sql/autopatches/20250227.paste.02.mailkey.sql | ||
|---|---|---|
| 3 | I think the canonical way is DROP COLUMN | |
Comment Actions
Theoretically, yes, but in practice no, provided they ran storage upgrade. After a certain point, the deprecations in newer PHP versions would prevent you from running the migrations anyway. Afterwards, the paste table would have been renamed, breaking the upgrade entirely.
(Also I forgot to drop the column from PhabricatorPaste - whoops)
| resources/sql/autopatches/20250227.paste.02.mailkey.sql | ||
|---|---|---|
| 3 | I just copied from 20181220.pholio.02.dropmailkey.sql in rP1e2bc7775bc4. | |
Comment Actions
What is (or rather was, I guess) that mailKey thingie good for? Or more relevant, I assume it's intentional after performing the Test Plan steps above that creating a new Paste does not create a new row in phabricator_metamta.metamta_mailproperties?
Apart from that, seems to work as expected. :) Thanks!
Comment Actions
According to https://secure.phabricator.com/T13065: This key is used as a seed to generate unique addresses for each <object, user> pair so that I send mail to T123+1+abe2h3f2 and you send mail to T123+2+fo1m213n and I can't just fiddle with my "+1" and impersonate someone else.
Or more relevant, I assume it's intentional after performing the Test Plan steps above that creating a new Paste does not create a new row in phabricator_metamta.metamta_mailproperties?
Apart from that, seems to work as expected. :) Thanks!
I had to go dig for this, but from what I understand of looking at PhabricatorMetaMTAMailProperties, which is the class now responsible for this functionality, creating a mail key only happens when its loadMailKey method is called. That method is only called when email functionality engaged. That does make testing slightly more difficult. The updated test plan (thanks, by the way!) is a good compromise.
Comment Actions
Ah, thanks. That explains why phabricator_metamta.metamta_mailproperties had been empty for me locally.
Going through the code I ended up in PhabricatorMailReplyHandler::supportsPublicReplies() checking for $this->getReplyHandlerDomain() being set. So yeah, this all seems to make sense.