WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12005 closed defect (bug) (fixed)

thousandsSeparator and decimalPoint are not being escaped

Reported by: Chionsas Owned by: nbachiyski
Milestone: 3.0 Priority: low
Severity: normal Version: 2.9.1
Component: I18N Keywords: thousandsSeparator decimalPoint javascript has-patch
Focuses: Cc:

Description

file: wp-admin/admin-header.php
line: 44

[..] thousandsSeparator = '<?php echo $wp_locale->number_format['thousands_sep']; ?>', decimalPoint = '<?php echo $wp_locale->number_format['decimal_point']; ?>';

When the translation file has "'" put in for thousands separator, you get JavaScript code:

thousandsSeparator = '''

which raises JS syntax error and therefore the media buttons (add-file/add-image while editing page/post and possibly some other places) do not work.
I suppose some languages could also have "'" as a decimal point, though it's more less likely than the thousands separator.


There can be several approaches to solving this problem:

  • wrapping the variables in esc_js() before echo (clean, but wastes CPU cycles)
  • changing the quotes from ' to " (double quotes), which are less likely to be used as a thousands separator. This could be used in combination with a comment in the translations (.pot) file for the translators to be aware of this problem and not use " in delimiters.

Attachments (1)

12005_2.9.diff (1.3 KB) - added by eddieringle 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 azaozz4 years ago

As we only expect one character there the "usual" fix would be echo addslashes(...).

comment:2 nacin4 years ago

(In [13078]) Escape thousandsSeparator and decimalPoint JS variables, see #12005

comment:3 nacin4 years ago

[13078] fixes this for trunk. Leaving open if there's a desire to apply to 2.9.

eddieringle4 years ago

comment:4 eddieringle4 years ago

  • Cc eddie@… added

This looks like it's been inactive, so I'm adding a patch for 2.9 if that is what it takes to get this moving.

comment:5 eddieringle4 years ago

  • Keywords has-patch added

comment:6 nacin4 years ago

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

(In [14258]) Escape thousandsSeparator and decimalPoint JS variables. fixes #12005 for 2.9.

comment:7 nacin4 years ago

  • Milestone 2.9.3 deleted

Milestone 2.9.3 deleted

comment:8 nacin4 years ago

  • Milestone set to 3.0
Note: See TracTickets for help on using tickets.