fix: escape calendar item action link hrefs with esc_url() and link text with esc_html__()#1016
Conversation
…p; (esc_html encoding)
|
Reviewed — fix is correct, CI green. The escaping is right and the mutation-style tests (assert One scope question: the |
…et registrations Folds in the two remaining admin_url()-in-href spots flagged in review, closing out the escaping sweep for this module surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks @GaryJones — folded both in (63b1e7d). |
There was a problem hiding this comment.
Pull request overview
This PR hardens output escaping in the Calendar module’s per-item action links and in module “short_description” admin links, and adds integration coverage to prevent regressions in calendar item action link escaping.
Changes:
- Escape Calendar item action link
hrefvalues withesc_url()and action link text withesc_html__()inEF_Calendar::get_inner_information(). - Escape
admin_url()-based links embedded in the Calendar and Story Budget module short descriptions. - Add a new integration test (
CalendarEscapingTest) to asserthrefURLs are escaped (specifically, bare&becomes&).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
modules/calendar/calendar.php |
Escapes calendar action link URLs and link text; escapes module short-description URL. |
modules/story-budget/story-budget.php |
Escapes the short-description link URL with esc_url(). |
tests/Integration/CalendarEscapingTest.php |
New regression test for calendar action link URL escaping. |
CHANGELOG.md |
Adds Unreleased security notes documenting these escaping fixes. |
| add_filter( | ||
| 'post_link', | ||
| static function () { | ||
| return 'http://example.com/?p=1&preview=true'; | ||
| } | ||
| ); | ||
|
|
||
| ob_start(); | ||
| $edit_flow->calendar->get_inner_information( | ||
| $edit_flow->calendar->get_post_information_fields( $post ), | ||
| $post | ||
| ); | ||
| $html = ob_get_clean(); | ||
|
|
||
| remove_all_filters( 'post_link' ); | ||
|
|
| // Inject a URL with a bare & so we can assert esc_url() encoding. | ||
| // esc_url() converts & to & (numeric entity), not &. | ||
| add_filter( | ||
| 'get_edit_post_link', | ||
| static function () { | ||
| return 'http://example.com/wp-admin/post.php?post=1&action=edit'; | ||
| } | ||
| ); | ||
|
|
||
| ob_start(); | ||
| $edit_flow->calendar->get_inner_information( | ||
| $edit_flow->calendar->get_post_information_fields( $post ), | ||
| $post | ||
| ); | ||
| $html = ob_get_clean(); | ||
|
|
||
| remove_all_filters( 'get_edit_post_link' ); | ||
|
|
`EF_Calendar::get_inner_information()` builds five action links for each calendar item — Edit, Trash, Preview, View, and Save. Three of those hrefs came from functions that return plain URLs (`get_edit_post_link()`, `get_delete_post_link()`, `get_permalink()`), and all five link text strings used `__()` directly. Neither set went through an escape function.
PR #993 made the same fix for `story-budget.php`; the calendar module's `item_actions` block was the parallel section that was missed.
This PR wraps the three hrefs in `esc_url()` and changes all five link-text calls to `esc_html__()`. The Preview branch already had `esc_url()` on its href — only the link text needed updating there.
The new test class `CalendarEscapingTest` injects a URL with a bare `&` via a WordPress filter (`get_edit_post_link`, `post_link`) and asserts the output contains `&` (what `esc_url()` produces for `&`) rather than the raw `&`. A separate assertion confirms no raw `&` survives in any href attribute. `test_calendar_view_link_href_is_url_escaped` uses a published post so `get_permalink()` is the active code path (not the preview branch). The trash link is omitted from integration coverage because `get_delete_post_link()` routes through `wp_nonce_url()` which pre-encodes `&` as `&` — an integration test for that path cannot distinguish "esc_url() ran" from "wp_nonce_url() pre-encoded the value."
Tests reviewed by inspection; not executed locally (no integration harness in my environment) — relying on CI for the first run.
No visual or interaction change; escaping only, link text and structure unchanged, no a11y impact.
(props @thisismyurl)
(full disclosure: AI helped me identify the issue and verify my work)