Paths

Table of Contentst

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

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

Branch

master

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 509
Build 509: arc lint + arc unit

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

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

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 ↗	(On Diff #1036)	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 ↗	(On Diff #1036)	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

Path

Size

Packages

src/

applications/

metamta/

management/

PhabricatorMailManagementShowOutboundWorkflow.php

2 lines

O1: Blessed Committers

Diff 883

View Options

src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php

fix strlen in PhabricatorMailManagementShowOutboundWorkflowClosedPublicActions