Make incoming mail handling more robust / correct
ClosedPublic
Actions

Authored by aklapper on Apr 1 2024, 10:32.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15769: Make handling incoming mail a bit more robust/correct

Commits

rP644f179dd27a: Make incoming mail handling more robust / correct

Summary

Properly handle when no mail headers at all can be parsed
Properly handle when mail headers can be parsed but no subject line can be found

EXCEPTION: (RuntimeException) Undefined index: subject

Closes T15769

Test Plan

See T15769

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Apr 1 2024, 10:32

Owners added a reviewer: O1: Blessed Committers.Apr 1 2024, 10:32

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptApr 1 2024, 10:32

aklapper requested review of this revision.Apr 1 2024, 10:32

Harbormaster completed remote builds in B1135: Diff 1802.Apr 1 2024, 10:33

valerio.bozzolan added inline comments.Apr 3 2024, 09:55

externals/mimemailparser/MimeMailParser.class.php
314	This is becoming a bit hacky now, since false is probably never returned anymore. Maybe moving this exception to the consumer? I mean throwing in line 57 of mail_handler.php

aklapper added inline comments.Apr 3 2024, 10:55

externals/mimemailparser/MimeMailParser.class.php
314	In my understanding the return value `false` was not handled anywhere anyway, so I didn't mind. I was after throwing a correct error message instead of failing way later with a misleading error message. :)

20after4 subscribed.Wed, Apr 3, 17:12

20after4 added inline comments.

externals/mimemailparser/MimeMailParser.class.php
310–314	This is becoming a bit hacky now, since false is probably never returned anymore. should it be like this then?

Simplify logic (thanks Mukunda!)

Harbormaster completed remote builds in B1150: Diff 1820.Thu, Apr 4, 09:05

aklapper marked an inline comment as done.Thu, Apr 4, 09:05

sgtm

Go MoreMimeMailParserPeouw

This revision is now accepted and ready to land.Thu, Apr 4, 10:46

FTR I gave this last version another test locally, feeding an mbox file with no subject header (which is accepted now without complaints), and bugging input again (which prints a correct error message instead of misleadingly complaining about a missing subject line)

Closed by commit rP644f179dd27a: Make incoming mail handling more robust / correct. · Explain WhyThu, Apr 4, 20:20

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rP644f179dd27a: Make incoming mail handling more robust / correct.