Analytics Posts Endpoint: Fix incorrect validation logic #3454
Conversation
📝 WalkthroughWalkthroughThe changes update how 'author' and 'section' query parameters are handled in both REST API route registration and endpoint URL generation. Validation, sanitization, and multi-value handling for these parameters are removed, simplifying them to single string filters. Multi-value handling for the 'tag' parameter remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST_API
participant Analytics_Service
Client->>REST_API: Request posts (author/section/tag as query params)
REST_API->>Analytics_Service: Forward request (author/section as single string, tag as comma-separated)
Analytics_Service-->>REST_API: Return filtered posts
REST_API-->>Client: Return response
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/rest-api/stats/class-endpoint-posts.php (1)
168-173: Description should mention the 5-tag cap for consistency.The parameter still enforces
validate_max_length_is_5, but the description omits that limit. A tiny copy tweak prevents surprises for API consumers.- 'description' => 'The tags to filter by (in comma-separated format).', + 'description' => 'The tags to filter by (comma-separated, max 5).',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/rest-api/stats/class-endpoint-posts.php(1 hunks)src/services/content-api/endpoints/class-endpoint-analytics-posts.php(0 hunks)
💤 Files with no reviewable changes (1)
- src/services/content-api/endpoints/class-endpoint-analytics-posts.php
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the HTML and PHP code to ensure it is well-structured and adheres ...
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/rest-api/stats/class-endpoint-posts.php
🔇 Additional comments (1)
src/rest-api/stats/class-endpoint-posts.php (1)
158-166: Add sanitization & validation forauthor/sectionparameters.Switching these args from multi-value to single strings is correct, but they now arrive unsanitized. Because the value is later interpolated into the upstream Content API URL, any unexpected characters (spaces,
&,%, etc.) could break the request or open the door to header-injection style issues.
WordPress best-practice is to run user-supplied strings throughsanitize_text_field()(or stricter) and, optionally, a lightweight validator.- 'author' => array( - 'description' => 'The author to filter by.', - 'type' => 'string', - 'required' => false, + 'author' => array( + 'description' => 'The author to filter by.', + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', + 'required' => false, ), - 'section' => array( - 'description' => 'The section to filter by.', - 'type' => 'string', - 'required' => false, + 'section' => array( + 'description' => 'The section to filter by.', + 'type' => 'string', + 'sanitize_callback' => 'sanitize_text_field', + 'required' => false, ),This keeps the new single-value behaviour while hardening the endpoint against malformed input.
[ suggest_essential_refactor ]
…cs-posts-endpoint
…-logic-in-analytics-posts-endpoint" (fe0fa4a)
Description
Our validation logic in
class-endpoint-posts.phpwould allow passing multipleauthorandsectionparameters. However, the receiving API only supports one of each, so passing many of these would end up in unwanted results.This PR changes this behavior so only one
authorandsectioncan be passed to the endpoint.Motivation and context
Sync our validation logic to match the spec of the API we're using.
How has this been tested?
Manually tested by step debugging and seeing that the API URL was correctly being formed. Also gave it some local testing using PCH Related Posts.
Summary by CodeRabbit