-
Notifications
You must be signed in to change notification settings - Fork 3.2k
50568 improve wp term's sanitization calls #9349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
50568 improve wp term's sanitization calls #9349
Conversation
The primary goal of this diff is to enhance the `sanitize_term()` function and related code to make the sanitization of term data more robust, consistent, and efficient. It also fixes several bugs and edge cases in how terms were sanitized in various contexts.
**Key Changes**
1. **`sanitize_term()` Enhancements:**
* **Context-Specific Sanitization:** The core of the changes revolves around ensuring that terms are correctly sanitized based on the specified `$context` ('edit', 'db', 'display', 'rss', etc.).
* **Filter Tracking:** A new `filter` property is added to the term object (or array) to keep track of the context in which it was last sanitized. This helps prevent redundant sanitization and ensures the correct filters are applied.
* **Object/Array Handling** The code has been updated to handle both array and object more carefully.
* **Raw Context is now handled by WP_Term:** The new function `WP_Term->filter('raw')` now properly returns the raw term data.
* **Slug Sanitization**: The slug is now sanitized using sanitize_title in multiple contexts.
* **Description Sanitization in 'db' context:** The Description is now more carefully sanitized in 'db' context, it is not simply stripped of HTML, but content in scripts is removed.
* **Sanitization in display context:** The code now escapes html when using the display context.
* **Sanitization in 'rss' context:** The code has been modified to stop stripping html when in rss context.
* **Improved Code Clarity:** The code within `sanitize_term()` and `sanitize_term_field()` is more organized and easier to understand.
2. **`sanitize_term_field()` Refinements:**
* **Context-Aware Filters:** The logic for applying filters in `sanitize_term_field()` is improved to apply the appropriate filters for each context.
* **'db' Context:** The 'db' context now more aggressively strips HTML and potentially harmful content.
* **'edit' Context:** The 'edit' context HTML-encodes data to make it safe for display in input fields.
* **'display' Context:** Now does html escaping.
* **Slug Sanitization** The slug is now sanitized in all contexts, which should help prevent a wide range of unexpected data.
* **Rss Context**: Html is not stripped when in this context.
3. **`WP_Term` Class Changes:**
* **`filter()` Method:** The `WP_Term` class now has a dedicated `filter()` method to apply sanitization to a `WP_Term` object.
* **`get_instance()` Improvements:** The logic in `get_instance()` is updated to handle term objects with different filter states.
* **Filter Property**: This property now accurately reflects the state of the object.
4. **New Unit Tests:**
* **`Tests_Term_SanitizeTerm`:** A comprehensive test suite is added to cover various sanitization scenarios, including different contexts, input types, and potential edge cases. This ensures that the changes are well-tested and don't introduce regressions.
5. **Deprecation of `sanitize_category` and `sanitize_category_field`:**
* These functions are now deprecated in favor of the more general `sanitize_term` and `sanitize_term_field`.
* The functions have been moved to deprecated.php.
6. **Remove redundant sanitization**: `WP_Term->filter()` now checks the current filter, to ensure that the same filters are not applied multiple times.
7. **Fix bug**: An edge case where the slug was not being sanitized when dealing with objects.
**Potential Implications**
* **Enhanced Security:** The more rigorous sanitization, especially in the 'db' and 'edit' contexts, significantly improves security by preventing XSS vulnerabilities and data corruption.
* **Improved Data Integrity:** Sanitizing the slug more thoroughly across contexts enhances data integrity and consistency.
* **Less Unexpected Behavior:** The clear differentiation between contexts should result in more predictable behavior when working with term data.
* **Code Maintainability:** The code structure is improved, making it easier to maintain and extend in the future.
* **Breaking changes:** Deprecating `sanitize_category` and `sanitize_category_field` means that developers will need to start updating their code to use `sanitize_term` and `sanitize_term_field`.
**Code Review Notes**
* **Unit Tests are Excellent:** The new unit tests are thorough and comprehensive. This is a huge plus for ensuring code quality.
* **Code Clarity:** The code is generally well-structured and readable.
* **Slug Sanitization**: The decision to sanitize the slug in multiple contexts is a good one.
* **Description Sanitization**: The changes made to the sanitization of the description is a big improvement.
* **Rss context:** The changes made to the sanitization in the rss context is a big improvement.
**In summary**
This diff is a substantial improvement to term sanitization in WordPress. It addresses several security concerns, enhances data integrity, and makes the code more robust and maintainable. The added unit tests are a significant contribution to the project. I highly recommend merging these changes.
# Conflicts: # src/wp-includes/deprecated.php
…tion-calls' into 50568-Improve-WP_Term's-sanitization-calls # Conflicts: # src/wp-includes/deprecated.php
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @[email protected]. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
white space
… to string and using `absint`.
… to string and using `absint`.
… to string and using `absint`.
|
Most of failed checks here are about unit testing.. |
Trac ticket: https://core.trac.wordpress.org/ticket/50568