Conversation
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
|
/backport to stable5.7 |
There was a problem hiding this comment.
Pull request overview
Fixes a Nextcloud Mail crash caused by passing an empty HTML string into DOMDocument::loadHTML() during phishing link checking (issue #12674) by short-circuiting link detection on empty messages.
Changes:
- Add an early return in
LinkCheck::run()when the HTML message is empty to avoid parsing. - Refactor result creation into a shared
generateResult()helper. - Add a unit test to ensure empty messages do not trigger phishing detection (and do not crash).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/Service/PhishingDetection/LinkCheck.php |
Skips DOM parsing for empty input and consolidates result construction. |
tests/Unit/Service/Phishing/LinkCheckTest.php |
Adds coverage to ensure empty HTML input is handled safely. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $results = []; | ||
| $zippedArray = []; | ||
|
|
||
| if (empty($htmlMessage)) { |
There was a problem hiding this comment.
empty($htmlMessage) treats the string "0" as empty and will skip link checking in that (non-empty) case. Since the goal is to avoid calling the HTML parser on truly empty/blank content, consider using a stricter check like $htmlMessage === '' (or trim($htmlMessage) === '' if whitespace-only messages should also be skipped) to avoid surprising behavior.
| if (empty($htmlMessage)) { | |
| if (trim($htmlMessage) === '') { |
There was a problem hiding this comment.
True, but 0 is not a valid url/link either ;)
Fix #12674