search

xwp / wp-dependency-minification

Dependency Minification plugin for WordPress
http://wordpress.org/plugins/dependency-minification/
52 stars 10 forks source link

Skip SSL verification for IIS support. #58

Open msigley opened 10 years ago

msigley commented 10 years ago

Fixes issue #57.

westonruter commented 10 years ago

@msigley many would probably want sslverify left intact.

Perhaps it would be better to just throw in another mu-plugin which does:

$minify_action_priority = 10;

add_action( Dependency_Minification::CRON_MINIFY_ACTION, function () {
    add_filter( 'https_local_ssl_verify', '__return_false' );
    add_filter( 'https_ssl_verify', '__return_false' );
}, $minify_action_priority - 1 );

add_action( Dependency_Minification::CRON_MINIFY_ACTION, function () {
    remove_filter( 'https_local_ssl_verify', '__return_false' );
    remove_filter( 'https_ssl_verify', '__return_false' );
}, $minify_action_priority + 1 );

Too bad that https_ssl_verify and https_local_ssl_verify don't pass the $url as an additional filter arg, and then these could be used target those specific JS requests.

msigley commented 10 years ago

We could also do an $is_IIS check as well and only disable the sslverify option in the HTTP request if IIS is detected. http://codex.wordpress.org/Global_Variables The biggest issue is for some reason the default WIMP stack installed on Windows via the Web PI service doesn't install the dependencies necessary for SSL verification via PHP because IIS servers don't use OpenSSL (yeah no heartbleed). You are fairly safe to assume 99% of people running PHP with IIS aren't running OpenSSL, so disabling sslverify for all IIS users is a fair fix.

Let me know the direction you wish to go in and I will revise or revoke my pull request.

westonruter commented 10 years ago

@msigley does my plugin above not do the trick to turn off SSL verification on your site?

msigley commented 10 years ago

It does, but this is still a IIS compatibility issue in the plugins implementation. Again virtually anyone running IIS will experience this issue.

westonruter commented 10 years ago

@msigley I was trying to apply your patch to the develop branch, because there was some cleanup and refactoring going on there which is still not merged. If you could open another PR to merge into the develop branch that would be great.

Also, before doing another plugin release (1.0), I want to get unit tests in place (#6) so that we can check for regressions.