Make WordPress Core

Opened 12 years ago

Last modified 3 weeks ago

#26649 reopened defect (bug)

escaped shortcodes should not be expanded during 'get_the_excerpt'

Reported by: bobbingwide's profile bobbingwide Owned by:
Milestone: 7.0 Priority: normal
Severity: normal Version: 3.7.1
Component: Shortcodes Keywords: has-patch good-first-bug has-unit-tests early
Focuses: Cc:

Description

It is possible for "the_content" filter to be invoked while processing "get_the_excerpt".

If the do_shortcodes() filter hook is attached to both "the_content" and "get_the_excerpt" then this can lead to an unexpected expansion of an escaped shortcode.

This can lead to unwanted side effects, as reported here.
http://www.oik-plugins.com/2013/12/escaped-shortcodes-unexpectedly-expanded-get_the_excerpt/

This minor problem can be alleviated by a simple change to strip_shortcode_tag(), returning the HTML code [ as the first character rather than the left square bracket.

function strip_shortcode_tag( $m ) {
	// allow [[foo]] syntax for escaping a tag
	if ( $m[1] == '[' && $m[6] == ']' ) {
		return "[" . substr($m[0], 2, -1) ;
	}
	return $m[1] . $m[6];
}

I don't believe that it's necessary to make the same change in do_shortcode_tag().

Attachments (4)

26649.patch (648 bytes) - added by bobbingwide 12 years ago.
patch for 26649
26649-escaped-shortcodes-fix.patch (841 bytes) - added by sachinrajcp123 7 months ago.
This patch ensures that escaped shortcodes like shortcode? are not expanded or parsed during get_the_excerpt(). 🔹 Problem: Currently, even escaped shortcodes may get processed and output unexpectedly in post excerpts. 🔹 Fix: The patch updates shortcode_unautop() to correctly detect and preserve escaped shortcodes by treating them as plain text. 🔹 Benefit: This allows users to display shortcode syntax examples in content or documentation without triggering actual shortcode output. This fix improves shortcode handling and respects user intent when using double square brackets.
26649-fixed.patch (973 bytes) - added by sachinrajcp123 5 months ago.
escaped-shortcodes-get-the-excerpt.patch (535 bytes) - added by sachinrajcp123 8 weeks ago.

Download all attachments as: .zip

Change History (43)

@bobbingwide
12 years ago

patch for 26649

#1 @SergeyBiryukov
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #19927.

#2 follow-up: @bobbingwide
12 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Hi Sergey. Thanks for finding 19927 for me. I wondered why my patch had two changes.
The change in 19927 is for do_shortcode_tag()
The second part of the change, in 26649, is for strip_shortcode_tag().

The problem is not exactly a duplicate as the fix for 26649 handles the situation where shortcodes are being removed.

The patch for 26649 will work for 19927 but not vice-versa.
How should this be addressed?

#3 in reply to: ↑ 2 @SergeyBiryukov
12 years ago

  • Milestone set to Awaiting Review

Replying to bobbingwide:

The patch for 26649 will work for 19927 but not vice-versa.

I see, thanks for the clarification. Let's close #19927 in favor if this ticket then.

#4 @SergeyBiryukov
12 years ago

#19927 was marked as a duplicate.

#6 @chriscct7
10 years ago

  • Keywords needs-patch needs-unit-tests added

#7 @dd32
14 months ago

I tend to agree that simply encoding the shortcode as text makes sense, as that's what the string is after strip_shortcodes() has run.

I would not expect the encoded shortcode to be completely removed, nor would I expect it to be escaped still, so encoding it such that do_shortcode( strip_shortcode( '[[example]]' ) ) doesn't execute the example shortcode makes sense to me.

#8 @johnbillion
14 months ago

  • Keywords has-patch needs-refresh good-first-bug added; needs-patch removed
  • Milestone set to Future Release

This ticket was mentioned in PR #8221 on WordPress/wordpress-develop by @sukhendu2002.


14 months ago
#9

  • Keywords needs-refresh removed

#10 @sukhendu2002
14 months ago

  • Keywords needs-refresh added

This ticket was mentioned in Slack in #core by michaelwp85. View the logs.


14 months ago

This ticket was mentioned in PR #8231 on WordPress/wordpress-develop by @michaelwp85.


14 months ago
#12

  • Keywords has-unit-tests added; needs-unit-tests needs-refresh removed

Converts escaped shortcodes [[example]] into [example] to prevent do_shortcode from ever executing the shortcode.

Trac ticket: https://core.trac.wordpress.org/ticket/26649

#13 @michaelwp85
14 months ago

I've created a pull request with a fix and updated unit tests.

There is one scenario in which I revert the html entities back into brackets, this is when an escaped shortcode is used inside an attribute. Specifically for this test scenario: <div style="selector:url([[gallery]])">
If I do not revert the HTML entities will interfere with the processing of safecss_filter_attr stripping the code.

#14 @michaelwp85
14 months ago

All builds passed so I guess this can be tested :)
This is my first core contribution so any feedback is welcome.

This ticket was mentioned in Slack in #core by michaelwp85. View the logs.


13 months ago

#16 @michaelwp85
13 months ago

I would love to get this ticket to a closed state. The PR has been reviewed and approved. Is there anything blocking this from being merged? Can I do something to get this to move forward?

This ticket was mentioned in Slack in #core by michaelwp85. View the logs.


13 months ago

#18 @johnbillion
13 months ago

  • Keywords early added
  • Milestone changed from Future Release to 6.9

@dd32 Is this something you can take a look at for 6.9?

#19 @devsahadat
12 months ago

The issue is valid, and the proposed fix correctly prevents escaped shortcodes from executing unexpectedly. I have tested it, and it works as expected, handling edge cases without side effects.

The approach aligns with strip_shortcodes(), ensuring consistent behavior.

This ticket was mentioned in PR #8717 on WordPress/wordpress-develop by @dilip2615.


11 months ago
#20

Fixes #26649.

This changeset prevents escaped shortcodes (e.g., [[shortcode]]) from being incorrectly expanded when generating excerpts using get_the_excerpt().

  • Temporarily removes all registered shortcodes before applying the get_the_excerpt filters.
  • Restores the original shortcode handlers after excerpt generation.
  • Ensures escaped shortcodes display correctly without being processed.

#21 @logicrays
11 months ago

Hello
I've tested the fix, and it correctly prevents shortcode? from executing in get_the_excerpt(). The updated strip_shortcode_tag() handles escaping as expected:

function strip_shortcode_tag( $m ) {
    if ( $m[1] == '[' && $m[6] == ']' ) {
        return '&#91;' . substr($m[0], 2, -1);
    }
    return $m[1] . $m[6];
}

@sachinrajcp123
7 months ago

This patch ensures that escaped shortcodes like shortcode? are not expanded or parsed during get_the_excerpt(). 🔹 Problem: Currently, even escaped shortcodes may get processed and output unexpectedly in post excerpts. 🔹 Fix: The patch updates shortcode_unautop() to correctly detect and preserve escaped shortcodes by treating them as plain text. 🔹 Benefit: This allows users to display shortcode syntax examples in content or documentation without triggering actual shortcode output. This fix improves shortcode handling and respects user intent when using double square brackets.

This ticket was mentioned in Slack in #core by nagpai. View the logs.


5 months ago

This ticket was mentioned in PR #10433 on WordPress/wordpress-develop by @micahele.


5 months ago
#23

Modified strip_shortcode_tag() to return HTML entities (&#91;gallery&#93;) instead of raw brackets

Trac ticket: https://core.trac.wordpress.org/ticket/26649

#24 @micahele
5 months ago

I allowed for the strip_shortcode_tag() function to return HTML entities (&#91;gallery&#93;) for both the opening and closing bracket to allow for more consistency

#25 follow-up: @wildworks
4 months ago

  • Milestone changed from 6.9 to 7.0

Since the 6.9 RC1 release is coming soon, I will punt this ticket to 7.0.

#26 in reply to: ↑ 25 ; follow-up: @michaelwp85
4 months ago

Replying to wildworks:

Since the 6.9 RC1 release is coming soon, I will punt this ticket to 7.0.

What is the reason to postpone instead of including it as planned?

#27 in reply to: ↑ 26 ; follow-up: @wildworks
4 months ago

  • Milestone changed from 7.0 to 6.9

Replying to michaelwp85:

Replying to wildworks:

Since the 6.9 RC1 release is coming soon, I will punt this ticket to 7.0.

What is the reason to postpone instead of including it as planned?

Oh sorry, I overlooked the fact that PR 8231 has already been approved. However, since more than six months have passed since it was approved, it might need to be tested again.

@dd32 Is it possible to commit this PR to the 6.9 release? Also, a new PR has been submitted; do you have any feedback on that?

#28 in reply to: ↑ 27 @michaelwp85
4 months ago

Replying to wildworks:

Oh sorry, I overlooked the fact that PR 8231 has already been approved. However, since more than six months have passed since it was approved, it might need to be tested again.

The PR has unit tests :)

#29 @wildworks
4 months ago

@michaelwp85 Just in case, I've merged the latest upstream changes into your PR branch.

#30 @dd32
4 months ago

  • Milestone changed from 6.9 to 7.0

Punting to 7.0, this needs to be included early in a release cycle, not during beta.

The early keyword should signify to committers to review this after the 6.9 release to see if it's something they're willing to pick up.

I'm not personally able to shepperd this into core though, due to focuses elsewhere.

Version 0, edited 4 months ago by dd32 (next)

This ticket was mentioned in Slack in #core by roelof. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by juanmaguitar. View the logs.


5 weeks ago

#33 @juanmaguitar
5 weeks ago

  • Keywords needs-testing added

(from today's WP 7.0 Bug Scrub)

https://github.com/WordPress/wordpress-develop/pull/8231 is the valid and approved fix for this ticket. It needs testing.

Last edited 5 weeks ago by juanmaguitar (previous) (diff)

#34 @huzaifaalmesbah
5 weeks ago

  • Keywords needs-testing removed

Patch Testing Report

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/8231

Environment

  • WordPress: 7.0-alpha-61215-src
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.7 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 144.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins:
    • Reproduce Issue 26649 1.0
  • Plugins:
    • Test Reports 1.2.1

Steps taken

  1. Created an MU plugin to reproduce the issue.
  2. The plugin registers a shortcode [test_shortcode_26649] that expands to "SHORTCODE EXPANDED".
  3. The plugin simulates content processing where [[test_shortcode_26649]] is passed to strip_shortcodes and then the_content filter.
  4. Without the patch, strip_shortcodes returns [test_shortcode_26649], which the_content then expands.
  5. With the patch, strip_shortcodes returns &#91;test_shortcode_26649&#93;, preventing expansion.
  6. ✅ Patch is solving the problem.

Expected result

  • We expect the escaped shortcode [[test_shortcode_26649]] to be displayed as [test_shortcode_26649] (or its HTML entity equivalent) in the exempt/content, and NOT be expanded to "SHORTCODE EXPANDED".

Additional Notes

  • The patch modifies strip_shortcode_tag to return HTML entities for brackets, effectively neutralizing specific shortcode tags while preserving their visual representation.

Screenshots/Screencast with results

This video demonstrates the issue before the patch and the resolution after applying the patch: https://files.catbox.moe/7yvtq4.mp4

Support Content

#### MU Plugin Code

/**
 * Plugin Name: Reproduce Issue 26649
 * Description: Reproduces the issue where escaped shortcodes are expanded in get_the_excerpt.
 * Version: 1.0
 */

// 1. Register a test shortcode
add_shortcode( 'test_shortcode_26649', function() {
    return 'SHORTCODE EXPANDED';
} );

// 2. Add an admin notice to display the test result
add_action( 'admin_notices', function() {
    $content = 'Testing escaped shortcode: [[test_shortcode_26649]]';
    
    // Simulate the flow that causes the issue:
    // 1. strip_shortcodes is called (e.g. by wp_trim_excerpt)
    // 2. the_content filter is applied (which runs do_shortcode)
    
    $stripped = strip_shortcodes( $content );
    
    // We ensure 'do_shortcode' is hooked (default behavior)
    $output = apply_filters( 'the_content', $stripped );
    
    // Check if the shortcode was expanded
    $failed = strpos( $output, 'SHORTCODE EXPANDED' ) !== false;
    
    $color = $failed ? 'red' : 'green';
    $message = $failed ? 'FAILED: Shortcode was expanded!' : 'PASSED: Shortcode was NOT expanded.';
    
    echo '<div class="notice notice-info is-dismissible">';
    echo '<h3>Ticket 26649 Reproduction</h3>';
    echo '<p><strong>Input:</strong> ' . htmlspecialchars( $content ) . '</p>';
    echo '<p><strong>After strip_shortcodes:</strong> ' . htmlspecialchars( $stripped ) . '</p>';
    echo '<p><strong>Final Output (after the_content filter):</strong> ' . htmlspecialchars( $output ) . '</p>';
    echo '<p style="color: ' . $color . '; font-weight: bold;">' . $message . '</p>';
    if ( $failed ) {
        echo '<p><em>The bug triggers because <code>strip_shortcodes</code> turns <code>[[...]]</code> into <code>[...]</code>, which is then caught by <code>do_shortcode</code> in <code>the_content</code> filter.</em></p>';
    }
    echo '</div>';
} );


This ticket was mentioned in Slack in #core by juanmaguitar. View the logs.


5 weeks ago

#37 @wildworks
3 weeks ago

Although multiple patches and PRs have been submitted, but only https://github.com/WordPress/wordpress-develop/pull/8231 that have already been approved should be tested. Let me close all other PRs.

@wildworks commented on PR #8717:


3 weeks ago
#38

Thanks for the PR! Although multiple PRs have been submitted, but let me close this PR in order to prioritize #8231, which has already been approved.

@wildworks commented on PR #10433:


3 weeks ago
#39

Thanks for the PR! Although multiple PRs have been submitted, but let me close this PR in order to prioritize #8231, which has already been approved.

This ticket was mentioned in Slack in #core-test by ozgur_sar. View the logs.


3 weeks ago

Note: See TracTickets for help on using tickets.