Escape translated strings on output - #895
Closed
obenland wants to merge 19 commits into
Closed
Conversation
Rewrites the _e(), _ex(), __(), _x(), _n() and _nx() calls the EscapeOutput sniff flags across the sites. Plain strings go through esc_html_e() and esc_html__(), or the esc_attr variants inside attributes; plural calls are wrapped in esc_html(); strings that carry markup are wrapped in wp_kses_post(), so no translation string changes. One string printed into a JavaScript literal uses esc_js(). Call spacing and missing statement semicolons on the touched lines are normalised alongside. Also corrects a `<span5%s</span>` typo in the bb-base activity nav that broke the BuddyPress translation lookup for that label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Only the label needs translating, so the anchor becomes the format string and the label goes through esc_html__(). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The changed-lines linter reports every placeholder string on a touched line that lacks one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Escapes the neighbouring output on the same lines: bbPress titles and author links, counts, dates, widget title markup and the BuddyPress activity nav URLs. - Numbers the placeholders in five strings that use %s more than once, and turns the concatenated _n() in the Trac components page into a proper plural string with the milestone as a placeholder. - Reflows the multi-line printf() calls into the call-signature format the ruleset wants, and adds file docblocks to the templates whose first line changed. - Registers jobswp_field_value() as auto-escaping in the ruleset; it returns selected() markup, a kses-filtered description, or an esc_attr()'d value. - Marks the jobswp templates' reads of $_POST['errors'] and $_POST['job_token'], which the plugin's form handler sets, and the search-term subtitles that are escaped for display, with scoped phpcs:ignore comments. - Converts two rosetta templates and two forum notices from CRLF to LF. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 204 files, which is 104 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (204)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Straight apostrophes in the translated strings on touched lines become ’, the form the surrounding strings already use, and the strings that only needed double quotes for an apostrophe move to single quotes. Also fixes "are't" in the Photo Directory stats notice. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The freshness links, the author card status labels and the rejecting user link are anchors or styled spans, so esc_html() printed their tags as text. Also restores the $format template the moderator row in the photo submitbox still uses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The support forums prepend an <abbr> to NSFW topic titles through the bbp_get_topic_title filter, so the support-2024 headings take wp_kses_post() rather than esc_html(). Escapes the plugin title and the committer name that reach HTML slots as printf() arguments, and restores the "No site" string the first commit had turned into "None". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd it. Fixes the one user-visible regression in the batch: repairing the `<span5%s</span>` typo in the bb-base activity nav made the corrupted `'ms'` argument visible, so the sitewide activity directory rendered "All Members ms". It now passes bp_get_total_member_count(), matching the four sibling tabs. Five translators comments sat on their own line above a line holding two gettext calls, so the extractor bound them to the leading placeholder-free string and the string with the placeholder got nothing. They move inline, immediately before the call they describe. The jobswp ruleset entry claimed jobswp_field_value() is safe in every context, but its job_description branch returns kses-filtered HTML that carries unescaped quotes. That branch becomes jobswp_field_description_value(), so the allowlisted function only ever returns an attribute fragment, and the now-redundant per-line ignore goes away. The `$_POST['errors']` and `$_POST['job_token']` ignores said the values are "not read from the request", which is the opposite of what those lines do: the form handlers are gated on postjob/removejob and check their nonce inside that gate, while the templates read the keys on a bare isset(). The justification now names the real protection, the reads are unslashed, and the sniff scope narrows to InputNotSanitized so MissingUnslash is no longer hidden. Also escapes the arguments the sweep left raw next to ones it fixed: the bbPress forum title in the topic sidebar, the .tar.gz size, the plugin titles and permalinks in the release confirmations list, the review author name and login, the current user's display name, and the ratings-compat notice that carries an anchor. Nits: aligns @Package with each directory's convention, corrects the two GlotPress form partial docblocks to say they serve both new and edit, names the placeholder in the meetups comment, and normalises the translators comments this batch touched to sentence case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The .tar.gz size collapses onto one line, matching the .zip sibling above it, rather than tripping the multi-line call-signature rules; its URL gets esc_url() now that the line is in the diff. Narrowing the job token's ignore to InputNotSanitized uncovered InputNotValidated, which the previous blanket ignore had hidden. The index is validated instead of suppressed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…comment binding. The (int) casts on the activity nav counts truncated them. All five bp_get_total_*() helpers are filtered through bp_core_number_format(), so they return number_format_i18n() output; (int) "1,234" is 1, and the tabs rendered "1" for any count over 999. Four of those casts were a regression against trunk, which passed the values raw. They now go through esc_html() instead. wp_kses_post() around bbp_get_topic_author_link() silently dropped the avatar's srcset and decoding attributes, which are not in $allowedposttags['img'], leaving blurry 1x gravatars on HiDPI in the topic lists. Those two call sites now echo bbPress's markup with a scoped ignore, matching the sibling templates that already do. The translators comments above the wptv2 archive headings sat on the line before a line holding two gettext calls, so the extractor bound them to the date-format msgids as well; a translator reading "translators: %s: Year." on the msgid "Y" could reasonably put a placeholder into a date format. The date format moves to its own line. The two term titles there are escaped to match their date siblings. The buddypress-domain tooltip goes back to the straight apostrophe: that msgid belongs to BuddyPress, and it has to match upstream's catalogue byte for byte. Reverts the @Package tags on three theme root templates: the bbpress/ subdirectory convention does not apply to footer.php and header-accessibility.php, which are the themes' own templates. Corrects two comments of mine that said the opposite of what the code does: jobswp_field_value() returns a whole attribute pair rather than something safe inside an attribute's quotes and can return nothing at all, and wp_filter_kses() re-slashes on the way out, which is why the outer stripslashes() has to stay. Also escapes the zip download link and version, the category select's classes, and the two term titles; drops two stale translators comments that described the wrong string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bb-base post byline escaped its format string but passed both arguments through raw, so WordPress.Security.EscapeOutput still errored on archive.php, index.php and single.php. get_the_author_link() returns markup, so it takes wp_kses_post() rather than esc_html(). Also drops the translators comment that became a duplicate: WPCS wants it directly above the gettext call, which is where this batch put it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sweep added a translators comment inside the printf, where WPCS wants it, leaving the pre-existing one above the call saying the same thing. Same cleanup as the bb-base byline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The topic title in these headings was wrapped in straight double quotes. They become “/”, matching the ’ the rest of the batch uses. esc_html__() leaves the entities alone, since _wp_specialchars() passes double_encode false through to htmlspecialchars(). These strings are in the wporg-forums domain, so the new msgids orphan their existing translations; bbPress's own copies of these templates are in the bbpress domain and are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The heading put "in the %s component" outside the _n() call, so the sentence could only ever render in English word order. Both halves move into the plural forms with numbered placeholders, letting translators reorder them. Rendered output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two codex themes carry the same byline as the bb-base templates: an escaped format string with both arguments passed raw. get_the_author_link() returns markup, so it takes wp_kses_post(); the date takes esc_html(). The translators comment the batch superseded goes away with them. The showcase description textarea echoed post_excerpt behind a // textarea_escaped annotation that asserted an escaping the line did not do. textarea is in kses' post set, so a saved excerpt containing </textarea> closes the field early on the edit screen. It goes through esc_textarea() now and the annotation is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same treatment as the support forums: bbPress builds the markup, and wp_kses_post() would strip the srcset and decoding attributes from the avatar. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Escapes translated strings on their way into markup, clearing every
WordPress.Security.EscapeOutput.UnsafePrintingFunctionviolation and the__()-familyOutputNotEscapedones (EscapeOutput total 2,437 → 1,080; UnsafePrintingFunction 826 → 0). Second batch after #814.Where a rewrite put a line into the diff, the other violations the changed-lines linter reports for that line are fixed alongside it, which is what the fourth commit is.
Commit by commit:
_e(),_ex(),__(),_x(),_n()and_nx()calls. Plain strings becomeesc_html_e()/esc_html__(), or theesc_attrvariants where the call sits inside an attribute. Plural calls are wrapped inesc_html(). The 139 strings that carry markup are wrapped inwp_kses_post()rather than restructured, so those strings keep their existing translations. One string printed into a JavaScript literal goes throughesc_js(). Call spacing and missing semicolons on the touched lines are normalised. Also fixes a<span5%s</span>typo in the bb-base activity nav that broke the BuddyPress translation lookup for that label.esc_html__().%smore than once; turns the concatenated_n()in the Trac components page into a proper plural string with the milestone as a placeholder; reflows multi-lineprintf()calls into the ruleset's call-signature format; adds file docblocks to templates whose first line changed; registersjobswp_field_value()as auto-escaping in the ruleset since it returnsselected()markup, a kses-filtered description, or anesc_attr()'d value; and marks with scopedphpcs:ignorecomments the jobswp templates' reads of$_POST['errors']and$_POST['job_token'], which the plugin's own form handler sets, and the two list-table search subtitles that are escaped for display. Two rosetta templates and two forum notices go from CRLF to LF.’, the form the surrounding strings already use, and strings that only needed double quotes for an apostrophe move to single quotes. Also fixes "are't" in the Photo Directory stats notice.wp_kses_post()where the wrapped value is markup (bbPress freshness links, the author card status labels, the Photo Directory rejecting-user link) and the$formattemplate the photo submitbox still used.<abbr>to NSFW topic titles, so the support-2024 headings usewp_kses_post(); the plugin title and committer name that reach HTML slots asprintf()arguments are escaped; and the "No site" string the first commit had turned into "None" is restored.Decisions worth a look:
wp_kses_post(). Moving the tags out would change the translation keys for dozens of locales. The handbook watch link is the one place that was done, on request. The strings that do change are the two watch labels, the five with numbered placeholders, the Trac milestone plural, the<span5typo, and the 21 apostrophes.bbp_get_topic_author_link()andget_the_author_posts_link()return markup and getwp_kses_post(); widget$before_title/$after_titletoo.(int)cast rather thanesc_html().Left for follow-ups, in this order: the known-safe function outputs (
number_format_i18n(),human_time_diff(),get_the_*(), bbPress getters), the 29 exception messages, then the plain variables one component at a time.🤖 Generated with Claude Code