WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9402 closed enhancement (fixed)

Validation fixes - "&" instead of "&"

Reported by: simek Owned by:
Milestone: 2.8 Priority: low
Severity: trivial Version: 2.8
Component: Validation Keywords: has-patch tested commit
Focuses: Cc:

Description

Validation fixes for latest WP revision.

Some "&" signs are replaced with "&" tag in mentioned below files:

  • wp-admin\plugin-editor.php
  • wp-admin\theme-editor.php
  • wp-admin\includes\update.php

Patch file attached into ticket

Attachments (4)

validation_fixes.patch (4.7 KB) - added by simek 5 years ago.
more_validation_fixes.patch (5.6 KB) - added by simek 5 years ago.
more_more_validation_fixes.patch (5.8 KB) - added by simek 5 years ago.
9402.patch (613 bytes) - added by hakre 5 years ago.
Updated Patch against 2.8 bleeding

Download all attachments as: .zip

Change History (15)

simek5 years ago

comment:1 simek5 years ago

More validation fixes in next patch for mentioned below files:

  • wp-admin\import\blogger.php
  • wp-admin\includes\dashboard.php

comment:2 follow-ups: markjaquith5 years ago

Did you do these changes based on finding invalid ampersands, or did you just replace them all? clean_url() does the & entity encoding, so those changes aren't necessary if clean_url() is still functioning properly.

comment:3 in reply to: ↑ 2 markjaquith5 years ago

And when I say "these changes" I mean "the changes for strings passed through clean_url()"

comment:6 in reply to: ↑ 2 hakre5 years ago

Replying to markjaquith:

Did you do these changes based on finding invalid ampersands, or did you just replace them all? clean_url() does the & entity encoding, so those changes aren't necessary if clean_url() is still functioning properly.

I did this by validating the output. So the patched places are locations that I could identify as the source of the invalid output passed through all programm logic before outputted. well, make easy things complicated. short answer is: "by finding invalid ampersands by hand, not by replace all".

comment:7 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.8 to 2.9

moving to 2.9, as patch needs to be refreshed

comment:8 hakre5 years ago

I got myself mixed up with the wrong ticket in my last comment. anyway, I will take a look into the issue to get a more actual patch.

hakre5 years ago

Updated Patch against 2.8 bleeding

comment:9 hakre5 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from 2.9 to 2.8

The following Functions add & or & automatically and therefore the places did not needed a patch any longer:

  • clean_url
  • wp_nonce_url

The the wp() function is based on the server request which is an URI that contains &. & / & is only used for XHTML output. This reduces the patch as well.

Some functions aren't using XHTML-Encoded URIs:#

  • get_categories

wp-admin/includes/plugin-install.php - already fixed
wp-admin/includes/plugin.php - already fixed
wp-admin/includes/post.php - partially fixed, one script tag left
wp-admin/includes/template.php - already fixed
wp-admin/includes/theme-install.php - alread fixed

for the one script tag left I've created a patch against 2.8 bleeding.

This is only a single fix now. I would suggest to put that into 2.8. I know that it is part of another patch I've put on here: #9432 / qcop-r00-ampersand-post.patch.

comment:10 Denis-de-Bernardy5 years ago

  • Keywords tested commit added

for 9402.patch

comment:11 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11296]) Ampersand validation fix. Props hakre. fixes #9402

Note: See TracTickets for help on using tickets.