markjaquith/WP-Stack

Detect or enable SSL CDN?

vidluther opened this issue · 11 comments

It'd be nice if the wp-stack-cdn plugin could detect SSL. When I specify WP_STACK_CDN_DOMAIN, 'https://cdn.zippykid.com'); The final string turns out to be http://https://cdn.zippykid.com, if I don't specify the scheme, it forces http..

Not sure what the best way to do this is, maybe a third option WP_STACK_CDN_SSL, true, which forces the filter to use ssl?

Hey @vidluther! I just ran into this same issue. The regex for the replacement always replaces the URL with "http://". I think the easiest solution is to just remove the "http://" and replace it with "//", which will produce protocol aware URLs.

I am interested in why @markjaquith made this work for "http" only. I'm guessing that SSL was not required for the project in which this originated and thus it was never addressed. The patch should be as easy as deleting the 5 characters.

@vidluther we use an is_ssl check and change the output an see it on the gist below. Not sure its the best fix but it works well for us.

https://gist.github.com/joshlyford/8700675

Thanks @joshlyford , @tollmanz does that work for you as well?

The is_ssl() approach will definitely work; however, there are some potential pitfalls. If your page caching solution does not take into account the page protocol when caching the page (i.e., treats https://mysite.com and http://mysite.com as the same), you will end up with mixed content warnings if the first person to visit your page is using http and the second user uses https. I recently learned that batcache has issues with this.

My solution would use a protocol relative URL, which allows you to use the same URL regardless of whether the current page is http or https. You can take a look at my diff.

Confirmed that the project I originally wrote it for did not require SSL. I like the protocol-relative trick. If you're serving up SSL pages, you'll need an SSL-capable CDN to avoid mixed content warnings, so I'm okay with this assumption.

@markjaquith any chance we can re-open this for discussion?

I think the fix that @joshlyford put forward by using is_ssl() may be a better solution... although as @tollmanz points out there may be problems.

After having used the protocol relative fix in a "wide" scale for a few weeks, there are quite a few issues with some third party tools that use content from the sites:

  • Facebook Open Graph doesn't allow them
  • Some RSS Feed Readers don't support them
  • Some email/newsletter tools don't support them
  • Google News doesn't support them

Granted, the URLs are RFC compliant (http://tools.ietf.org/html/rfc3986), so it's a tough call.

@markjaquith +1 to @phikai's notes.

We're struggling w/ the relative protocol URLs in Mailchimp RSS campaigns, as well as Open Graph Facebook images. I'm all for the new standard, but even though the browser support is good, it seems many applications aren't there yet.

This is a bummer :( It's weird that those services won't respect those URLs.

although as @tollmanz points out there may be problems.

Just note that the issues I mentioned are fairly easily mitigated if your caching solution is aware of SSL vs. non-SSL requests. There are a host of issue you'll likely face if it is not, so it's probably better to patch that first.

Our caching solution is batcache... which means we're waiting on your PR there (Automattic/batcache#25).

@phikai I think that the issue has been fixed with Automattic/batcache@484c7b9. @Viper007Bond dumped what looks to be a few years of code into that commit, which syncs up what they are running on WordPress.com with the released version. I have not updated to that version, so cannot speak to whether or not it fixes the issue. Worse case scenario, you can simply patch Batcache with my PR and you should be set!

Yeah, the version that was here on GitHub was 4 or 5 years out of date. I dumped what we're currently running on WP.com onto GitHub in that commit.

I can't speak to whether that issue is fixed or not. Closer examination would be required.