Page MenuHomePhorge
Differential D25275

fix strlen in PhabricatorMailManagementShowOutboundWorkflow
ClosedPublic
Actions

Authored by avivey on Jun 3 2023, 16:30.

Details

Reviewers
valerio.bozzolan
Group Reviewers
O1: Blessed Committers
Maniphest Tasks
T15064: Make Phorge compatible with PHP 8.1/8.2/8.3/8.4
Commits
rP9aafbb42ef8d: fix strlen in PhabricatorMailManagementShowOutboundWorkflow
Summary

Ref T15064

Test Plan

run ./bin/mail show-outbound --id 1 on a non-html email with php 8.1, no crash.

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

avivey created this revision.Jun 3 2023, 16:30
Owners added a reviewer: O1: Blessed Committers.Jun 3 2023, 16:30
Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJun 3 2023, 16:30
avivey requested review of this revision.Jun 3 2023, 16:30
Harbormaster completed remote builds in B509: Diff 883.Jun 3 2023, 16:30
valerio.bozzolan accepted this revision.Jun 3 2023, 17:11

Nice

Followed the test plan.

Maybe we can add a small PHPDoc in PhabricatorMetaMTAMail::getHTMLBody() to make it clear that it @return string|null

This revision is now accepted and ready to land.Jun 3 2023, 17:11
speck added inline comments.Jun 3 2023, 19:01
src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php
68

This one probably also needs updated I'm guessing.

191

Looks like this might be nullable too

avivey planned changes to this revision.Jun 24 2023, 19:22
Cigaryno added a project: PHP 8 support.Jun 28 2023, 21:04
avivey updated this revision to Diff 1036.Jun 30 2023, 08:23

more cases

This revision is now accepted and ready to land.Jun 30 2023, 08:23
Harbormaster completed remote builds in B611: Diff 1036.Jun 30 2023, 08:23
avivey added inline comments.Jun 30 2023, 08:24
src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php
77

The default for most args is "null", so it was crashing here. This was just a default-check anyway,

valerio.bozzolan added inline comments.Jun 30 2023, 08:26
src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php
77

Yep. Also I wonder why they have not used also $args->getArg('type', $something_else); before.

Closed by commit rP9aafbb42ef8d: fix strlen in PhabricatorMailManagementShowOutboundWorkflow. · Explain WhyJul 2 2023, 15:49
This revision was automatically updated to reflect the committed changes.
avivey added a commit: rP9aafbb42ef8d: fix strlen in PhabricatorMailManagementShowOutboundWorkflow.
avivey mentioned this in D25256: T15064: PHP8 related corrections.Jul 11 2023, 07:41

Revision Contents
Changeset List

Diff 1046

View Options

src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php

Loading...
View Options

src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php

Loading...
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0