WordPress Trac: Ticket #11257: Reproducible Output Failurehttps://core.trac.wordpress.org/ticket/11257
<p>
Steps to reproduce:
</p>
<ol><li>Open any browser and start a new post.
</li></ol><ol start="2"><li>Paste the attached file into HTML view.
</li></ol><ol start="3"><li>Click the Preview button.
</li></ol><p>
Expected Result: Preview :)
</p>
<p>
Actual Result: Blank Post D:
</p>
<p>
Tested only on the one site so far.
</p>
en-usWordPress Trachttps://core.trac.wordpress.org/chrome/site/your_project_logo.pnghttps://core.trac.wordpress.org/ticket/11257
Trac 1.0.1miqrogrooveWed, 25 Nov 2009 02:56:02 GMTattachment sethttps://core.trac.wordpress.org/ticket/11257
https://core.trac.wordpress.org/ticket/11257
<ul>
<li><strong>attachment</strong>
set to <em>wordpress-io-crash.html</em>
</li>
</ul>
<p>
This post body makes WordPress epic fail.
</p>
TicketninjaWRWed, 25 Nov 2009 04:01:21 GMTmilestone changed; keywords sethttps://core.trac.wordpress.org/ticket/11257#comment:1
https://core.trac.wordpress.org/ticket/11257#comment:1
<ul>
<li><strong>keywords</strong>
<em>reporter-feedback</em> added
</li>
<li><strong>milestone</strong>
changed from <em>2.9</em> to <em>Future Release</em>
</li>
</ul>
<p>
Using <a class="changeset" href="https://core.trac.wordpress.org/changeset/12273" title="Don't require titles for attachments. Props caesarsgrunt. fixes #11052">r12273</a> and 2.7.1 (first time in a while using that site), I don't have any problems with the attached code.
</p>
<p>
In any case, moving to Future Release since 2.9 is in beta.
</p>
TicketscribuWed, 25 Nov 2009 12:39:30 GMThttps://core.trac.wordpress.org/ticket/11257#comment:2
https://core.trac.wordpress.org/ticket/11257#comment:2
<p>
What is the URL of the page you are sent to?
</p>
<p>
If it contains post-new.php, it just means that the post hasn't been saved yet. In that case, this ticket would be invalid.
</p>
TicketmiqrogrooveSat, 28 Nov 2009 22:07:17 GMTmilestone changedhttps://core.trac.wordpress.org/ticket/11257#comment:3
https://core.trac.wordpress.org/ticket/11257#comment:3
<ul>
<li><strong>milestone</strong>
changed from <em>Future Release</em> to <em>2.9</em>
</li>
</ul>
<p>
Hi
</p>
<p>
ninjaWR, I'm not sure if the 2.7.1 test would be relevant. Hopefully I will have some time to set up a proper test environment to get some more detail for you. Moving back to 2.9 because that's what betas are for. If we can't reproduce this bug before release then it has no business being assigned to another version.
</p>
<p>
scribu, the URL can't be relevant, because I was able to reproduce this on a published post as well as in preview.
</p>
<p>
This bug is blocking me from reporting some other bugs, so I will be increasing the Severity if I can pinpoint the cause/trigger.
</p>
TicketazaozzSun, 29 Nov 2009 00:47:38 GMThttps://core.trac.wordpress.org/ticket/11257#comment:4
https://core.trac.wordpress.org/ticket/11257#comment:4
<p>
Seems to work as expected in trunk. Technically you cannot preview something that's not in the db yet so when clicking Preview for an unsaved post, it's saved as draft first and then the browser is redirected to a new tab/window to display it. So the URL your browser is redirected to may be relevant when that fails.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 03:07:39 GMTkeywords deletedhttps://core.trac.wordpress.org/ticket/11257#comment:5
https://core.trac.wordpress.org/ticket/11257#comment:5
<ul>
<li><strong>keywords</strong>
<em>reporter-feedback</em> removed
</li>
</ul>
<p>
Problem did not show up on a clean install, nor with the usual suspect plugins.
</p>
<p>
Debugging on the problem site so far shows that the function wpautop() is not returning a value when filters such as 'the_content' are applied.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 03:18:49 GMThttps://core.trac.wordpress.org/ticket/11257#comment:6
https://core.trac.wordpress.org/ticket/11257#comment:6
<p>
I traced it down to the last line of function wpautop()
</p>
<pre class="wiki">$pee = preg_replace('/&lt;p&gt;\s*?(' . get_shortcode_regex() . ')\s*&lt;\/p&gt;/s', '$1', $pee);
</pre><p>
I don't have time tonight to debug get_shortcode_regex(), but I'll keep you all posted when I do.
</p>
Ticketdd32Sun, 29 Nov 2009 03:45:37 GMThttps://core.trac.wordpress.org/ticket/11257#comment:7
https://core.trac.wordpress.org/ticket/11257#comment:7
<p>
Sounds like the all too common preg backtrace limit to me: <a class="closed ticket" href="https://core.trac.wordpress.org/ticket/8553" title="defect (bug): preg_replace_callback in do_shortcode returns empty for large posts (closed: fixed)">#8553</a>
</p>
TicketmiqrogrooveSun, 29 Nov 2009 20:42:56 GMThttps://core.trac.wordpress.org/ticket/11257#comment:8
https://core.trac.wordpress.org/ticket/11257#comment:8
<p>
hi dd32, thanks for the heads up, but if this little post of mine triggered 100,000 recursion depth then something has gone horribly wrong. I'm going to check it out now.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 21:17:40 GMThttps://core.trac.wordpress.org/ticket/11257#comment:9
https://core.trac.wordpress.org/ticket/11257#comment:9
<p>
I've confirmed get_shortcode_regex is not the cause of the problem.
</p>
<p>
It's starting to look like there is a conflict between the P elements and shortcode matching.
</p>
<p>
Note the regexp is incorrect for situations like
</p>
<pre class="wiki">&lt;p&gt;[shortcode]&lt;p&gt;[shortcode]&lt;p&gt;Hello World&lt;/p&gt;
</pre><p>
... which is exactly what wpautop() is emitting before the last statement. Could the syntax error be causing the recursion problem? I guess I'll have to test that theory as well.
</p>
Ticketdd32Sun, 29 Nov 2009 21:37:36 GMThttps://core.trac.wordpress.org/ticket/11257#comment:10
https://core.trac.wordpress.org/ticket/11257#comment:10
<blockquote class="citation">
<p>
hi dd32, thanks for the heads up, but if this little post of mine triggered 100,000 recursion depth then something has gone horribly wrong. I'm going to check it out now.
</p>
</blockquote>
<p>
I assume you actually checked the depth on your setup? I've seen it set incredibly small :)
</p>
<p>
If that wasnt your problem (then GREAT!.. because that tickets been open for awhile now)
</p>
<p>
The syntax could definately be causing it, causing the regex not to match and then infinitely loop.. or, remove the entire post if it matches the right(wrong) rule possibly.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 21:42:45 GMThttps://core.trac.wordpress.org/ticket/11257#comment:11
https://core.trac.wordpress.org/ticket/11257#comment:11
<p>
Wait a sec. Doesn't "\2" backtrace the WRONG value?
</p>
<p>
And on another note, removing some of the extraneous parentheses from get_shortcode_regex() actually solved my test case, but I'm still looking if it's dependent on post length before patching.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 22:18:19 GMThttps://core.trac.wordpress.org/ticket/11257#comment:12
https://core.trac.wordpress.org/ticket/11257#comment:12
<p>
I'm too sexy for my PHP [) Patch coming right up.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 22:26:48 GMTattachment sethttps://core.trac.wordpress.org/ticket/11257
https://core.trac.wordpress.org/ticket/11257
<ul>
<li><strong>attachment</strong>
set to <em>ticket-11257.patch</em>
</li>
</ul>
<p>
Resolves unreasonable recursion depth during wpautop. Explanation to follow.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 22:31:05 GMThttps://core.trac.wordpress.org/ticket/11257#comment:13
https://core.trac.wordpress.org/ticket/11257#comment:13
<p>
The primary change made by this patch is to remove the following code, which until now has served only to heat up server CPUs across the planet:
</p>
<pre class="wiki">(?:(\/))?
</pre><p>
It is replaced by the equivalent non-recursive form:
</p>
<pre class="wiki">\\/?
</pre><p>
This patch also fixes a related bug wherein wpautop() backreferences the wrong preg match when looking for shortcode close tags like /caption.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 22:31:30 GMTpriority, severity changed; keywords sethttps://core.trac.wordpress.org/ticket/11257#comment:14
https://core.trac.wordpress.org/ticket/11257#comment:14
<ul>
<li><strong>keywords</strong>
<em>has-patch</em> added
</li>
<li><strong>priority</strong>
changed from <em>normal</em> to <em>high</em>
</li>
<li><strong>severity</strong>
changed from <em>normal</em> to <em>critical</em>
</li>
</ul>
TicketmiqrogrooveSun, 29 Nov 2009 22:31:41 GMTcomponent changedhttps://core.trac.wordpress.org/ticket/11257#comment:15
https://core.trac.wordpress.org/ticket/11257#comment:15
<ul>
<li><strong>component</strong>
changed from <em>General</em> to <em>Shortcodes</em>
</li>
</ul>
TicketmiqrogrooveSun, 29 Nov 2009 22:40:34 GMTattachment sethttps://core.trac.wordpress.org/ticket/11257
https://core.trac.wordpress.org/ticket/11257
<ul>
<li><strong>attachment</strong>
set to <em>ticket-11257-v2.patch</em>
</li>
</ul>
<p>
First version missed strip_shortcodes
</p>
TicketmiqrogrooveSun, 29 Nov 2009 23:08:19 GMThttps://core.trac.wordpress.org/ticket/11257#comment:16
https://core.trac.wordpress.org/ticket/11257#comment:16
<p>
I'm working on better steps to reproduce. There's something going on before that one line of code that's causing the bug to show up/not on my different sites.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 23:20:40 GMThttps://core.trac.wordpress.org/ticket/11257#comment:17
https://core.trac.wordpress.org/ticket/11257#comment:17
<p>
OK FYI, when you use my file wordpress-io-crash.html, WordPress will rewrite all of the HREF values based on the attachment ID values. This will prevent you from reproducing the bug using my code.
</p>
TicketscribuSun, 29 Nov 2009 23:22:01 GMThttps://core.trac.wordpress.org/ticket/11257#comment:18
https://core.trac.wordpress.org/ticket/11257#comment:18
<p>
If you can prove that your patch performs better and doesn't cause any regressions, that would be good enough.
</p>
TicketmiqrogrooveSun, 29 Nov 2009 23:28:45 GMTattachment sethttps://core.trac.wordpress.org/ticket/11257
https://core.trac.wordpress.org/ticket/11257
<ul>
<li><strong>attachment</strong>
set to <em>wpautop-test-case.php</em>
</li>
</ul>
<p>
Test Case demonstrates failure of the last assignment to $pee in wpautop()
</p>
TicketmiqrogrooveSun, 29 Nov 2009 23:32:50 GMThttps://core.trac.wordpress.org/ticket/11257#comment:19
https://core.trac.wordpress.org/ticket/11257#comment:19
<p>
Hi scribu, using the new Test Case, I am able to fail the test using the old regexp, and pass it using the new regexp, on two different servers. The patch is also installed and running on my 2.8.4 site with no problems.
</p>
TicketazaozzMon, 30 Nov 2009 11:59:40 GMThttps://core.trac.wordpress.org/ticket/11257#comment:20
https://core.trac.wordpress.org/ticket/11257#comment:20
<p>
Not sure if we need to use the same regex there, no need to capture all the stuff that's needed for parsing the shortcodes.
</p>
<p>
We are only looking for &lt;p&gt; before and &lt;/p&gt; after. Perhaps it would be better to build another pattern using the global <tt>$shortcode_tags</tt>, even split it in two. Wouldn't be affected by future changes to get_shortcode_regex() and will be faster.
</p>
TicketmiqrogrooveMon, 30 Nov 2009 16:16:15 GMTattachment sethttps://core.trac.wordpress.org/ticket/11257
https://core.trac.wordpress.org/ticket/11257
<ul>
<li><strong>attachment</strong>
set to <em>ticket-11257-v3.patch</em>
</li>
</ul>
<p>
Alternative patch design with changes suggested by azaozz.
</p>
TicketazaozzTue, 01 Dec 2009 07:46:39 GMThttps://core.trac.wordpress.org/ticket/11257#comment:21
https://core.trac.wordpress.org/ticket/11257#comment:21
<p>
(In <a class="changeset" href="https://core.trac.wordpress.org/changeset/12302" title="Separate the removal of &lt;p&gt; wrapping from shortcodes into another function ...">[12302]</a>) Separate the removal of &lt;p&gt; wrapping from shortcodes into another function and apply it with different filter, props miqrogroove, props mdawaffe, see <a class="closed ticket" href="https://core.trac.wordpress.org/ticket/11257" title="defect (bug): Reproducible Output Failure (closed: fixed)">#11257</a>, see <a class="closed ticket" href="https://core.trac.wordpress.org/ticket/11249" title="enhancement: wpautop no longer a standalone function (closed: fixed)">#11249</a>
</p>
TicketryanTue, 01 Dec 2009 20:40:20 GMTseverity changedhttps://core.trac.wordpress.org/ticket/11257#comment:22
https://core.trac.wordpress.org/ticket/11257#comment:22
<ul>
<li><strong>severity</strong>
changed from <em>critical</em> to <em>normal</em>
</li>
</ul>
<p>
Downgrading severity since it seems like the critical part is taken care of.
</p>
TicketmiqrogrooveTue, 01 Dec 2009 20:58:09 GMThttps://core.trac.wordpress.org/ticket/11257#comment:23
https://core.trac.wordpress.org/ticket/11257#comment:23
<p>
ryan, are you sure shortcodes.php isn't going to NULL-out the content right after wpautop() finishes? I haven't tested the patch piecemeal like this.
</p>
TicketmiqrogrooveTue, 01 Dec 2009 21:08:23 GMThttps://core.trac.wordpress.org/ticket/11257#comment:24
https://core.trac.wordpress.org/ticket/11257#comment:24
<p>
OK it does pass the test case without the second half of the patch. The shortcodes.php portion will just be a performance boost until someone fixes <a class="closed ticket" href="https://core.trac.wordpress.org/ticket/8553" title="defect (bug): preg_replace_callback in do_shortcode returns empty for large posts (closed: fixed)">#8553</a>
</p>
TicketryanMon, 07 Dec 2009 21:03:01 GMTmilestone changedhttps://core.trac.wordpress.org/ticket/11257#comment:25
https://core.trac.wordpress.org/ticket/11257#comment:25
<ul>
<li><strong>milestone</strong>
changed from <em>2.9</em> to <em>3.0</em>
</li>
</ul>
TicketmiqrogrooveTue, 08 Dec 2009 02:04:59 GMTstatus, milestone changed; resolution sethttps://core.trac.wordpress.org/ticket/11257#comment:26
https://core.trac.wordpress.org/ticket/11257#comment:26
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>milestone</strong>
changed from <em>3.0</em> to <em>2.9</em>
</li>
</ul>
<p>
Punting the performance boost part of this patch seems like a waste of time. Can we call it fixed and move on to something that's still broken?
</p>
TicketmiqrogrooveTue, 08 Dec 2009 02:05:50 GMTseverity changedhttps://core.trac.wordpress.org/ticket/11257#comment:27
https://core.trac.wordpress.org/ticket/11257#comment:27
<ul>
<li><strong>severity</strong>
changed from <em>normal</em> to <em>major</em>
</li>
</ul>
Ticket