Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#2130 closed defect (bug) (fixed)

wp_kses kills < !-- more --> from xmlrpc posted entries

Reported by: tsimmons Owned by: ryan
Milestone: Priority: normal
Severity: major Version: 2.0
Component: XML-RPC Keywords: bg|has-patch bg|commit
Focuses: Cc:


I couldn't find the exact point where it happens, but somewhere when XMLRPC posted entries are being processed, the


tags are being stripped which gets rid of the "more..." link and instead displays the entire post on the indexes. I tracked it down to somewhere in the nested functions called in kses.php, somewhere after line 55:

	return wp_kses_split($string, $allowed_html_fixed, $allowed_protocols);

Attachments (4)

kses.php.diff (468 bytes) - added by tsimmons 10 years ago.
Proposed patch
kses.php.2.diff (782 bytes) - added by tsimmons 10 years ago.
New version of suggested patch
kses-comments.diff (668 bytes) - added by skeltoac 10 years ago.
perfect-comments.diff (1.1 KB) - added by skeltoac 10 years ago.

Download all attachments as: .zip

Change History (19)

#1 @tsimmons
10 years ago

Okay, after more digging, I found it is in wp_kses_split2() on line 101:

if (!preg_match('%^<\s*(/\s*)?([a-zA-Z0-9]+)([^>]*)>?$%', $string, $matches))

This always strips the <!--more--> tags. My regular expression skillz are way down right now (mighty tired after long night ...)

Sorry I can't offer a fix right off ...

#2 @tsimmons
10 years ago

I can't take credit for this, but how does the fix from http://mu.wordpress.org/forums/topic/450 sound? Add three lines to the function at line 101:

	if (preg_match('%^<!--[^>-]+-->$%', $string))
		return $string;
	# Allow HTML comments

10 years ago

Proposed patch

#3 @skeltoac
10 years ago

No, this expression will miss valid comments such as this:

10 years ago

New version of suggested patch

#4 @tsimmons
10 years ago

I have attached a new proposed fix, based on Owen's suggestion in wp-testers. It works for me !!

#5 @davidhouse
10 years ago

  • Keywords bg|has-patch added

#6 @ryan
10 years ago

  • Owner changed from anonymous to ryan

#7 @masquerade
10 years ago

Based on the patch above (untested), the following would pass right through.

&lt;!--something--&gt;&lt;script&gt;malicious code&lt;/script&gt;&lt;!--somethingelse--&gt;

#8 @masquerade
10 years ago

Er, pretend that didn't get mangled

<!--something--><script>malicious code</script><!--somethingelse-->

#9 @ringmaster
10 years ago

Can you fix this by adding a ? to the regex in the first replacement block? Like:

return preg_replace('%(<!--.*?-->)|(<'.# EITHER: <

#10 @ryan
10 years ago

  • Milestone set to 2.0.1

#11 @ryan
10 years ago

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

(In [3417]) Pass comments through kses. Props tsimmons. fixes #2130 #2167

#13 @skeltoac
10 years ago

  • Keywords bg|commit added; kses wp_kses xmlrpc more removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

That last patch is broken: the first regex is missing a () so the eval never gets the right string.

perfect-comments.diff fixes that and more carefully filters the contents of the comment. Now, if a comment is left unclosed, it will be closed at the end of the string. Also, nested comments and uneven open/close markers are fixed. Comments are rock-solid.

#14 @ryan
10 years ago

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

(In [3429]) kses regex tweakage for better comment filtering. fixes #2130

#15 @anonymous
9 years ago

  • Milestone 2.0.1 deleted

Milestone 2.0.1 deleted

Note: See TracTickets for help on using tickets.