CRM: Use proper string escape functions by tbradsha · Pull Request #48496 · Automattic/jetpack

tbradsha · 2026-05-04T19:33:40Z

Proposed changes

This PR updates a few instances that were escaped in a JS context, but not with the proper function. Examples include:

esc_js() → wp_json_encode() or (int)
esc_html() → wp_json_encode()or(int)`
parseInt() → (int)
zeroBSCRM_slashOut() → wp_json_encode()

Note that a lot of this could be refactored, but right now the focus is escaping. I did move several arrays out to PHP so they could be one-lined in JS (avoiding PHPCS hassle).

Also, I found and fixed some more broken JS var regressions from #46809: 143263f

Related product discussion/links

Does this pull request change what data or activity we track or use?

Testing instructions

I guess look at the code. The context should be fairly clear.

github-actions · 2026-05-04T19:34:51Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

jp-launch-control · 2026-05-04T19:36:44Z

Code Coverage Summary

Coverage changed in 18 files. Only the first 5 are listed here.

File	Coverage	Δ%	Δ Uncovered
projects/plugins/crm/includes/ZeroBSCRM.MetaBoxes3.Transactions.php	0/296 (0.00%)	0.00%	6 💔
projects/plugins/crm/admin/settings/custom-fields.page.php	0/258 (0.00%)	0.00%	5 💔
projects/plugins/crm/admin/settings/tax.page.php	0/121 (0.00%)	0.00%	4 💔
projects/plugins/crm/includes/ZeroBSCRM.Edit.Segment.php	0/168 (0.00%)	0.00%	4 💔
projects/plugins/crm/admin/contact/view.page.php	0/746 (0.00%)	0.00%	3 ❤️‍🩹

Full summary · PHP report

_{If appropriate, add one of these labels to override the failing coverage check:

Covered by non-unit tests
Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code

Coverage tests to be added later
Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR

I don't care about code coverage for this PR
Use this label to ignore the check for insufficient code coveage.}

anomiex

Overall, looks reasonable. Haven't tested at all.

Something else you may want to double check is whether any downstream uses of the JS vars were expecting PHP to have HTML-escaped them, since CRM does a lot of building of HTML strings in JS. Then again, maybe we already took care of those in #48390. 😀

anomiex · 2026-05-04T19:57:53Z

 							}

-							echo esc_html( $labelK ) . ":'" . esc_html( zeroBSCRM_slashOut( $labelV ) ) . "'";
+							echo esc_html( $labelK ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );


Suggested change

echo esc_html( $labelK ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );

echo wp_json_encode( $labelK, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );

Or really, though, you could replace this whole block with

var zbsEditViewLangLabels = <?php echo wp_json_encode( array_merge( array( 'today' => __( 'Today', 'zero-bs-crm' ), ), $this->langLabels ), JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP ); ?>;

Yep, this is a case where I held back (part of that refactor note in the description). 😄 I've done similar things in other places in the CRM for language arrays. But not today.

On second thought, I'll adjust this for better readability, as I've found several other such instances. 😄

anomiex · 2026-05-04T20:03:11Z

 			<?php
 			#} Nonce for AJAX
-					echo "var zbscrmjs_secToken = '" . esc_js( wp_create_nonce( 'zbscrmjs-ajax-nonce' ) ) . "';";
+					echo 'var zbscrmjs_secToken = ' . wp_json_encode( wp_create_nonce( 'zbscrmjs-ajax-nonce' ), JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP ) . ';';
 			?>


Seems odd this isn't like

var zbscrmjs_secToken = <?php echo wp_json_encode( wp_create_nonce( 'zbscrmjs-ajax-nonce' ), JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP ); ?>;

like the rest here.

anomiex · 2026-05-04T20:05:47Z

 							}

-							echo esc_html( $labelK ) . ":'" . esc_html( zeroBSCRM_slashOut( $labelV, true ) ) . "'";
+							echo esc_html( $labelK ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );


Same as above, the simple fix being to also json_encode $labelK and the better fix being an array_merge and letting json_encode do the object.

anomiex · 2026-05-04T20:12:07Z

 							}

-							echo esc_html( $labelK ) . ":'" . esc_html( zeroBSCRM_slashOut( $labelV ) ) . "'";
+							echo esc_html( $labelK ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );


Same as above.

Newlines added in #46809 broke these. Let's one-line them by moving most logic to separate PHP.

tbradsha requested a review from anomiex May 4, 2026 19:33

tbradsha self-assigned this May 4, 2026

tbradsha added the [Status] Needs Review This PR is ready for review. label May 4, 2026

github-actions Bot added [CRM] MailPoet Module [CRM] Portal Module [CRM] WooSync Module [Plugin] CRM Issues about the Jetpack CRM plugin Admin Page React-powered dashboard under the Jetpack menu labels May 4, 2026

tbradsha requested review from a team and removed request for anomiex May 4, 2026 19:38

anomiex reviewed May 4, 2026

View reviewed changes

tbradsha mentioned this pull request May 5, 2026

CRM: Fix code style formatting throughout codebase #46809

Merged

3 tasks

tbradsha added 9 commits May 5, 2026 13:03

Escape CRM strings

58171cf

Add changelog

857540a

More esc_js replacements

0e81d8b

Escape key as well

3f8b2d1

Simplify output

912406d

Process arrays in PHP and escape with wp_json_encode()

adbb47e

More escaping

0fe58ec

Fix broken JS vars

143263f

Newlines added in #46809 broke these. Let's one-line them by moving most logic to separate PHP.

Update Phan baseline

eb86b1e

tbradsha force-pushed the fix/crm/escape_strings_properly branch from 9777595 to eb86b1e Compare May 5, 2026 19:03

	echo esc_html( $labelK ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES \| JSON_HEX_TAG \| JSON_HEX_AMP );
	echo wp_json_encode( $labelK, JSON_UNESCAPED_SLASHES \| JSON_HEX_TAG \| JSON_HEX_AMP ) . ':' . wp_json_encode( $labelV, JSON_UNESCAPED_SLASHES \| JSON_HEX_TAG \| JSON_HEX_AMP );

Conversation

tbradsha commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Related product discussion/links

Does this pull request change what data or activity we track or use?

Testing instructions

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

jp-launch-control Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

anomiex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tbradsha May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tbradsha commented May 4, 2026 •

edited

Loading

jp-launch-control Bot commented May 4, 2026 •

edited

Loading

tbradsha May 4, 2026 •

edited

Loading