Skip to content

Invert if statement for https check, better compatibility#140

Merged
mesa57 merged 1 commit intospotweb:masterfrom
guyspr:master
Apr 7, 2016
Merged

Invert if statement for https check, better compatibility#140
mesa57 merged 1 commit intospotweb:masterfrom
guyspr:master

Conversation

@guyspr
Copy link
Copy Markdown

@guyspr guyspr commented Apr 5, 2016

I inverted the https check for compatibility. On some setups it happens that $_SERVER['HTTPS'] is undefined or nil if it is turned on, but it is set to 'off' when using http. (this was the case for me)

This needs testing on some different machines (which I don't have), to validate if this works on all systems.

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

May I ask what webserver you use and if you use https ?
I do not think we're able to test this on several machines, as the team is very small.
And most of us do not use https.

@guyspr
Copy link
Copy Markdown
Author

guyspr commented Apr 5, 2016

The server which it is running on is using Apache and I am running on https. My https connection actually runs trough a proxy, which makes https to undefined instead of on.
Might be a very specific case though.

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

Ok. could you look at this issue : https://bugs.php.net/bug.php?id=66398
and tell me what in you're case the $SERVER['X-FORWARDED-PROTO'] contains ?

@guyspr
Copy link
Copy Markdown
Author

guyspr commented Apr 5, 2016

That issue basically describes my situation. Directly taken from phpinfo:

X-Forwarded-HTTPS https
X-Forwarded-Port 443
X-Forwarded-Proto https

So maybe adding a check for X-Forwarded-Proto == https is a better way around this issue.

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

Thats what the topic suggests. But be aware that $_SERVER['X-FORWARDED-PROTO'] can be empty (as in my case without any proxy).

@WzL
Copy link
Copy Markdown

WzL commented Apr 5, 2016

Maybe HTTP_X_FORWARDED_SSL can be set? Function getRequestProtocol() 33920a2 has a check for this. Please correct me when I am wrong.

2016-04-05 19:38 GMT+02:00 mesa57 notifications@github.com:

Thats what the topic suggests. But be aware that
$_SERVER['X-FORWARDED-PROTO'] can be empty (as in my case without any
proxy).


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#140 (comment)

Met vriendelijke groet,

With kind regards,

Mit freundlichen Grüßen,

Distinti saluti,

Atentos saludos,

Jacco Onderdijk

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

That function is not used anymore (?). Anyway, the proxy server is responsible for filling those variables.

@guyspr
Copy link
Copy Markdown
Author

guyspr commented Apr 5, 2016

$settings['spotweburl'] = (@$_SERVER['HTTPS'] == 'on' || (@$_SERVER['HTTPS']== 'https')? 'https' : 'http') . '://' . @$_SERVER['HTTP_HOST'] . (dirname($_SERVER['PHP_SELF']) != '/' && dirname($_SERVER['PHP_SELF']) != '\\' ? dirname($_SERVER['PHP_SELF']). '/' : '/');   

Just checked and my server actually sets _SERVER["HTTPS"] to 'https' instead of on. So the above check would also solve the problem.

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

As the documentation enhancement in the issue suggest the value $_SERVER['HTTPS'] will be non empty if called thu an https proxy. The value is not of importance.
So the test could better be : !empty($_SERVER['HTTPS'] ? 'https' : 'http'

@guyspr
Copy link
Copy Markdown
Author

guyspr commented Apr 5, 2016

The documentation suggests it is either empty or off. (See the note)
http://www.php.net/manual/en/reserved.variables.server.php

'HTTPS'
Set to a non-empty value if the script was queried through the HTTPS protocol.
Note: Note that when using ISAPI with IIS, the value will be off if the request was not made through the HTTPS protocol.

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 5, 2016

ISAPI filters are not often used. I am running an iis server without https and it is also returning 'off'.
So after all discussions, it looks like you're solution is the best one.
However, I would like @Sweepr to agree, and if possible to execute a test with a cloud proxy (or is that you're situation ?).

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 7, 2016

No reaction, going to merge.

@mesa57 mesa57 merged commit f07ff36 into spotweb:master Apr 7, 2016
@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 9, 2016

Reverting this due to some reported problems.
http://gathering.tweakers.net/forum/view_message/46455033

@mesa57
Copy link
Copy Markdown
Collaborator

mesa57 commented Apr 9, 2016

@guyspr
Copy link
Copy Markdown
Author

guyspr commented Apr 11, 2016

Allright, I will have a look into a better way of handling this. Maybe as said before, using this statement should work pretty good:

$settings['spotweburl'] = (@$_SERVER['HTTPS'] == 'on' || (@$_SERVER['HTTPS']== 'https')? 'https' : 'http') . '://' . @$_SERVER['HTTP_HOST'] . (dirname($_SERVER['PHP_SELF']) != '/' && dirname($_SERVER['PHP_SELF']) != '\\' ? dirname($_SERVER['PHP_SELF']). '/' : '/');   

Here you are absolutely sure it is set to https, otherwise use http. Which would be a good flow of thoughts (in my opinion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants