WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 2 months ago

#45292 new defect (bug)

wp_targeted_link_rel "corrupts" the customized changeset JSON when inserting the changeset post in database

Reported by: nikeo Owned by:
Milestone: 5.1 Priority: high
Severity: normal Version: trunk
Component: Customize Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description

The problem

It happen when trying to publish customizations that include a link with a target attribute

When customizing a setting that generates a customized value containing html link strings with target attributes, it breaks the publishing action. The customizations are visible in the customizer preview, even when published, but the customized data are not actually saved in the database when publishing. This means that when you close the customizer, you don't see your customizations on front.

What happen ?

When the customized JSON includes links with targets attributes, the filter wp_targeted_link_rel "corrupts" the JSON in the post_content of the changeset_post. This occurs when the filter content_save_pre is fired. It generates a JSON_ERROR_SYNTAX preventing the customizer publish action to be done. see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-customize-manager.php#L1119

How to reproduce ?

The problem can be reproduced with any plugin or theme including a customizer setting and control ( textarea or editor ) allowing users to generate html links strings.

Reproduce with the Twentynineteen theme

Copy and paste the following code in functions.php

// Registers a test section, setting and textarea control
add_action('customize_register', function( $wp_customize ) {
  $wp_customize->add_section(
    'test_target_link', array(
      'title'    => __( 'Tests : wp_targeted_link_rel', 'twentyseventeen' ),
      'priority' => 0, // Before Additional CSS.
    )
  );
  $wp_customize->add_setting(
    'test_target_link', array(
      'default'           => '',
      //'transport'         => 'postMessage',
      //'sanitize_callback' => 'twentyseventeen_sanitize_colorscheme',
    )
  );
  $wp_customize->add_control(
    'test_target_link', array(
      'label'           => __( 'Test target link breaks the publish action', 'twentyseventeen' ),
      'section'         => 'test_target_link',
      'type'            => 'textarea',
      'description'     => __( 'Test a link with target attribute not published', 'twentyseventeen' )
    )
  );
});

// Prints once at loop_start
add_action('loop_start', function() {
  if ( ! did_action( 'test_printed') ) {
      printf( '<br>%1$s<br>', get_theme_mod('test_target_link') );
  }
  do_action('test_printed');
});

Open the customizer and navigate to the test control. Paste the following html code in the textarea :

Global Climate Change : this <a href="https://climate.nasa.gov/vital-signs/carbon-dioxide/" target="_blank">time series</a> below shows global distribution and variation of the concentration of mid-tropospheric carbon dioxide in parts per million (ppm).

If you click on publish and quit the customizer, you won't see the text on front. It will be published only if the target attribute is removed.

Reproduce with a plugin

Customize Posts plugin ( https://wordpress.org/plugins/customize-posts/ )

Try to edit any post content by adding a link with a target attribute. It won't be published.

Nimble Builder plugin ( https://wordpress.org/plugins/nimble-builder/ )

Try to drag and drop the quote module ( which includes a link with a target attribute ). It won't be saved on publish.

As you will observe it, the json_parse_error customizer notification is displayed sometimes. But there's no error printed in the javascript console and the php debug_log.

How does the corrupted JSON look like when inserted in the changeset post

Corrupted JSON

{
    "twentynineteen::test_target_link": {
        "value": "Global Climate Change : this <a href=\"https://climate.nasa.gov/vital-signs/carbon-dioxide/\" target=\"_blank\" rel="noopener noreferrer">time series</a> below shows global distribution and variation of the concentration of mid-tropospheric carbon dioxide in parts per million (ppm).",
        "type": "theme_mod",
        "user_id": 1,
        "date_modified_gmt": "2018-11-05 20:02:57"
    }
}

As you can see, the attributes noopener noreferrer added by the wp_targeted_link_rel are not escaped with backslashes, which generates a JSON validation error.

Possible solutions

proposition 1

The function wp_targeted_link_rel could check if the $text param is a JSON, and then json_decode before the regex, and json_encode after. see https://core.trac.wordpress.org/browser/trunk/src/wp-includes/formatting.php#L3036

proposition 2

The preg_replace_callback in the function could be improved to include the JSON case, and handle the escape backslashes.

proposition 3

Inspired by what is already done with the KSES filters here https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-customize-manager.php#L2890. The idea would be to remove and add back the wp_targeted_link_rel filter, before and after the insertion of the changeset post in database.

Change History (6)

#1 @peterwilsoncc
2 months ago

  • Keywords needs-patch needs-testing added
  • Milestone changed from Awaiting Review to 5.0
  • Severity changed from major to normal

Thanks for logging this and including some proposed solutions.

I've moved this on to the 5.0 milestone.

#2 follow-up: @peterwilsoncc
2 months ago

  • Keywords needs-unit-tests added; needs-testing removed

#3 in reply to: ↑ 2 @nikeo
2 months ago

Replying to peterwilsoncc: @peterwilsoncc thanks for taking care of it.

#4 @antpb
2 months ago

  • Milestone changed from 5.0 to Future Release

Work for this is not yet begun so I am moving this to a future release as we are moving toward 5.0 RC currently.

#5 @ocean90
2 months ago

  • Milestone changed from Future Release to 5.0
  • Priority changed from normal to high

The filter was introduced in #43187 so it needs to be fixed or reverted from the branch.

#6 @pento
2 months ago

  • Milestone changed from 5.0 to 5.1
  • Version changed from 5.0 to trunk

[42770] is being reverted from the 5.0 branch, so I'm moving this ticket to 5.1, so it can be addressed there, instead.

Note: See TracTickets for help on using tickets.