Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP8 対応漏れ?「受注管理>受注登録」画面でシステムエラー #829

Open
bbkids opened this issue Feb 4, 2024 · 11 comments
Assignees
Labels
Milestone

Comments

@bbkids
Copy link
Contributor

bbkids commented Feb 4, 2024

開発コミュニティ参照

error.log
[/manager/order/edit.php] Fatal error(E_ERROR): Uncaught TypeError: Unsupported operand types: float + string in data/class/pages/admin/order/LC_Page_Admin_Order_Edit.php:707

既存の受注情報を編集で、商品の単価を数字以外(例えば あ )を入力し、計算結果の確認ボタンを押すとシステムエラーで落ちます。(数量、税率、値引き、送料、手数料、使用ポイントも同様です。)
数字+文字ではWarninngこそ出ますが落ちません。また、PHP7.4では発生せず。

試しに、LC_Page_Admin_Order_Edit.php の lfCheckError() で

        $emsg = false;
        if ( isset($arrErrTemp['price']) || isset($arrErrTemp['quantity']) || isset($arrErrTemp['tax_rate']) || isset($arrErrTemp['discount']) || isset($arrErrTemp['deliv_fee']) || isset($arrErrTemp['charge']) || isset($arrErrTemp['use_point']) ) {
            $emsg = true;
        }

        for ($i = 0; $i < $max; $i++) {
                if($emsg){
                    $tax = 0;
                    $subtotal += 0;
                    $totaltax += 0;
                    $totalpoint += 0;
        	}
        	else{
                    // 小計の計算
                    $tax = SC_Helper_TaxRule_Ex::calcTax($arrValues['price'][$i], $arrValues['tax_rate'][$i], $arrValues['tax_rule'][$i]);
                    $subtotal += ($tax + $arrValues['price'][$i]) * $arrValues['quantity'][$i];
                    // 税額の計算
                    $totaltax += $tax * $arrValues['quantity'][$i];
                    // 加算ポイントの計算
                    $totalpoint += SC_Utils_Ex::sfPrePoint($arrValues['price'][$i], $arrValues['point_rate'][$i]) * $arrValues['quantity'][$i];
                }

入力値(単価、数量、税率、値引き、送料、手数料、使用ポイント)のチェックでエラーが無い場合のみ現行の処理、
エラーがある場合は、計算に必要な数字を0として処理するよう処理として試してみましたが、

テンプレートedit.tpl:432 側で呼び出される
<!--{$price|sfCalcIncTax:$tax_rate:$tax_rule|n2s}--> 円<br />
においても、Fatal error が発生します。

error.log
[/manager/order/edit.php] Fatal error(E_ERROR): Uncaught TypeError: Unsupported operand types: string * float in data/class/util\SC_Utils.php:711
となります。

@nobuhiko
Copy link
Contributor

nobuhiko commented Feb 4, 2024

例えばテンプレートの修正するのはどうでしょう
https://jp-seemore.com/web/2657/

@bbkids
Copy link
Contributor Author

bbkids commented Feb 4, 2024

あまり私自身どの様な仕様にすべきか言う事に知見がなくご意見するのは、恐縮なのですが、確かにテンプレート側で数字だけを受け付けるように修正するのが手っ取り早そうですね。
管理者側画面ですし、JavaScript回避して数字以外を入力する事もないでしょうから。

@bbkids
Copy link
Contributor Author

bbkids commented Feb 4, 2024

テンプレート側でINPUTタグのTYPE属 number が何よりも一番手っ取り早いですね。
ただこれだと空白が入力出来てしまうので、その場合はFatal errorで落ちますね。

テンプレートから sfCalcIncTax 呼び出すところで条件分岐し
<!--{if !($arrErr['price'][$product_index] || $arrErr['quantity'][$product_index] || $arrErr['tax_rate'][$product_index])}-->
何かしらのエラーがある場合は sfCalcIncTax を呼ばないようにするか

そもそも
JavaScriptで数字以外は入力させない&数字以外の文字を入力された場合にはそれを削除ですかね。

@seasoftjapan
Copy link
Contributor

従来と同じ動作をするように、キャストしてしまう方法もありますが、PHP の改善を無駄にしてしまうのも気が引けて悩ましいですね…

税込単価を PHP 側で計算して、テンプレート変数に渡す方向での回避を試みてます。

$objFormParam->addParam('税込単価', 'price_inctax', '', '', [], [], false);

@seasoftjapan
Copy link
Contributor

seasoftjapan commented Feb 4, 2024

お試しいただけますと幸いです。
master...seasoftjapan:eccube-2_13:seasoft-829

@seasoftjapan seasoftjapan added this to the 2.17.3 milestone Feb 4, 2024
@seasoftjapan seasoftjapan self-assigned this Feb 4, 2024
@bbkids
Copy link
Contributor Author

bbkids commented Feb 4, 2024

早速修正案ご作成有難う御座います。
master...seasoftjapan:eccube-2_13:seasoft-829
試しましたとろこ、良い感じで御座います。当方で確認されておりました不具合は解消致しました。

PHP 側で計算してテンプレート変数に渡す方法での修正は、やはり気持ちいいですね。

@bbkids
Copy link
Contributor Author

bbkids commented Feb 4, 2024

すみません、1点気になることが。
Warningなので気にしていてはキリが無いのですが、商品追加や変更の度に以下のWarningが出ます。
function calcPriceInctax(&$objFormParam)が使われる度に出るので出来れば解消したいのですが、どうでしょうか?

[/manager/order/edit.php] Warning(E_WARNING): Only the first byte will be assigned to the string offset on [data/class/pages/admin/order/LC_Page_Admin_Order_Edit.php(1332)] from ::1

@seasoftjapan
Copy link
Contributor

@bbkids 不具合報告ありがとうございます。

手元環境を別件使用中で検証できないのですが、キーが被ってしまったようです。

        $objFormParam->addParam('税額', 'tax', '', '', [], [], false);
        $objFormParam->addParam('消費税合計', 'tax');

追って修正を Push します。

@bbkids
Copy link
Contributor Author

bbkids commented Feb 4, 2024

早々にご確認有難う御座います。
キーを被らいよう試してみましたとろこ、問題のWarningは出なくなりました。
Pushされましたら、あらためて確認したいと思います。

seasoftjapan added a commit to seasoftjapan/eccube-2_13 that referenced this issue Feb 4, 2024
@bbkids
Copy link
Contributor Author

bbkids commented Feb 5, 2024

修正試して見ました。
当初のシステムエラー、及びWarninngの解消確認出来ました。

@seasoftjapan
Copy link
Contributor

seasoftjapan commented Feb 7, 2024

他に、SC_FormParam で適切な初期値を設定していなくてコケるパターンがあるようなので、修正予定です。

【追記】
push しました。
https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2

明細データは SC_FormParam のインスタンスを分けた方が良い気もしますが、とりあえず現状で目先のエラーが無くなれば、別 Issue かなと考えています。

seasoftjapan added a commit to seasoftjapan/eccube-2_13 that referenced this issue Feb 7, 2024
setProductsQuantity() 内のエラーを回避するため、lfCheckError() の事前呼び出しを徹底した。
seasoftjapan added a commit to seasoftjapan/eccube-2_13 that referenced this issue Feb 7, 2024
- 新規登録時に point (受注前ポイント) が空文字のため、生じるエラーを回避した。SC_FormParam を初期化。
- 上記の他にも SC_FormParam の初期化が不適当と思われる箇所を調整した。
seasoftjapan added a commit to seasoftjapan/eccube-2_13 that referenced this issue Feb 11, 2024
- 演算子の空文字の扱いが厄介なので null とする。
- SC_FormParam::getValue() が多次元配列の1次要素の一部を空文字で返してエラーとなるのを回避した。
- SC_FormParam::getValue() で定義されていないキーを指定した場合に例外を補足する。暗黙的に空文字を返して、後続の処理でエラーとなるのを防ぐ意図。
seasoftjapan added a commit to seasoftjapan/eccube-2_13 that referenced this issue Feb 11, 2024
- 演算子の空文字の扱いが厄介なので null とする。
- SC_FormParam の order_id が配列となり、Smarty h 修飾子がエラーとなるのを回避した。
- shipment_classcategory_name1, shipment_classcategory_name2 の処理が抜けていた箇所を補った。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants