Ticket #2059 (closed defect (bug): fixed)

Opened 6 years ago

Last modified 4 years ago

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

Reported by: allan Owned by: anonymous
Priority: normal Milestone: 2.3
Component: General Version: 1.5.2
Severity: normal Keywords: has-patch dev-feedback
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

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

Change History

allan6 years ago

Patch to fix the problem

  • 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 :)

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

comment:3   matt5 years ago

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

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.

comment:6 follow-up: ↓ 7   Albertane4 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.

comment:7 in reply to: ↑ 6   DelGurth4 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

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

comment:8 follow-up: ↓ 10   mdawaffe4 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.

uses preg_replace_callback() and modifies clean_pre()

comment:9   ryan4 years ago

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

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

comment:10 in reply to: ↑ 8   DelGurth4 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.