Last modified: 2014-05-09 15:18:43 UTC
HTTPS detection is not reliable. It uses $_SERVER['HTTPS'] == 'on', but according to PHP docs [1]: > 'HTTPS' Set to a non-empty value if the script was queried through the HTTPS protocol. This maybe problem e.g. when mediawiki is running on Amazon cloud through HTTPS, their load balancer sets the HTTPS to '1'. Also the detection code suppose the HTTPS to be SSL on port 443, but IMHO it can be also TLS on port 80 and injecting the explicit port in this case also breaks things for me (Amazon/HTTPS/Firefox). Attached is the fix for mediawiki 1.16. It seems that the latest mediawiki 1.20.3 is also affected, but the code is slightly different there. This problem results in e.g. inability to save the user preferences. [1] http://php.net/manual/en/reserved.variables.server.php
Created attachment 11983 [details] Proposed fix for mediawiki-1.16 The latest trunk has the code refactored into functions, but the principle should be the same.
(In reply to comment #1) > Created attachment 11983 [details] > Proposed fix for mediawiki-1.16 > Maybe the check should be against '' and not 'off' to be according to the PHP docs.
(In reply to comment #2) > (In reply to comment #1) > > Created attachment 11983 [details] > > Proposed fix for mediawiki-1.16 > > > Maybe the check should be against '' and not 'off' to be according to the PHP > docs. ISAPI with IIS uses 'off', so probably the following could be OK: $wgProto = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != '' && $_SERVER['HTTPS'] != 'off') ? 'https' : 'http';
Maybe the relative URLs could be used.
Hi Jaroslav! Thanks for your patch, but unfortunately 1.16 is not maintained anymore. Could you provide a hint (line number) where to see the same problem in 1.20.3, if possible?
(In reply to comment #5) > Hi Jaroslav! Thanks for your patch, but unfortunately 1.16 is not maintained > anymore. Could you provide a hint (line number) where to see the same problem > in 1.20.3, if possible? It seems to be in the includes/WebRequest.php (starting from line 1560, functions detectServer() and detectProtocolAndStdPort(). I will try to provide the patch (I need to get it into cloud first to check whether the fix really works there).
(In reply to comment #6) > starting from line 1560, Actually line 156.
Created attachment 11984 [details] Proposed fix for mediawiki-1.20.3 This resolves the problem for me. - Fixed HTTPS check to be according to PHP docs. - Added port 80 as another default port for HTTPS (TLS) Reproducer: - hosted mediawiki-1.20.3 on Openshift - accessed the user preferences through TLS, changed anything and clicked save Result without patch: Warning, that the data will be sent unencrypted, if HTTPS is enforced through the Apache rewrite rule, nothing get saved Result with patch: No warning, data get saved
Platonides: Wondering if you'd be interested in this / could comment, as per bug 28798 comment 10 but maybe I'm wrong. Anybody could put the patch into Gerrit so we can get it in. See https://www.mediawiki.org/wiki/Developer_access and https://www.mediawiki.org/wiki/Git/Tutorial
(In reply to Jaroslav Škarvada from comment #8) > Created attachment 11984 [details] > Proposed fix for mediawiki-1.20.3 Hmm, https://git.wikimedia.org/tree/mediawiki%2Fcore.git does not list any "php" subfolder in the top level, neither does the 1.19.12 tarball, so it cannot be applied and I am not sure which base this patch is against. Can you clarify? (Also, this needs to go into Gerrit nowadays: See https://www.mediawiki.org/wiki/Developer_access and https://www.mediawiki.org/wiki/Git/Tutorial )
(In reply to Andre Klapper from comment #10) > (In reply to Jaroslav Škarvada from comment #8) > > Created attachment 11984 [details] > > Proposed fix for mediawiki-1.20.3 > > Hmm, https://git.wikimedia.org/tree/mediawiki%2Fcore.git does not list any > "php" subfolder in the top level, neither does the 1.19.12 tarball, so it > cannot be applied and I am not sure which base this patch is against. Can > you clarify? > I will check the current status and in case the problem is still there I will refresh the patch. > (Also, this needs to go into Gerrit nowadays: See > https://www.mediawiki.org/wiki/Developer_access and > https://www.mediawiki.org/wiki/Git/Tutorial ) > OK, thanks for info, I will sign-up for the Gerrit account.
It seems the problem with the Amazon WS load balancer was already resolved in the GIT, but the issue with the $_SERVER['HTTPS'] seems still to be unresolved. The problem is that $_SERVER['HTTPS'] == 'on' is used in the MediaWiki for detections of the HTTPS protocol which is not correct according to PHP documentation [1], citing: --- '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. --- I also saw implementation where it was set to '1' instead of 'on'. So to be in sync with the PHP documentation I think it should use: $_SERVER['HTTPS'] != '' && $_SERVER['HTTPS'] != 'off' instead of: $_SERVER['HTTPS'] == 'on' I will post the patch through Gerrit. [1] http://www.php.net/manual/en/reserved.variables.server.php
Change 116943 had a related patch set uploaded by Yarda: Fix HTTPS protocol detection https://gerrit.wikimedia.org/r/116943
I have no idea who to assign the review, the list http://www.mediawiki.org/wiki/Developers/Maintainers wasn't helpful, so keeping it as unassigned.
(In reply to Gerrit Notification Bot from comment #13) > Change 116943 had a related patch set uploaded by Yarda: > Fix HTTPS protocol detection > > https://gerrit.wikimedia.org/r/116943 Awesome, welcome in gerrit and thanks for your first patch; hopefully the first of a long series. (In reply to Jaroslav Škarvada from comment #14) > I have no idea who to assign the review, the list > http://www.mediawiki.org/wiki/Developers/Maintainers wasn't helpful, so > keeping it as unassigned. Usually we just assign to whoever submits the patch, but then it's a collaborative effort. I see you already received a review, if after several days everything is still silent please add more reviewers active on those files and related and/or poke someone on IRC.
Change 132435 had a related patch set uploaded by Reedy: Fix HTTPS protocol detection https://gerrit.wikimedia.org/r/132435
Change 132435 abandoned by Reedy: Fix HTTPS protocol detection https://gerrit.wikimedia.org/r/132435
