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 Owned by: kovshenin
Priority: normal Milestone: 4.5
Component: Options, Meta APIs Version: 2.9
Severity: normal Keywords: has-patch has-unit-tests commit
Cc: Focuses: administration

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 Awaiting Review → 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 trunk → 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 Future Release → 4.5
  • Owner set to kovshenin
  • Status new → 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 → fixed
  • Status assigned → 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.