0

We're using Advanced Custom Fields in our company and sometimes we need to use shortcode in our custom fields. In the php code we then use the do_shortcode functions for these fields. There is no reason to not use it for most fields, as it could be we want to add shortcodes to a field later.

We're also using PHP CodeSniffer with the WordPress Coding Standards rule. Whenever we use echo do_shortcode the escaping warning is gone. So is it safe to just use do_shortcode or do we need to use e.g. wp_kses_post additionally?

Is there any best practice on that?

Thanks for any advice

  • do_shortcode is not for escaping. – Joel M Jan 28 at 8:45
  • Hm okay, but why does codesniffer then not show any warning? – maysi Jan 28 at 12:18
  • Perhaps it does not show a warning because the shortcode rendering function should be responsible for sanitizing its output. But any string can be passed to the function and if it doesnt contain registered shortcodes then i'm pretty sure the string will not be modified at all. It is definitely not a generic sanitation method. The code sniffer can only do so much. Read here on how to sanitize data properly. There are many functions, the ones I use a lot are esc_attr, sanitize_text_field, esc_url. codex.wordpress.org/… – Joel M Jan 28 at 16:41
0

The WordPress Coding Standards sniffs treat do_shortcode() as an "autoescaped function". This appears to have been discussed in 2015 in these GitHub issues:

https://github.com/WordPress/WordPress-Coding-Standards/issues/167 https://github.com/WordPress/WordPress-Coding-Standards/issues/428

The explanation used when it was added to the list was:

I discussed this with VIP support (#44195). David, after conferring with another team member, said that it's unnecessary, as we do the escaping where the HTML is emitted (in the shortcode's code).

But I'm not sure I agree with the reasoning. If you used it like this:

<?php echo do_shortcode( '[liveperson]' ); ?>

Where the shortcode is hard-coded, it makes sense, because the only thing to escape is the output of the shortcode, which should be escaped by the shortcode callback.

However, in your situation, you are using do_shortcode() to allow execution of shortcodes within user-supplied text, from a custom field. That text does need to be escaped, but do_shortcode() does not do any actual escaping.

So the safe way to handle fields like this would be to put the text through wp_kses() or wp_kses_post(), to remove disallowed tags, and then put it through do_shortcode() so that the shortcodes can be executed without being escaped again.

$text = get_field( 'field_name' );

echo do_shortcode( wp_kses_post( $text ) );

It won't make any difference to what the code sniffer does/does not report, but at least the user supplied text is sanitised.

One limitation is that it won't work if you use esc_html(), because that will interfere with shortcode attributes. This sanitisation also won't prevent users entering unbalanced tags that could affect the layout, which is one thing escaping normally helps with. To address that issue you could add force_balance_tags():

$text = get_field( 'field_name' );

echo do_shortcode( force_balance_tags( wp_kses_post( $text ) ) );
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.