-
Notifications
You must be signed in to change notification settings - Fork 122
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
Fix for issue #257 #328
base: master
Are you sure you want to change the base?
Fix for issue #257 #328
Conversation
please add fix upstream! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach to solve the issue by setting blocking
to false
will add more issues to logging and to any action that is hooked on to rt_nginx_helper_after_remote_purge_url
. So not approving the PR.
$response = wp_remote_get( $url ); | ||
$request_args = array( | ||
'blocking' => false, | ||
'sslverify' => false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a security issue and sslverify
should not be set to false
at all unless you're testing things locally.
@@ -461,7 +461,11 @@ protected function do_remote_get( $url ) { | |||
*/ | |||
do_action( 'rt_nginx_helper_before_remote_purge_url', $url ); | |||
|
|||
$response = wp_remote_get( $url ); | |||
$request_args = array( | |||
'blocking' => false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this will break the logging functionality and any actions that other plugins are performing on the hook rt_nginx_helper_after_remote_purge_url
since a non blocking request initially starts with status 0, meaning the request is still processing, instead of actual status code like 200 or 404.
#257