Ensure check_rest_api_enabled respects mainwp_sslVerifyCertificate op… #836
Conversation
WalkthroughAdds SSL certificate verification control to the REST API availability check by introducing an sslverify option in the HTTP args of check_rest_api_enabled(). Request arg formatting adjusted; control flow, endpoints, and parsing remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Plugin as check_rest_api_enabled()
participant WPHTTP as wp_remote_post
participant WP as WordPress Options
Caller->>Plugin: check_rest_api_enabled($site_url, $check_logged_in)
Plugin->>WP: get_option('mainwp_sslVerifyCertificate', true)
WP-->>Plugin: bool sslverify
alt $check_logged_in && cookies exist
Plugin->>Plugin: Attach cookies to args
end
Plugin->>WPHTTP: POST $site_url/wp-json with args {method: GET, timeout:45, headers, sslverify}
WPHTTP-->>Plugin: Response
alt Response has data.routes
Plugin-->>Caller: true
else Response invalid and !$check_logged_in
Plugin->>Plugin: Recurse with $check_logged_in = true
Plugin-->>Caller: result
else
Plugin-->>Caller: false
end
Note over Plugin: New: sslverify is toggled via WP option
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/page-mainwp-rest-api-page.php (1)
1236-1236: Use logical AND (&&), not bitwise &.Bitwise & evaluates both sides and can trigger notices on non-arrays; && short-circuits and is the intended logic.
Apply:
- if ( is_array( $data ) & isset( $data['routes'] ) && ! empty( $data['routes'] ) ) { + if ( is_array( $data ) && isset( $data['routes'] ) && ! empty( $data['routes'] ) ) {
🧹 Nitpick comments (2)
pages/page-mainwp-rest-api-page.php (2)
1219-1225: Good addition; parse the option to a real boolean.Casting may misread values like "no"/"false" as true. Use FILTER_VALIDATE_BOOLEAN for robustness.
Apply:
- 'sslverify' => (bool) get_option( 'mainwp_sslVerifyCertificate', true ), + 'sslverify' => filter_var( get_option( 'mainwp_sslVerifyCertificate', true ), FILTER_VALIDATE_BOOLEAN ),
1203-1216: Guard LOGGED_IN_COOKIE usage.Avoid notices when the cookie is missing by checking existence and initializing the variable.
Apply:
- if ( $check_logged_in && is_user_logged_in() && defined( 'LOGGED_IN_COOKIE' ) ) { - $cookies = array(); - $auth_cookies = wp_parse_auth_cookie( $_COOKIE[ LOGGED_IN_COOKIE ], 'logged_in' ); // phpcs:ignore -- ok. + if ( $check_logged_in && is_user_logged_in() && defined( 'LOGGED_IN_COOKIE' ) ) { + $cookies = array(); + $auth_cookies = null; + if ( isset( $_COOKIE[ LOGGED_IN_COOKIE ] ) ) { + $auth_cookies = wp_parse_auth_cookie( $_COOKIE[ LOGGED_IN_COOKIE ], 'logged_in' ); // phpcs:ignore -- ok. + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
pages/page-mainwp-rest-api-page.php(1 hunks)
| 'method' => 'GET', | ||
| 'timeout' => 45, | ||
| 'headers' => array( | ||
| 'content-type' => 'application/json', | ||
| ), | ||
| 'sslverify' => (bool) get_option( 'mainwp_sslVerifyCertificate', true ), | ||
| ); |
There was a problem hiding this comment.
Use wp_remote_get + rest_url() and drop the method override.
wp_remote_post() forces POST, so 'method' => 'GET' is ignored. The REST index expects GET; POST can yield rest_no_route. Also no need to send Content-Type for a GET.
Apply:
- $args = array(
- 'method' => 'GET',
- 'timeout' => 45,
- 'headers' => array(
- 'content-type' => 'application/json',
- ),
- 'sslverify' => filter_var( get_option( 'mainwp_sslVerifyCertificate', true ), FILTER_VALIDATE_BOOLEAN ),
- );
+ $args = array(
+ 'timeout' => 45,
+ 'sslverify' => filter_var( get_option( 'mainwp_sslVerifyCertificate', true ), FILTER_VALIDATE_BOOLEAN ),
+ );
…
- $site_url = get_option( 'home' );
- $response = wp_remote_post( $site_url . '/wp-json', $args );
+ $response = wp_remote_get( rest_url(), $args );
+ if ( is_wp_error( $response ) ) {
+ return false;
+ }Also applies to: 1231-1233
🤖 Prompt for AI Agents
In pages/page-mainwp-rest-api-page.php around lines 1219-1225 (and also apply
same change for lines 1231-1233), the code uses wp_remote_post with a 'method'
=> 'GET' override and sets a Content-Type header for a GET; wp_remote_post
forces POST so the override is ignored which can trigger rest_no_route. Replace
wp_remote_post with wp_remote_get, remove the explicit 'method' entry and drop
the 'content-type' header (no Content-Type needed for GET), keep timeout and
sslverify options as-is, and update any variable names or call sites if
necessary to reflect using wp_remote_get.
3 participants
When using MainWP REST API I expect it to respect the setting "Verify SSL certificate" (which translates to option "mainwp_sslVerifyCertificate").
However, the wp_remote_post used to check if the REST API is enabled does not follow the settings and will always try to verify the SSL certificate. This is particularly annoying in case one's using a self-signed certificate
Summary by CodeRabbit