$_SERVER['HTTP_X_FORWARDED_PROTO']) が設定されている環境で、 不正な遷移になっていたのを修正

[nanasess]

• $_SERVER['HTTP_X_FORWARDED_PROTO']) が設定されている環境で、 不正な遷移になっていたのを修正
  • Net_URL で生成した URL が http になってしまうため
• app_initial.php で $_SERVER['HTTPS'] = 1 に設定されているため、これに合わせる
  https://github.com/EC-CUBE/eccube-2_13/blob/3ac54e063c27fd09d99b0e0fe20fde2f57832c4a/data/app_initial.php#L25-L28

[coveralls]

Coverage remained the same at 42.986% when pulling 6a913d4 on nanasess:fix-https into 7f77a44 on EC-CUBE:improve/php7.

[3tiles]

横からすみません。
#88 でのapp_initial.phpの修正内容の方を直した方が良いような気がするんですけど、どうでしょうか？
通常、$_SERVER['HTTPS']は、1ではなくonかと思うので、そこだけ直せばPEARライブラリ側を弄らなくても良さそうに思えますがどうでしょう？

[nanasess]

@3tiles
$_SERVER のドキュメントを見ると、 空ではない値 が正しい仕様で、例外的に IIS は off を入れてきます。

スクリプトが HTTPS プロトコルを通じて実行されている場合に 空でない値が設定されます。
注意: ISAPI を IIS で使用している場合は、HTTPS プロトコルを通さないでリクエストが行われたときの値は off となることに注意しましょう。

条件としては、 空ではなく、かつ off でもない のがベストなので、 Net_URL 側を修正してます

[3tiles]

Net_URL側の条件判定式も修正の余地があることは理解致しました。

ただ、
data/module/SOAP/Disco.php
data/module/SOAP/WSDL.php
でも同様にonかどうかで判別している部分があったりしますし(こちらに関しては実害が無いのかもしれませんが)、PEARライブラリ自体を修正してしまうと、ライブラリをバージョンアップとかする際に独自に修正した部分の再取り込み等が必要になり保守性が下がるかなと思いまして。
(Net_URLに関しては、もうメンテされていないようなのでその危惧は皆無かもしれませんが、もしNet_URL2とかへ移行することがあった場合とかは影響があるかなと思いまして。)

そういったところで、起因となっているapp_initial.php側"も"、この際修正した方が良いのではと思うのですが如何でしょうか？
(通常はonだと思っているのですが、もしかして1がセットされるな環境も普通にあったりするものなのでしょうか？であれば勉強不足ですみません。)

[nobuhiko]

data/module 以下にあるPEARライブラリは、保守を自力でやる部分になるので修正するのは構わないと思います。（Net_URL2への移行は様々な事情により出来ません）

Net_URL2では判定分はこれですかね・・
$url->_scheme = isset($_SERVER['HTTPS']) ? 'https' : 'http';

[nanasess]

ただ、
data/module/SOAP/Disco.php
data/module/SOAP/WSDL.php
でも同様にonかどうかで判別している部分があったりしますし

PEAR_SOAP に関しては、本体で使われてないですし、通常は PHP の soap extension を使用すると思いますので無視しても良いレベルかなと。一部の決済で依存しているので残しているだけです。

通常はonだと思っているのですが、もしかして1がセットされるな環境も普通にあったりするものなのでしょうか？
WordPress は 1 もチェックしているので、おそらく何かしらあるんだろうと推測しています
https://core.trac.wordpress.org/browser/tags/5.2.2/src/wp-includes/load.php#L1256

公式にあるサンプルコードは off 以外になってますね

if (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off') {
      // You are sure it's really HTTPS
  }

https://www.php.net/manual/ja/reserved.variables.server.php#122859

[3tiles]

"1"のケースに関しては、
https://core.trac.wordpress.org/ticket/8641
(このチケットにて1かどうかもチェックされるようになったようです。)
によれば、Apache 2.0未満でsuEXEC設定している環境とかでありえるみたいですね。

"off"のケースに関しては、
IISでISAPIモジュールで動かしている場合(when using ISAPI with IIS)とのことですが、
https://www.php.net/manual/en/migration53.windows.php
によれば、

Warning Support for the ISAPI module has been dropped. Use the improved FastCGI SAPI module instead.

とのことで、PHP5.3以降はISAPIモジュールは提供されなくなっているようです。

どちらのケースも現在だと使われない環境下のものになるかと思いますので、どこまで考慮すべきかは再考の余地があるかと。

各種プロダクトで歴史的経緯等から判別ルーチンは異なるようですが、現在だとNet_URL2みたいに、

isset($_SERVER['HTTPS']) ?

のようにシンプルなものでも事足りるのかもしれません。
(Net_URL2 1.0.0 リリース時[2011-10-20]には、Apache HTTP Server 1.3 系もPHP 5.2系もサポート期限が終了していたため？)

システム要件的には、
https://www.ec-cube.net/product/system_213.php
では、PHP5.2以降となっているので、
"off"のケースは考慮が必要だったかもしれませんが、
https://www.ec-cube.net/news/detail.php?news_id=290
によれば、

▼システム要件の変更
EC-CUBE 2.13系のシステム要件からPHPバージョンが変更になります。
EC-CUBE 2.17.0αは、PHP5.4以降が必要となります。

とのことなので、もはや考慮不要ではないでしょうか？

ただ、このあたりの判別方法として、どういった方針で何を採用するかは、私なんかが決める立場ではないのでお任せしますが、app_initial.phpに関してだけは、最も無難な"on"をセットするように修正した方が良いんじゃないかと強く思います。

[nanasess]

@3tiles @nobuhiko 特に深いこだわりは無いので app_initial.php の方を $_SERVER['HTTPS'] = 'on' にしました

[3tiles]

ありがとうございます！

[nobuhiko]

@3tiles さん、 Approve しちゃってください

[nobuhiko]

@nanasess テスト落ちてます😳

[nanasess]

@nobuhiko #283 のテストをマージしたのが原因で落ちてるっぽいので、 #285 に修正含めておきます

[chihiro-adachi]

@nanasess
ありがとうございます。マージしました。