PhileCMS / Phile

A flat file CMS with a swappable parser and template engine.
https://philecms.github.io/
Other
257 stars 49 forks source link

[BUGFIX] Nginx: https/http protocol detection #172

Closed DenBeke closed 9 years ago

DenBeke commented 9 years ago

In Nginx the $_SERVER['HTTPS'] is always set, but when it is not https, it will be empty.

In Wordpress they use the following function

function is_ssl() {
    if ( isset($_SERVER['HTTPS']) ) {
        if ( 'on' == strtolower($_SERVER['HTTPS']) )
            return true;
        if ( '1' == $_SERVER['HTTPS'] )
            return true;
    } elseif ( isset($_SERVER['SERVER_PORT']) && ( '443' == $_SERVER['SERVER_PORT'] ) ) {
        return true;
    }
    return false;
}

Checking for the port is not very good, imho, but the first part of the function should be correct. Instead of checking for the variable not to be off, you should check for it to be on.

I have tested it on Nginx with/without https. I have tested it on Apache without https, but haven't any https Apache hosts where I can check it.

So please do some little testing before merging it. I have fixed this for a client, running on my Nginx installation, but I'm not experienced with PhileCMS, so please test a bit more than I have doen :)

STOWouters commented 9 years ago

I can add confirmly that it works for me :+1:

(and yes, I'm that client :smile: )

NeoBlack commented 9 years ago

Please change the line to:

if (!empty($_SERVER['HTTPS']) && strtolower($_SERVER['HTTPS']) !== 'off' ) {

If this work, please update your pull request and I am fine with a merge.

thanks.

NeoBlack commented 9 years ago

sorry, accidentally clicked the close button

DenBeke commented 9 years ago

Your solution is indeed correct as well, and stays closer to the original implementation.

DenBeke commented 9 years ago

(Oh, and sorry, I should have used [BUGFIX] instead of [FIX]. How can I change this?)