jskarvad wrote:

Proposed fix for mediawiki-1.16

The latest trunk has the code refactored into functions, but the principle should be the same.

Attached:

jskarvad wrote:

(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.

Attached:

jskarvad wrote:

(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';

Attached:

jskarvad wrote:

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?

jskarvad wrote:

(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).

jskarvad wrote:

(In reply to comment #6)

starting from line 1560,

Actually line 156.

jskarvad wrote:

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

Attached:

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 )

Attached:

jskarvad wrote:

(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.

Attached:

jskarvad wrote:

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

jskarvad wrote:

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

Change 116943 merged by jenkins-bot:
Fix HTTPS protocol detection

https://gerrit.wikimedia.org/r/116943