strlen($val) をどうするか

[seasoftjapan]

#996 で、strlen($val) は除外となったものの、気になる部分なので、備忘録を兼ねて。

strlen($val) は、挙動としては妥当なことが多いと思います。ただ、未定義な変数・要素の警告が、Warning に格上げされていることもあり、黙殺も難しくなっている認識です。

一方で、対象箇所も多いので、都度 isset($val) するなども冗長で避けたいです。

あと、PHP 7.4 では、配列などが入った場合の挙動も気になります。

思いつく折衷案として・・・

if (SC_Utils_Ex::strEmpty(@$var)) {
    // anything
}

class SC_Utils
{
    public static function strEmpty(?string $val)
    {
        return $val === '' || is_null($val);
    }
}

PHP 7.4 に関して目を瞑れば、既存の処理のまま strlen(@$val) で良い気もしてきました。

何れにしても、今どきのエラー制御演算子のコストは、ちょっと気になります。

[nanasess]

美しくないですが strlen($val ?? '') か strlen((string) $val) にしてしまうかですね。
全置換しても副作用無さそうですし

以下で Resolution になってます
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg

[seasoftjapan]

PHP の言語仕様上、$_REQUEST 等の要素に配列が入り得るので、そこの考慮は必要だと考えます。

EC-CUBE では strlen() への配列の流入を十分に考慮できていないと思うので、PHP 8 で言語仕様として strlen([]) が Fatal error となったのは歓迎しています。ただ、PHP 7.4 をサポートしている現状では、PHP バージョンによる挙動の違いを抱えた認識です。

$ php7.4 -r 'var_dump(strlen(["a"]));' 2>/dev/null
NULL

$ php8.3 -r 'var_dump(strlen(["a"]));'
PHP Fatal error:  Uncaught TypeError: strlen(): Argument #1 ($string) must be of type string, array given in Command line code:1
Stack trace:
#0 {main}
  thrown in Command line code on line 1

strlen($val ?? '') は、strlen(@$val) や strlen($val) と同じで、PHP バージョンによる挙動の違いを埋められないのが難でしょうか。

strlen((string) $val) は配列を受け取った際に、PHP 8 での恩恵も受けれないどころか、== 0 や >= 1 の判定も変わってしまうので無しだと思います。

$ php7.4 -r 'var_dump(strlen(["a"]) == 0);' 2>/dev/null
bool(true)

$ php7.4 -r 'var_dump(strlen((string)["a"]) == 0);' 2>/dev/null
bool(false)

[nanasess]

@seasoftjapan

strlen((string) $val) は配列を受け取った際に、PHP 8 での恩恵も受けれないどころか、== 0 や >= 1 の判定も変わってしまうので無しだと思います。

EC-CUBE本体側では、配列を受け取る可能性はほとんど排除されていると思っていましたが、よくよく考えると独自カスタマイズやプラグイン等を組み合わせた場合に排除しきれないですね...

おそらく、PHP9 で現在の Warning は Fatal に格上げされるので、strlen(@$val) の採用は難しいかなと。

[seasoftjapan]

あと、悪意を持って配列を流されたケースも気になります。後続の処理との不整合で最悪脆弱性となる懸念も無くはないかなと。

確かに PHP 9 を考慮すると、エラー制御演算子ではダメそうですね。そうなると消去法でベースは Null 合体演算子で、あとは PHP 7.4 での配列パターンをケアするか否かでしょうか。

ただ、PHP 9 を踏まえると、未定義変数に関しては Fatal の挙動が望ましい気がしてきました。
PHP 7-8 で同じ挙動をさせようとすると、冗長になりそうで悩ましいですが。
少なくとも現状で Warning が出るとしたら、個別に実装を見直すのが良い段階でしょうか。

配列の未定義要素は微妙ですね。ざっくり grep すると、EC-CUBE のリポジトリ内で strlen(), mb_strlen() を配列要素に使っているのは、81 箇所ありそうです。

配列要素の場合のみ Null 合体演算子を使うなども考えられそうです。
一応直近の実装では、配列要素に対する strlen() で、必要そうな箇所では Null 合体演算子を既に使っています。
https://github.com/EC-CUBE/ec-cube2/blob/da8ad32d1b765e8651928c7e001e2c7cc2b6bf87/tests/class/fixtures/server/sc_response_sendRedirect.php

ただ、画一的に使うのが良いのかは悩ましいですね。PHP 9 を踏まえると、strlen() 固有の問題では無くなり、strlen() 以外では null を想定している場合もありそうですし。