WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#2059 closed defect (bug) (fixed)

Slashes stripped in <pre> (caused by #1793)

Reported by: allan Owned by:
Milestone: 2.3 Priority: normal
Severity: normal Version: 1.5.2
Component: General Keywords: has-patch dev-feedback
Focuses: Cc:

Description

Using a backslash in <pre> will cause it to be stripped, e.g.:

<pre>printf("test\n");</pre>

results in (after the wpautop filter):

<pre>printf("testn")</pre>

Attachments (3)

wpautop_no_removal_of_slashes_in_pre.patch (1.1 KB) - added by allan 12 years ago.
Patch to fix the problem
formatting.php.patch (782 bytes) - added by DelGurth 11 years ago.
Patch (for wordpress HEAD) to fix the removal of slashes in <pre> tags
2059.diff (1.1 KB) - added by mdawaffe 11 years ago.
uses preg_replace_callback() and modifies clean_pre()

Download all attachments as: .zip

Change History (13)

@allan
12 years ago

Patch to fix the problem

#1 @plaes
12 years ago

  • Version changed from 1.5.2 to 2.0.4

This is still present in 2.0.4

The patch submitted earlier by Allan fixes the problem :)

#2 @error
12 years ago

  • Keywords bg|has-patch bg|dev-feedback added
  • Milestone set to 2.1
  • Version changed from 2.0.4 to 1.5.2

#3 @matt
11 years ago

  • Milestone changed from 2.1 to 2.2

#4 @foolswisdom
11 years ago

  • Keywords has-patch dev-feedback added; bg|has-patch bg|dev-feedback removed
  • Milestone changed from 2.2 to 2.3

#5 @mdawaffe
11 years ago

Messing with stripslashes is the wrong approach.

A good rule of thumb, never use preg_replace(/e); the slashing is too complicated. Use preg_replace_callback() instead.

#6 follow-up: @Albertane
11 years ago

What is the right approach? I'd really like to get this fixed, it makes it a pain to try and post code snippets to my blog. Does the attached patch still work on 2.2? Is it going to cause other problems? Thanks.

#7 in reply to: ↑ 6 @DelGurth
11 years ago

Replying to Albertane:

What is the right approach? I'd really like to get this fixed, it makes it a pain to try and post code snippets to my blog. Does the attached patch still work on 2.2? Is it going to cause other problems? Thanks.


The right approach would be something like (at least, this code seems to work for me):

$pee = preg_replace_callback('!(<pre.*?>)(.*?)</pre>!is', create_function('$matches', 'return $matches[1] . clean_pre($matches[2]) . "</pre>";'), $pee);

Replying to mdawaffe:

Messing with stripslashes is the wrong approach.

A good rule of thumb, never use preg_replace(/e); the slashing is too complicated. Use preg_replace_callback() instead.

It can be a good rule of thumb, but it was in there already. So why not use it? But true, it sounds like you can better use preg_replace_callback then just the /e variant, since you can trap yourself in some nice problems (see the PHP documentation about it(1)).
But the stripslashes should not be needed in the first place. At least, according to me slashes should be allowed within a <pre> tag.



I was personally wondering why the stripslashes was there in the first place. But I guess that has something to do with a PHP bug(2) I found when I was looking why I was loosing slashes in wordpress within my <pre> tags.

(1) http://www.php.net/preg_replace
(2) http://bugs.php.net/bug.php?id=10666

@DelGurth
11 years ago

Patch (for wordpress HEAD) to fix the removal of slashes in <pre> tags

#8 follow-up: @mdawaffe
11 years ago

Since wpautop can be called dozens of times per page loead, we should probably not use create_function().

I suggest preg_replace_callback( regex, 'clean_pre' ) and modifying clean_pre() to accept ether the regex match array or a string of text for backward compatibility.

Attached is a patch which does this based on DelGurth's create_function() code.

@mdawaffe
11 years ago

uses preg_replace_callback() and modifies clean_pre()

#9 @ryan
11 years ago

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

(In [6102]) Don't strip slashes from pre. Props DelGurth and mdawaffe. fixes #2059

#10 in reply to: ↑ 8 @DelGurth
11 years ago

Replying to mdawaffe:

Since wpautop can be called dozens of times per page loead, we should probably not use create_function().

Sounds like a wise idea. I'm new to wordpress and with the first real item I wanted to post I encountered this bug, so I was kinda in a hurry to create a fix. I was not sure if I could alter the clean_pre() function, but if I had used your method I should not have worried about it.

Thanks for using my input to kill the bug.

Note: See TracTickets for help on using tickets.