Skip to content

Fix grabUrlsFromLastEmail for HTML entities and single-part emails - #139

Merged
Jamesking56 merged 2 commits into
captbaritone:masterfrom
wazum:bugfix/137-138-grab-urls
Oct 2, 2026
Merged

Jamesking56 merged 2 commits into
captbaritone:masterfrom
wazum:bugfix/137-138-grab-urls

Conversation

@wazum

@wazum wazum commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Resolves: #137, #138

Rebased onto current master (after #130, #136 and #140). There are two commits, one per issue.

#137: & in URLs from the HTML part

The URL regex runs over raw HTML markup, so href="…?id=1&token=abc" came back with a literal &. The application then never received token. grabUrlsFromEmail() now entity-decodes URLs from the HTML part, which fixes both grabUrlsFromLastEmail() and grabUrlsFromNthEmail(). URLs from the text part are left untouched, because entities have no meaning in plain text. The order and number of returned URLs are unchanged, since callers index into this array.

Tests:

  • Unit: testGrabUrlsFromLastEmailDecodesHtmlEntities. It fails on current master with …?id=1&token=abc.
  • Acceptance: test_grab_urls_from_html_email_with_multiple_query_parameters against a real MailCatcher.

#138: null passed to preg_match_all()

The null guard itself already reached master with #130, when the URL code moved into grabUrlsFromEmail(). This commit adds the missing regression test, testGrabUrlsFromSinglePartEmailsRaisesNoDeprecation. It covers a plain-text-only and an HTML-only email, and turns notices raised from src/ into failures. Notices from dependencies are ignored, because mail-mime-parser 3.0.1 raises its own deprecation under --prefer-lowest.

Both suites pass locally on PHP 8.5 with the latest dependencies and on PHP 8.3 with --prefer-lowest (109 tests).

@github-actions
github-actions Bot requested a review from Jamesking56 October 1, 2026 09:24
@github-actions github-actions Bot added the bug label Oct 1, 2026
@wazum
wazum force-pushed the bugfix/137-138-grab-urls branch from f64f2d9 to a22b4f3 Compare October 1, 2026 09:27
@Jamesking56

Copy link
Copy Markdown
Collaborator

@wazum Please resolve the conflicts

@Jamesking56 Jamesking56 added this to the 4.0.1 milestone Oct 1, 2026
wazum added 2 commits October 1, 2026 19:14
mail-mime-parser returns null for a missing text or HTML part, which used
to be passed to preg_match_all() and raised a deprecation notice. The
null guard landed with captbaritone#130; this test keeps it from regressing.

Resolves: captbaritone#138
The URL regex runs over raw HTML markup, so an href written as
"?id=1&token=abc" came back with the literal "&" and the
second query parameter was lost when the URL was opened.

Resolves: captbaritone#137
@wazum
wazum force-pushed the bugfix/137-138-grab-urls branch from a22b4f3 to 45df715 Compare October 1, 2026 17:15
@wazum

wazum commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@Jamesking56 Rebased onto current master, so the conflicts are resolved. The entity decoding now lives in the shared grabUrlsFromEmail() helper, so grabUrlsFromNthEmail() is fixed as well. The #138 null guard had already landed with #130, so that commit now only adds the regression test. I updated the description to match.

@Jamesking56
Jamesking56 merged commit d96ed2c into captbaritone:master Oct 2, 2026
10 checks passed
@Jamesking56

Copy link
Copy Markdown
Collaborator

Thanks @wazum

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Released in 4.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

grabUrlsFromLastEmail returns HTML-escaped URLs (&) from HTML emails

2 participants