Missing escaping in admin menu walker file

[aniketpatel]

I found one escaping issue in class-walker-nav-menu-edit.php admin file.

Screenshot: ​https://share.cleanshot.com/3Qwfq4XS16gHWH3LD3m9

Trac ticket: https://core.trac.wordpress.org/ticket/57541

[mukesh27]

Thanks @aniketpatel for the ticket and PR!

Moving to 6.2 milestone for visibility.

[audrasjb]

As $edit_url comes from admin_url() I'm unsure we can get anything else than an URL here, but it looks like a pattern used in some other places so let's consider this.

Thanks @aniketpatel32, PR look good to me.

@costdev I did like to get your feedback on this.

[mukesh27]

Thanks @audrasjb for taking ownership.

Thanks for the ping @mukeshpanchal27!

The PR looks good to me, thanks @aniketpatel32!

[costdev]

I agree @audrasjb. I've wondered a few times why we do this.

Here's what I can gather:

1. This variable is set to admin_url(...), which has apply_filters() as the final return value. Extenders could modify the URL with something unsafe, but if they're filtering, then they already have access to do a lot worse. Like cleaning out the fridge in case a burglar poisons the food.

2. However, the description of esc_url() states:

eliminates invalid characters and removes dangerous characters

So, even though esc_url() is mainly used for security, this also helps when extenders return a value that has invalid characters. I guess this is one reason why the pattern exists elsewhere in Core.

3. We tell extenders to sanitize all URLs before output, so maybe this pattern also exists to encourage this.

Given (2) and (3), it makes sense to continue this pattern.

The PR looks good to me. 👍

[audrasjb]

I agree with both points of your comment Colin 👍

[audrasjb]

In 55135:

Menus: Add missing escaping function in Admin Menu walker.

Props aniketpatel, mukesh27, costdev.
Fixes #57541.

Committed in https://core.trac.wordpress.org/changeset/55135