WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#9633 closed defect (bug) (fixed)

wp:meta_value does not escape correctly

Reported by: gslin Owned by: westi
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Export Keywords: has-patch commit early
Focuses: Cc:

Description

In wp-admin/includes/export.php:

<wp:meta_value><?Php echo $meta->meta_value; ?></wp:meta_value>

Since meta_value might have "&" or "<" this kind of special chars, we need to escape it:

<wp:meta_value><?php echo htmlspecialchars($meta->meta_value); ?></wp:meta_value>

Attachments (2)

9633.patch (531 bytes) - added by hakre 5 years ago.
9633.2.patch (524 bytes) - added by hakre 5 years ago.
CDATA to the rescue

Download all attachments as: .zip

Change History (21)

comment:1 Denis-de-Bernardy5 years ago

  • Milestone changed from Unassigned to 2.8

hakre5 years ago

comment:2 hakre5 years ago

  • Keywords has-patch added; needs-patch removed

comment:3 Denis-de-Bernardy5 years ago

shouldn't that be html entities, even?

comment:4 hakre5 years ago

Well, send me the DTD for that export-xml and I can tell you. normally there is no problem with having elements inside other elements within xml.

comment:5 gslin5 years ago

I got an export file from our customer, the file's wp:meta_value has serialized value, like this one:

<wp:meta_value>a:9:{s:5:"title";s:61:"[keep private]";s:11:"description";s:744:"[some content with & char]";s:4:"date";s:14:"[keep private]";s:6:"author";s:11:"[keep private]";s:14:"author_profile";s:40:"[keep private]"";s:8:"duration";s:5:"05:10";s:9:"embedcode";s:198:"[some content with < char]";}</wp:meta_value>

comment:6 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch removed

I believe htmlentities should be used, or the thing should be put within a cdata. else, any meta value with invalid chars and invalid named entities will break the feed just as well

comment:7 hakre5 years ago

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

generally html_specialchars() should be sufficent because the export names the charset used and therefore html_enteties() is not needed.

but this is quite theoretical, most important is the import. there is not a problem with that invalid xml export, wordpress does not take care for validity on the import side nor does it have a problem with & characters inside the content:

// Now for post meta
preg_match_all('|<wp:postmeta>(.*?)</wp:postmeta>|is', $post, $postmeta);

/wordpress-trunk/wp-admin/import/wordpress.php ~ line 533

so there is no bug with the export. additionally, the import does not decode anything. therefore, the content should not be encoded on export, wether be it with html_specialchar() nor with html_entities(). this would break the import.

i suggest to close as won't fix now. the other suggestion is to improve the import as well but that needs a bigger patch and more developer feedback. without adoption in the immport a fix here should not considered to be usefull because it will break the import.

gerenally i think this should be fixed in import and export. feel free to reopen and provide more information.

comment:8 Denis-de-Bernardy5 years ago

  • Milestone 2.8 deleted

comment:9 Denis-de-Bernardy5 years ago

agreed to a large extent -- importer and exporter should get fixed together in 2.9 or 3.0.

hakre5 years ago

CDATA to the rescue

comment:10 hakre5 years ago

  • Milestone set to 2.8

The propper way in 2.8 is CDATA. For the rewrite of import and export I suggest to open another ticket.

comment:11 Denis-de-Bernardy5 years ago

  • Milestone 2.8 deleted

there is another ticket for import/export already, with a milestone of future. it initially dealt with beautifying the export, and Ryan highlighted that it might break the importer, which is regex based. he also hinted that there is no point in changing something that works, even if it doesn't technically validate against a dtd.

comment:12 hakre5 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone set to 2.8
  • Resolution wontfix deleted
  • Status changed from closed to reopened

-1 for close. CDATA does not break the importer. +1 for the rest as in #8471.

comment:13 Denis-de-Bernardy5 years ago

  • Keywords commit added

let's see if it generates any interest

comment:15 Denis-de-Bernardy5 years ago

  • Keywords early added
  • Milestone changed from 2.8 to 2.9

supposedly not for 2.8

comment:16 Denis-de-Bernardy5 years ago

patch still applies clean

comment:17 westi4 years ago

  • Milestone changed from 2.9 to 3.0
  • Owner set to westi
  • Status changed from reopened to accepted

Probably relates to #10619 as well.

We should fix both of these early in 3.0

comment:18 westi4 years ago

This patch looks ok and tests out well for me.

Old versions of WordPress back to at least 2.7.0 (I only tested this far back) will import these encoded values fine.

I added some simple test cases for the import of these files to wordpress-tests and also discovered #11574 in the process.

comment:19 westi4 years ago

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

(In [12506]) Wrap the content of <wp:meta_value></wp:meta_value> in CDATA for WXR exports. Fixes #9633 props hakre.

Note: See TracTickets for help on using tickets.