Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#27942 closed defect (bug) (fixed)

Site Title not escaped when using HTML entities

Reported by: bandonrandon's profile BandonRandon Owned by: kovshenin's profile kovshenin
Milestone: 4.5 Priority: normal
Severity: normal Version: 2.9
Component: Options, Meta APIs Keywords: has-patch has-unit-tests commit
Focuses: administration Cc:

Description

Today I tried to set my site title to <Brooke><Codes> and learned that the site title field escapes the < character. This is fine I thought, I'll use HTML entities. While this worked at first this also failed in the end. Is there a way to use the same escaping used on posts titles on site titles?

Here are a few screenshots to show the problem:

Title using HTML entities:
https://i.cloudup.com/ct_yH6t2er-3000x3000.png

After saving the above:
https://i.cloudup.com/k2-OCiyDA7-3000x3000.png

After saving the above:
https://i.cloudup.com/ImnHqRiPJJ-3000x3000.png

To me the real issue is that it fails silently. I know enough to know why it fails but to many of our users they may have a WTF reaction.

Attachments (6)

27942.patch (415 bytes) - added by BandonRandon 12 years ago.
27942.2.patch (417 bytes) - added by BandonRandon 12 years ago.
Add spacing around attribute
unit-tests.27942.diff (978 bytes) - added by pauldewouters 12 years ago.
unit tests for sanitize_option
27942.3.patch (398 bytes) - added by BandonRandon 11 years ago.
27942.4.patch (1.5 KB) - added by BandonRandon 11 years ago.
Patch With Unit Test
27942.5.patch (2.0 KB) - added by kovshenin 11 years ago.

Download all attachments as: .zip

Change History (19)

#1 @wonderboymusic
12 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#2 @BandonRandon
12 years ago

  • Keywords has-patch added; needs-patch removed

I dung into this some more and learned that you are able to use valid HTML in the site title. For example if you decided to use <em>Site</em> <strong> Title</strong> the title will save and output correctly.

It looks like this problem is only occurring when using invalid HTML.

https://i.cloudup.com/q6Gqt8Je-n-2000x2000.png

I have submitted a patch that switches from using wp_kses_post to htmlentities2 in formatting.php resolving this issue.

@BandonRandon
12 years ago

#3 @Clorith
12 years ago

  • Focuses administration added
  • Keywords needs-unit-tests added
  • Version changed from trunk to 2.9

Seems like a simple fix, should probably update the patch to space out the attribute to conform with the coding standards.

I did test a few versions back, and in 2.0 when sanitize_option was introduced this was not a problem, but in 2.9 it does happen, so somewhere along that road a regression occurred that was missed.

We should probably also have unit tests for sanitize_option when considering all the functions the various cases relies on.

@BandonRandon
12 years ago

Add spacing around attribute

#4 @BandonRandon
12 years ago

Should probably update the patch to space out the attribute to conform with the coding standards.

Added in 27942.2.patch

#5 @lancewillett
12 years ago

This could be a duplicate of #24328.

Last edited 12 years ago by lancewillett (previous) (diff)

This ticket was mentioned in IRC in #wordpress-dev by Clorith. View the logs.


12 years ago

#7 @SergeyBiryukov
12 years ago

#24328 was marked as a duplicate.

@pauldewouters
12 years ago

unit tests for sanitize_option

#8 @pauldewouters
12 years ago

I added some tests for 'blogname', is this a good start?

#9 @anonymized_8769252
11 years ago

  • Keywords needs-unit-tests removed

@BandonRandon
11 years ago

Patch With Unit Test

#10 @BandonRandon
11 years ago

  • Keywords needs-testing has-unit-tests added

Updated patch against trunk and added unit test.

#11 @markjaquith
11 years ago

  • Keywords 2nd-opinion added

Seems to work. Unit tests pass. Much better user experience.

@kovshenin
11 years ago

#12 @kovshenin
11 years ago

  • Keywords commit added; needs-testing 2nd-opinion removed
  • Milestone changed from Future Release to 4.5
  • Owner set to kovshenin
  • Status changed from new to assigned

Added unit tests for blogdescription too. Also looked into some history, goes all the way back to r5541 and looks like it's good to go.

#13 @kovshenin
11 years ago

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

In 35788:

Allow usage of angle brackets in a site title or tagline.

The whole string is escaped with esc_html() anyway, so we don't
need to wp_kses_post(). This is a better experience for users who
want to use angle brackets in their site title or description.
Does not allow any HTML, adds unit tests.

props BandonRandon, pauldewouters.
fixes #27942.

Note: See TracTickets for help on using tickets.