Skip to content

Use esc_url_raw() for avatar URLs in AJAX responses#4903

Open
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/avatar-ajax-url-encoding
Open

Use esc_url_raw() for avatar URLs in AJAX responses#4903
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/avatar-ajax-url-encoding

Conversation

@GaryJones
Copy link
Copy Markdown

Problem

The avatar upload and crop AJAX handlers in bp-core/bp-core-avatars.php use esc_url() to encode the avatar URL before returning it in a JSON response.

esc_url() is designed for HTML output contexts — it encodes ampersands as &, strips certain characters, and performs other transformations appropriate for embedding URLs in HTML attributes. When the URL is returned in a JSON response for JavaScript consumption, these transformations corrupt the URL. Query string parameters with & become &, and certain valid URL characters may be stripped entirely.

Fix

Switch both occurrences to esc_url_raw(), which performs the same sanitisation (protocol validation, invalid character removal) without the HTML-specific encoding. This is the correct function for URLs that will be used programmatically rather than output in HTML.

This follows the WordPress Codex guidance: use esc_url() for HTML output, esc_url_raw() for database storage, redirects, and HTTP API usage.

Test plan

  • Upload a new avatar — verify the returned avatar URL is valid and the avatar displays correctly
  • Crop an existing avatar — verify the cropped avatar URL is valid
  • Test with an avatar URL containing query string parameters — verify ampersands are not double-encoded

The avatar upload and crop AJAX handlers use esc_url() to encode
the avatar URL before returning it in a JSON response. esc_url()
is designed for HTML output and encodes ampersands and other
characters that are valid in raw URLs, which can break the URL
when JavaScript consumes it.

Switch to esc_url_raw() which sanitises without encoding for
display context, preserving the URL for programmatic use.
@chetansatasiya
Copy link
Copy Markdown
Contributor

@claude Review the PR

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Claude finished @chetansatasiya's task in 1m 12s —— View job


PR Review

  • Read changed file and gather context
  • Review the code changes
  • Post final review

Summary

The fix is correct and well-reasoned. Using esc_url_raw() instead of esc_url() for URLs returned in JSON/AJAX responses is the proper WordPress practice, as esc_url() HTML-encodes ampersands (&&) which corrupts query string parameters when consumed by JavaScript.

Changes Reviewed (src/bp-core/bp-core-avatars.php)

Line ~1494 (webcam/camera avatar capture path):

// Before
'avatar' => esc_url( bp_core_fetch_avatar( ... ) ),
// After
'avatar' => esc_url_raw( bp_core_fetch_avatar( ... ) ),

Line ~1566 (crop path):

// Before
'avatar' => esc_url( bp_core_fetch_avatar( ... ) ),
// After
'avatar' => esc_url_raw( bp_core_fetch_avatar( ... ) ),

Both changes are correct — these values go directly into wp_send_json_success() for JavaScript consumption.


Issue Found: Missed Third Occurrence

There is a third occurrence of this same pattern in the same function (bp_avatar_ajax_set()) that was not fixed. At line ~988, the avatar delete path still uses esc_url(), and a previous developer added html_entity_decode() as a workaround to un-corrupt the URL:

// Line ~988: still using esc_url() for an AJAX response
$avatar = esc_url(
    bp_core_fetch_avatar( ... )
);

// Line ~1002: html_entity_decode() workaround to undo esc_url()'s damage
$return = array(
    'avatar' => html_entity_decode( $avatar, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401 ),
    ...
);
wp_send_json_success( $return );

This is inconsistent with the fix applied in this PR. The html_entity_decode() workaround is a sign of a previous fix that should be cleaned up — the correct approach here is also esc_url_raw() (removing the need for html_entity_decode() entirely).

Fix this →


Other esc_url() Uses (Correct — No Changes Needed)

  • Lines ~717, ~740, ~749: Gravatar URLs being stored in a local variable that feeds into HTML output — esc_url() is appropriate.
  • Lines ~2361–2362: Inside a sprintf() building an <img> HTML tag — esc_url() is the correct function here.

Verdict

The two changes in this PR are correct. The PR should be merged, but consider following up with a separate fix for the missed third occurrence at line ~988 to remove the html_entity_decode() workaround and make the codebase consistent.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants