WordPress / theme-review-action

Other
31 stars 10 forks source link

Trac message not escaped properly #29

Open dd32 opened 3 years ago

dd32 commented 3 years ago

The message sent to trac is not escaped properly, for example: https://themes.trac.wordpress.org/ticket/99043#comment:2

Screen Shot 2021-05-10 at 11 38 45 am
  1. Path to theme is wp-content/themes/test-theme/ could probably just strip that out and leave it as includes/template-parts.php:413
  2. WP_Hook->do_action should be displayed WP_Hook->do_action
  3. #1 should not be linked, this can be escaped by prefixing # with !

Another option instead of escaping would be for the details to be code blocked or inserted within a html block, which takes care of 2/3 above, but the content would need to be run through nl2br() to add the <br> tags for newlines.

Examples of those approaches:

Screen Shot 2021-05-10 at 11 43 23 am
StevenDufresne commented 3 years ago

I originally went with an HTML approach but had to abandon it. I'm cursing myself for not documenting why. I think it was related to overflow... results ended up being hidden outside of the comments because of long continuous strings, like paths, and misinterpreted whitespace characters?

dd32 commented 3 years ago

Hmm.. Using a code block would result in it overflowing horizontally and being hidden, html blocks seem to work though.

Links were not automatically linked when using a html block though, but something like this appears to work:

-----------
{{{
#!html
<?php echo nl2br( make_clickable( $runner_output ) ); ?>
}}}
StevenDufresne commented 3 years ago

Fortunately I said "I think" so I can't be held liable :). I'll look through the commit history and add my findings to this thread.

StevenDufresne commented 3 years ago

The change was made in .org before the initial commit so I can't find the reason.

It's worth trying again with fresh eyes to confirm the approach. 👍