#11257 closed defect (bug) (fixed)
Reproducible Output Failure
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 2.9 | Priority: | high |
Severity: | major | Version: | 2.8.4 |
Component: | Shortcodes | Keywords: | has-patch |
Focuses: | Cc: |
Description
Steps to reproduce:
- Open any browser and start a new post.
- Paste the attached file into HTML view.
- Click the Preview button.
Expected Result: Preview :)
Actual Result: Blank Post D:
Tested only on the one site so far.
Attachments (5)
Change History (32)
#1
@
14 years ago
- Keywords reporter-feedback added
- Milestone changed from 2.9 to Future Release
Using r12273 and 2.7.1 (first time in a while using that site), I don't have any problems with the attached code.
In any case, moving to Future Release since 2.9 is in beta.
#2
@
14 years ago
What is the URL of the page you are sent to?
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.
#3
@
14 years ago
- Milestone changed from Future Release to 2.9
Hi
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.
scribu, the URL can't be relevant, because I was able to reproduce this on a published post as well as in preview.
This bug is blocking me from reporting some other bugs, so I will be increasing the Severity if I can pinpoint the cause/trigger.
#4
@
14 years ago
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.
#5
@
14 years ago
- Keywords reporter-feedback removed
Problem did not show up on a clean install, nor with the usual suspect plugins.
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.
#6
@
14 years ago
I traced it down to the last line of function wpautop()
$pee = preg_replace('/<p>\s*?(' . get_shortcode_regex() . ')\s*<\/p>/s', '$1', $pee);
I don't have time tonight to debug get_shortcode_regex(), but I'll keep you all posted when I do.
#8
@
14 years ago
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.
#9
@
14 years ago
I've confirmed get_shortcode_regex is not the cause of the problem.
It's starting to look like there is a conflict between the P elements and shortcode matching.
Note the regexp is incorrect for situations like
<p>[shortcode]<p>[shortcode]<p>Hello World</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.
#10
@
14 years ago
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.
I assume you actually checked the depth on your setup? I've seen it set incredibly small :)
If that wasnt your problem (then GREAT!.. because that tickets been open for awhile now)
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.
#11
@
14 years ago
Wait a sec. Doesn't "\2" backtrace the WRONG value?
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.
#13
@
14 years ago
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:
(?:(\/))?
It is replaced by the equivalent non-recursive form:
\\/?
This patch also fixes a related bug wherein wpautop() backreferences the wrong preg match when looking for shortcode close tags like /caption.
#14
@
14 years ago
- Keywords has-patch added
- Priority changed from normal to high
- Severity changed from normal to critical
#16
@
14 years ago
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.
#17
@
14 years ago
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.
#18
@
14 years ago
If you can prove that your patch performs better and doesn't cause any regressions, that would be good enough.
#19
@
14 years ago
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.
#20
@
14 years ago
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.
We are only looking for <p> before and </p> after. Perhaps it would be better to build another pattern using the global $shortcode_tags
, even split it in two. Wouldn't be affected by future changes to get_shortcode_regex() and will be faster.
#22
@
14 years ago
- Severity changed from critical to normal
Downgrading severity since it seems like the critical part is taken care of.
#23
@
14 years ago
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.
#24
@
14 years ago
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 #8553
This post body makes WordPress epic fail.