Skip to content

Commit 2f161d2

Browse files
obenlandobenland
committed
Plugin & Theme Directory: Treat a translated field like the value it replaces
Closes WordPress#926. git-svn-id: https://meta.svn.wordpress.org/sites/trunk@15208 74240141-8908-4e6f-9713-ba540dce6ec7
1 parent dc5ea43 commit 2f161d2

6 files changed

Lines changed: 433 additions & 9 deletions

File tree

‎wordpress.org/public_html/wp-content/plugins/plugin-directory/class-plugin-i18n.php‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ class Plugin_I18n {
4444
*/
4545
public static $set_cache = true;
4646

47+
/**
48+
* Readme parser, held only for its field sanitizer.
49+
*
50+
* @var Readme\Parser|null
51+
*/
52+
private $readme_parser = null;
53+
4754
/**
4855
* Fetch the instance of the Plugin_I18n class.
4956
*
@@ -366,6 +373,11 @@ public function translate( $key, $content, $args = array() ) {
366373
return $a_len == $b_len ? 0 : ($a_len > $b_len ? -1 : 1);
367374
} );
368375

376+
$stored = $content;
377+
378+
// Markers are ours; one arriving in stored content would pick its own substitution site.
379+
$content = self::remove_translation_markers( $content );
380+
369381
// Mark each original for translation
370382
foreach ( $originals as $original_id => $original ) {
371383
if ( isset( $translations[ $original_id ] ) ) {
@@ -374,9 +386,57 @@ public function translate( $key, $content, $args = array() ) {
374386
}
375387

376388
// Translate the marked originals.
377-
$content = $this->translate_marked_gp_originals( $content, $translations, $originals );
389+
$translated = $this->translate_marked_gp_originals( $content, $translations, $originals );
378390

379-
return $content;
391+
// Nothing was marked, so the stored value stands as it was.
392+
if ( $translated === $content ) {
393+
return $stored;
394+
}
395+
396+
return $this->sanitize_translation( $key, $translated );
397+
}
398+
399+
/**
400+
* Strips this class's substitution markers from a value it did not mark.
401+
*
402+
* Leaves a space where each one was. Deleting them instead would close the gap,
403+
* and the text on either side could meet as a marker of its own; a space cannot
404+
* appear in one, so nothing can form across it and a single pass is enough.
405+
*
406+
* @param string $content The content to be searched.
407+
* @return string The content, with no marker syntax of its own left in it.
408+
*/
409+
public static function remove_translation_markers( $content ) {
410+
return preg_replace( '/___TRANSLATION_\d+___/', ' ', $content );
411+
}
412+
413+
/**
414+
* Reduces a translated field to the markup it accepts.
415+
*
416+
* Runs on the assembled field, not on each substituted translation: a
417+
* translation that is harmless alone can still land somewhere its own
418+
* sanitizer cannot see, such as inside an attribute.
419+
*
420+
* @param string $key The translation key, as passed to {@see Plugin_I18n::translate()}.
421+
* @param string $translation The assembled field, or a single translation.
422+
* @return string The value, reduced to the field's own allow-list.
423+
*/
424+
public function sanitize_translation( $key, $translation ) {
425+
// A block title is stored as the block declared it, so markup is all there is to drop.
426+
if ( str_starts_with( $key, 'block_title:' ) ) {
427+
return wp_strip_all_tags( $translation );
428+
}
429+
430+
// Encoded because the readme path stores these encoded, and quotes reach attributes.
431+
if ( 'title' === $key || 'excerpt' === $key ) {
432+
return esc_html( wp_strip_all_tags( $translation ) );
433+
}
434+
435+
if ( ! $this->readme_parser ) {
436+
$this->readme_parser = new Readme\Parser( '' );
437+
}
438+
439+
return $this->readme_parser->filter_text( $translation );
380440
}
381441

382442
/**

‎wordpress.org/public_html/wp-content/plugins/plugin-directory/shortcodes/class-screenshots.php‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace WordPressdotorg\Plugin_Directory\Shortcodes;
99

10+
use WordPressdotorg\Plugin_Directory\Readme\Parser;
1011
use WordPressdotorg\Plugin_Directory\Template;
1112

1213
/**
@@ -421,18 +422,37 @@ protected static function build_image_block( $screenshot, $id, $above_fold = fal
421422
);
422423
$figure .= '</a>';
423424

425+
$caption = self::escape_block_delimiters( self::filter_caption( $caption ) );
426+
424427
if ( '' !== $caption ) {
425-
$figure .= sprintf(
426-
'<figcaption class="wp-element-caption">%s</figcaption>',
427-
self::escape_block_delimiters( wp_kses_post( $caption ) )
428-
);
428+
$figure .= sprintf( '<figcaption class="wp-element-caption">%s</figcaption>', $caption );
429429
}
430430

431431
$figure .= '</figure>';
432432

433433
return "<!-- wp:image {$attrs} -->\n{$figure}\n<!-- /wp:image -->\n";
434434
}
435435

436+
/**
437+
* Reduces a caption to the markup a readme caption may carry.
438+
*
439+
* `wp_kses_post()` keeps every `data-*` attribute, and the lightbox loads the
440+
* Interactivity runtime on the page, where a `data-wp-` attribute rewrites its
441+
* own tag after every server filter has run. The readme's list admits no `data-*`.
442+
*
443+
* @param string $caption Caption HTML.
444+
* @return string Caption HTML carrying only readme markup.
445+
*/
446+
protected static function filter_caption( $caption ) {
447+
static $parser = null;
448+
449+
if ( ! $parser ) {
450+
$parser = new Parser( '' );
451+
}
452+
453+
return $parser->filter_text( $caption );
454+
}
455+
436456
/**
437457
* Encodes block-comment delimiters in a caption.
438458
*

‎wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Plugin_I18n_Sanitization_Test.php‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
<?php
2+
/**
3+
* Tests that a GlotPress translation is held to its own field's markup boundary.
4+
*
5+
* @package WordPressdotorg\Plugin_Directory\Tests
6+
*/
7+
8+
declare( strict_types = 1 );
9+
10+
use PHPUnit\Framework\Attributes\DataProvider;
11+
use PHPUnit\Framework\Attributes\Group;
12+
use PHPUnit\Framework\TestCase;
13+
use WordPressdotorg\Plugin_Directory\Plugin_I18n;
14+
15+
/**
16+
* A translation replaces the value the readme parser sanitized at import, and is
17+
* then displayed wherever that value was.
18+
*
19+
* @group i18n
20+
*/
21+
#[Group( 'i18n' )]
22+
class Plugin_I18n_Sanitization_Test extends TestCase {
23+
24+
/**
25+
* The translator under test.
26+
*
27+
* @var Plugin_I18n
28+
*/
29+
protected $i18n;
30+
31+
/**
32+
* Builds the translator without its constructor, which needs cache globals.
33+
*/
34+
protected function setUp(): void {
35+
parent::setUp();
36+
37+
$reflection = new \ReflectionClass( Plugin_I18n::class );
38+
$this->i18n = $reflection->newInstanceWithoutConstructor();
39+
}
40+
41+
/**
42+
* Data provider for {@see test_markup_fields_keep_section_markup_only()}.
43+
*
44+
* @return array<string, array{0: string, 1: string, 2: string}>
45+
*/
46+
public static function markup_field_provider(): array {
47+
return array(
48+
'script in a section' => array(
49+
'description',
50+
'Bonjour <script>alert(1)</script>',
51+
'Bonjour alert(1)',
52+
),
53+
'directives in a section' => array(
54+
'faq',
55+
'<a data-wp-interactive="core/image" data-wp-context=\'{"t":"javascript:alert(1)"}\' data-wp-bind--href="context.t" href="#">Détails</a>',
56+
'<a href="#">Détails</a>',
57+
),
58+
'directives in a caption' => array(
59+
'screenshot-1',
60+
'<a data-wp-bind--href="context.t" href="#">Légende</a>',
61+
'<a href="#">Légende</a>',
62+
),
63+
'event attribute in a caption' => array(
64+
'screenshot-2',
65+
'<strong onmouseover="alert(1)">Légende</strong>',
66+
'<strong>Légende</strong>',
67+
),
68+
'comment syntax in a caption' => array(
69+
'screenshot-3',
70+
'Légende <!-- /wp:image --><!-- wp:image -->',
71+
'Légende',
72+
),
73+
'section markup survives unchanged' => array(
74+
'changelog',
75+
'<h4>1.0</h4><ul><li>Correction d\'un <code>bug</code>, voir la <a href="https://example.org/" rel="nofollow">doc</a>.</li></ul>',
76+
'<h4>1.0</h4><ul><li>Correction d\'un <code>bug</code>, voir la <a href="https://example.org/" rel="nofollow">doc</a>.</li></ul>',
77+
),
78+
);
79+
}
80+
81+
/**
82+
* Sections and screenshot captions carry readme markup and nothing else.
83+
*
84+
* @param string $key The translation key the field translates under.
85+
* @param string $translation The translation as GlotPress stored it.
86+
* @param string $expected The value the field may display.
87+
*/
88+
#[DataProvider( 'markup_field_provider' )]
89+
public function test_markup_fields_keep_section_markup_only( string $key, string $translation, string $expected ): void {
90+
$this->assertSame( $expected, $this->i18n->sanitize_translation( $key, $translation ) );
91+
}
92+
93+
/**
94+
* Data provider for {@see test_plain_text_fields_drop_all_markup()}.
95+
*
96+
* @return array<string, array{0: string}>
97+
*/
98+
public static function plain_text_field_provider(): array {
99+
return array(
100+
'title' => array( 'title' ),
101+
'excerpt' => array( 'excerpt' ),
102+
'block title' => array( 'block_title:' . md5( 'Block' ) ),
103+
);
104+
}
105+
106+
/**
107+
* Title, short description and block titles are plain text in every consumer.
108+
*
109+
* @param string $key The translation key the field translates under.
110+
*/
111+
#[DataProvider( 'plain_text_field_provider' )]
112+
public function test_plain_text_fields_drop_all_markup( string $key ): void {
113+
$translation = 'Mon Plugin <script>alert(1)</script><a href="#" data-wp-bind--href="context.t">x</a>';
114+
115+
$this->assertSame( 'Mon Plugin x', $this->i18n->sanitize_translation( $key, $translation ) );
116+
$this->assertStringNotContainsString( 'alert', $this->i18n->sanitize_translation( $key, $translation ) );
117+
}
118+
119+
/**
120+
* The title and short description are stored entity-encoded, so a translation is too.
121+
*
122+
* A block title is not: it is stored as the block declared it.
123+
*/
124+
public function test_plain_text_fields_match_their_stored_encoding(): void {
125+
$this->assertSame( 'Tom &amp; Jerry &lt; Co', $this->i18n->sanitize_translation( 'title', 'Tom & Jerry < Co' ) );
126+
$this->assertSame( 'Tom &amp; Jerry', $this->i18n->sanitize_translation( 'excerpt', 'Tom &amp; Jerry' ) );
127+
$this->assertSame( 'Tom & Jerry', $this->i18n->sanitize_translation( 'block_title:' . md5( 'B' ), 'Tom & Jerry' ) );
128+
}
129+
130+
/**
131+
* A stored marker cannot pick its own substitution site.
132+
*
133+
* `title` is on the readme's allow-list, so a readme can store marker syntax
134+
* inside an attribute and aim a translation at a context it would survive.
135+
*/
136+
public function test_a_stored_marker_is_not_a_substitution_site(): void {
137+
$stored = '<a href="https://example.org/" title="___TRANSLATION_7___">Docs</a>';
138+
139+
$this->assertSame(
140+
'<a href="https://example.org/" title=" ">Docs</a>',
141+
$this->i18n->translate_marked_gp_originals(
142+
Plugin_I18n::remove_translation_markers( $stored ),
143+
array( 7 => '" onmouseover="alert(1)' ),
144+
array( 7 => 'Docs' )
145+
)
146+
);
147+
}
148+
149+
/**
150+
* A marker split across a marker does not survive the one that wraps it.
151+
*/
152+
public function test_a_marker_cannot_be_spliced_from_the_text_around_one(): void {
153+
$crafted = '___TRAN___TRANSLATION_1___SLATION_7___';
154+
155+
$this->assertDoesNotMatchRegularExpression(
156+
'/___TRANSLATION_\d+___/',
157+
Plugin_I18n::remove_translation_markers( $crafted )
158+
);
159+
}
160+
161+
/**
162+
* Sanitizing the assembled field catches what a substitution opened up.
163+
*/
164+
public function test_assembled_field_drops_an_attribute_a_substitution_opened(): void {
165+
$assembled = '<a href="https://example.org/" title="" onmouseover="alert(1)">Docs</a>';
166+
167+
$this->assertSame(
168+
'<a href="https://example.org/" title="">Docs</a>',
169+
$this->i18n->sanitize_translation( 'description', $assembled )
170+
);
171+
}
172+
}

‎wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Screenshot_Caption_Directives_Test.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
/**
3+
* Tests that a screenshot caption cannot carry Interactivity directives.
4+
*
5+
* @package WordPressdotorg\Plugin_Directory\Tests
6+
*/
7+
8+
declare( strict_types = 1 );
9+
10+
namespace WordPressdotorg\Plugin_Directory\Shortcodes;
11+
12+
use PHPUnit\Framework\Attributes\Group;
13+
use PHPUnit\Framework\TestCase;
14+
use ReflectionMethod;
15+
16+
require_once __DIR__ . '/fixtures/production-environment.php';
17+
18+
/**
19+
* Every screenshot renders as a lightbox Image block, so the page carries the
20+
* Interactivity runtime and a `data-wp-` attribute on a caption would rewrite
21+
* its own tag once hydrated. A caption is display text.
22+
*
23+
* @group shortcodes
24+
*/
25+
#[Group( 'shortcodes' )]
26+
class Screenshot_Caption_Directives_Test extends TestCase {
27+
28+
/**
29+
* A caption whose anchor would bind its own `href` after hydration.
30+
*
31+
* @var string
32+
*/
33+
const CAPTION = '<a data-wp-interactive="core/image" data-wp-context=\'{"target":"javascript:alert(1)"}\' data-wp-bind--href="context.target" href="#">Open screenshot details</a>';
34+
35+
/**
36+
* A caption reaches the block as display markup, with no directives left on it.
37+
*/
38+
public function test_caption_directives_are_dropped(): void {
39+
$method = new ReflectionMethod( Screenshots::class, 'build_image_block' );
40+
$post_id = wp_insert_post(
41+
array(
42+
'post_name' => 'caption-directives',
43+
'post_title' => 'Caption Directives',
44+
'post_type' => 'plugin',
45+
'post_status' => 'publish',
46+
'post_modified' => current_time( 'mysql' ),
47+
'post_modified_gmt' => current_time( 'mysql', true ),
48+
)
49+
);
50+
51+
setup_postdata( get_post( $post_id ) );
52+
53+
try {
54+
$markup = $method->invoke(
55+
null,
56+
array(
57+
'src' => 'https://ps.w.org/caption-directives/assets/screenshot-1.png',
58+
'filename' => 'screenshot-1.png',
59+
'caption' => self::CAPTION,
60+
),
61+
9000003,
62+
true,
63+
array( 1200, 800 )
64+
);
65+
} finally {
66+
wp_reset_postdata();
67+
wp_delete_post( $post_id, true );
68+
}
69+
70+
$this->assertStringNotContainsString( 'data-wp-', $markup );
71+
$this->assertStringNotContainsString( 'javascript:', $markup );
72+
$this->assertStringContainsString( '>Open screenshot details</a>', $markup );
73+
}
74+
}

0 commit comments

Comments
 (0)