Skip to content

Conversation

@merkushin
Copy link
Contributor

@merkushin merkushin commented Jan 13, 2025

Resolves https://github.com/Automattic/sensei-security/issues/24

Proposed Changes

  • Use wp_strip_all_tags instead of a discouraged strip_tags

Testing Instructions

  1. Not all the changes is possible to test on a working website. Please, inspect the changes.
  2. Go to Sensei LMS -> Questions.
    • Add a question with arbitrary data.
    • Go to DB and in {prefix}_terms table and change the name of the term to have tags, for example: UPDATE sensei_test.wp_termsSETname= 'multiple-<b>choice</b>',slug= 'multiple-<b>choice</b>' WHEREterm_id = 13; (Make sure the resulting string after stripping is the same as the original, so it shouldn't be something like multiple-choice<b>123</b>.)
    • Go to Sensei LMS -> Questions and make sure you see the type for your question (should be "Multiple Choice", not "—").
    • Revert changes in DB.
  3. Go to Sensei LMS -> Questions -> Question Categories.
    • Add a new category.
    • Assign the category to your question.
    • Go to DB and adjust the category name to have tags. Something like: UPDATE sensei_test.wp_termsSETname= '<b>test1</b>' WHEREterm_id = 14;
    • Go to Sensei LMS -> Questions and make sure you see the category in the list for your question without tags.
    • Don't revert these changes in DB for now.
  4. Install and enable Classic Editor.
    • Go to Sensei LMS -> Lessons.
    • Create a new lesson.
    • Find Quiz Questions panel.
    • Click on the "Existing Questions" tab.
    • It is expected to see tags in the category dropdown, but not in the category column.
  5. Activate a theme that supports widgets. For example, Twenty Twenty-One.
  6. Go to Appearance -> Widgets.
  7. It's not possible to test two remaining changes in Sensei_Admin (we don't have settings there that use textarea) and in Sensei_Settings (we strip tags from escaped lines).

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • Code is tested on the minimum supported PHP and WordPress versions

@merkushin merkushin added this to the 4.24.5 milestone Jan 13, 2025
@merkushin merkushin requested a review from a team January 13, 2025 18:51
@merkushin merkushin self-assigned this Jan 13, 2025
@github-actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@merkushin merkushin marked this pull request as ready for review January 13, 2025 22:07
@github-actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@merkushin merkushin requested review from donnapep and m1r0 and removed request for a team January 13, 2025 22:13
Copy link
Member

@m1r0 m1r0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works as described. 👍

@merkushin merkushin merged commit aaea95e into trunk Jan 14, 2025
22 checks passed
@merkushin merkushin deleted the fix/use-wp-strip-all-tags branch January 14, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants