stat2
/
delightly-v2fork
3
0
Fork 4
Code Issues 25 Pull Requests 1 Packages Projects Releases 1 Wiki Activity

postDataのエスケープ処理方法等の修正 #24

Merged
stat2 merged 10 commits from konkon-fox/delightly-v2fork:main into main 2023-10-12 14:09:47 +09:00
Conversation 3 Commits 10 Files Changed 3 +83 -77
konkon-fox commented 2023-10-10 19:10:11 +09:00
Contributor
Copy Link

大きな変更点は以下の通りです。
複数の変更点が混ざっていて申し訳ないですがご確認いただけると助かります。

  • postDataのエスケープ処理方法
  • スレ主判別キー($supervisorID)の処理方法
  • !addコマンドの修正
  • !noid及び!idchangeのスレ立て時での有効化

issueの方にも修正点の説明があります。
#23 (comment)
#1 (comment)

大きな変更点は以下の通りです。 複数の変更点が混ざっていて申し訳ないですがご確認いただけると助かります。 - postDataのエスケープ処理方法 - スレ主判別キー(\$supervisorID)の処理方法 - !addコマンドの修正 - !noid及び!idchangeのスレ立て時での有効化 issueの方にも修正点の説明があります。 https://git.3chan.cc/stat2/delightly-v2fork/issues/23#issuecomment-378 https://git.3chan.cc/stat2/delightly-v2fork/issues/1#issuecomment-365
konkon-fox added 6 commits 2023-10-10 19:10:12 +09:00
konkon-fox changed title from WIP: main to WIP: postDataのエスケープ処理方法等の修正 2023-10-10 19:11:08 +09:00
kenmo-melon approved these changes 2023-10-10 22:36:48 +09:00
kenmo-melon left a comment
First-time contributor
Copy Link

ありがとうございます！とりあえずこれでいいと思います。</b><b>は気になりますね？どういう意図だったんでしょう。

ありがとうございます！とりあえずこれでいいと思います。`</b><b>`は気になりますね？どういう意図だったんでしょう。
test/bbs-main.php Outdated
@ -856,7 +862,7 @@ fclose($fp);
}
// レス情報を本文末尾に追加
if ($M) $_POST['comment'] .= '<br><font color="gray"><small>['.$M.']</small></font>';
kenmo-melon commented 2023-10-10 21:19:42 +09:00
First-time contributor
Copy Link

これ本文末尾にワッチョイ追加してるってことですか？

これ本文末尾にワッチョイ追加してるってことですか？
konkon-fox commented 2023-10-11 00:03:33 +09:00
Author
Contributor
Copy Link

ﾜｯﾁｮｲや新規表示等の回線情報をまとめて本文末尾に追加する処理ですね。
存在理由が不明かつ不要だと判断したので削除しました。
削除の代わりに名前欄に追加するようにしてあります。それがﾜｯﾁｮｲ等の自然な位置だと思いますので。

ﾜｯﾁｮｲや新規表示等の回線情報をまとめて本文末尾に追加する処理ですね。 存在理由が不明かつ不要だと判断したので削除しました。 削除の代わりに名前欄に追加するようにしてあります。それがﾜｯﾁｮｲ等の自然な位置だと思いますので。
konkon-fox marked this conversation as resolved
test/bbs-main.php Outdated
@ -100,0 +80,4 @@
$postData = str_replace('"', '&quot;', $postData);
$postData = str_replace("'", '&apos;', $postData);
}else {
// 絵文字等禁止モードなのでhtmlspecialcharsでok
kenmo-melon commented 2023-10-10 21:03:57 +09:00
First-time contributor
Copy Link

5ちゃんねるのdatだと絵文字escapeされてません？専ブラによっては表示できないとかあるんですかね

5ちゃんねるのdatだと絵文字escapeされてません？専ブラによっては表示できないとかあるんですかね
konkon-fox commented 2023-10-11 00:24:08 +09:00
Author
Contributor
Copy Link

今ちらっと見た感じだと5chでは特に処理されていないように思います。 数値文字参照に変換されていますね。
ここの部分は厳密に言うと数値文字参照の文字列(&#1F97A;など)をキープするかエスケープ処理するかという分岐ですね。
再確認して気づきましたがこのままだと絵文字や特殊記号がエスケープ処理されないのでほぼ意味のない分岐になっています。
else節ではmb_encode_numericentity等で特殊文字を一旦数値文字参照に変換してからエスケープ処理するのが本来すべき処理でしょうか。
要修正項目ですが優先度は高くないかと思いますので今回は見送りという形でどうでしょうか。

※補足ですが現在の絵文字の取り扱いです。
ブラウザ用ファイル(index.json、subject.json、$THREADFILEのdat)は絵文字がそのまま格納されています。
専ブラ用ファイル(subject.txt、$DATFILEのdat)はShift_JISのため絵文字が数値文字参照に変換され格納されています。

今ちらっと見た感じだと5chでは~~特に処理されていないように思います。~~ 数値文字参照に変換されていますね。 ここの部分は厳密に言うと数値文字参照の文字列(\&#1F97A;など)をキープするかエスケープ処理するかという分岐ですね。 再確認して気づきましたがこのままだと絵文字や特殊記号がエスケープ処理されないのでほぼ意味のない分岐になっています。 else節ではmb_encode_numericentity等で特殊文字を一旦数値文字参照に変換してからエスケープ処理するのが本来すべき処理でしょうか。 要修正項目ですが優先度は高くないかと思いますので今回は見送りという形でどうでしょうか。 ※補足ですが現在の絵文字の取り扱いです。 ブラウザ用ファイル(index.json、subject.json、$THREADFILEのdat)は絵文字がそのまま格納されています。 専ブラ用ファイル(subject.txt、$DATFILEのdat)はShift_JISのため絵文字が数値文字参照に変換され格納されています。
konkon-fox marked this conversation as resolved
test/bbs-main.php Outdated
@ -174,3 +186,3 @@
// トリップを表示する場合
if ($trip) {
if ((strpos($_POST['name'], '!hide') === false && strpos($_POST['mail'], '!hide') === false && $SETTING['DISABLE_TRIP'] != "checked") || $SETTING['FORCE_DISP_TRIP'] == "checked") $_POST['name'] .= " </b>◆".$trip." <b>";
if ((strpos($_POST['name'], '!hide') === false && strpos($_POST['mail'], '!hide') === false && $SETTING['DISABLE_TRIP'] != "checked") || $SETTING['FORCE_DISP_TRIP'] == "checked") $_POST['name'] .= " <b>◆".$trip." </b>";
kenmo-melon commented 2023-10-10 21:08:47 +09:00
First-time contributor
Copy Link

これもともとの意図はなんだったんですかね
ただのタイポ？

これもともとの意図はなんだったんですかね ただのタイポ？
konkon-fox commented 2023-10-11 00:26:31 +09:00
Author
Contributor
Copy Link

単純なタイポだと思われます。

単純なタイポだと思われます。
konkon-fox commented 2023-10-11 18:58:12 +09:00
Author
Contributor
Copy Link

大変お恥ずかしい話ですがタイポではなく元のコードが正しいです。
2ch式掲示板の名前欄は太字がデフォルトでトリップ等システムから出力される文字列のみ細字となる仕様です。
ですのでシステムから出力される文字列を<b>の外に出すために元コードのような処理をしているようです。
これは速やかに修正します。

※追記　修正完了しました。

大変お恥ずかしい話ですがタイポではなく元のコードが正しいです。 2ch式掲示板の名前欄は太字がデフォルトでトリップ等システムから出力される文字列のみ細字となる仕様です。 ですのでシステムから出力される文字列を\<b\>の外に出すために元コードのような処理をしているようです。 これは速やかに修正します。 ※追記　修正完了しました。
konkon-fox marked this conversation as resolved
test/bbs-main.php Outdated
@ -290,3 +302,3 @@
$LOG = file($THREADFILE);
list($n,$m,$d,$message,$subject) = explode("<>", $LOG[0]);
if (strpos($m, substr(md5($_POST['thread'].$HAP['range'].$HAP['provider'].$HAP['CH_UA'].$HAP['ACCEPT']), 0, 5)) !== false) $supervisor = true;
if (strpos($m, substr(md5($_POST['thread'].$HAP['range'].$HAP['provider'].$HAP['CH_UA'].$HAP['ACCEPT']), 0, 8)) !== false) $supervisor = true;
kenmo-melon commented 2023-10-10 21:10:55 +09:00
First-time contributor
Copy Link

https://www.php.net/manual/ja/function.str-contains.php
この変更とは関係ありませんが、str_containsの方が良さそうですね

https://www.php.net/manual/ja/function.str-contains.php この変更とは関係ありませんが、`str_contains`の方が良さそうですね
konkon-fox commented 2023-10-11 00:38:00 +09:00
Author
Contributor
Copy Link

確かに用途としてはstr_containsの方が適していそうですね。
書き換えるにしてもstrposの使用箇所が非常に多いのでまた別の機会に一括で行うべきでしょうか。
あとstr_containsがPHP8以降でしか対応していないのが少し引っかかります。最近のサーバー事情に詳しくないので割合は不明ですがPHP7までしか対応していないサーバーではdelightly自体が使えなくなる、というのは避けたい気もします。

確かに用途としてはstr_containsの方が適していそうですね。 書き換えるにしてもstrposの使用箇所が非常に多いのでまた別の機会に一括で行うべきでしょうか。 あとstr_containsがPHP8以降でしか対応していないのが少し引っかかります。最近のサーバー事情に詳しくないので割合は不明ですがPHP7までしか対応していないサーバーではdelightly自体が使えなくなる、というのは避けたい気もします。
kenmo-melon commented 2023-10-11 11:06:45 +09:00
First-time contributor
Copy Link

なるほど、はるひさんや農園主さんに聞いてみて大丈夫そうならバージョン上げちゃってもいいと思いますが、とりあえず今回は見送ったほうが良さそうですね。

なるほど、はるひさんや農園主さんに聞いてみて大丈夫そうならバージョン上げちゃってもいいと思いますが、とりあえず今回は見送ったほうが良さそうですね。
konkon-fox marked this conversation as resolved
konkon-fox added 1 commit 2023-10-11 02:39:25 +09:00
konkon-fox added 1 commit 2023-10-11 02:47:21 +09:00
konkon-fox reviewed 2023-10-11 02:54:39 +09:00
test/bbs-main.php
@ -97,3 +73,1 @@
$_POST['title'] = preg_replace("/&#[xX]0*[aA]([^a-zA-Z0-9]|$)/", "<br>", $_POST['title']);
$_POST['name'] = preg_replace("/&#0*10([^0-9]|$)/", "", $_POST['name']);
$_POST['name'] = preg_replace("/&#[xX]0*[aA]([^a-zA-Z0-9]|$)/", "", $_POST['name']);
function escapePostData(&$postData, $keepNewLine){
konkon-fox commented 2023-10-11 02:54:39 +09:00
Author
Contributor
Copy Link

function escapePostData 内での無意味な分岐を削除しました。
これによりHTMLエスケープの処理方法がhtmlspecialcharsに一本化されました。
それに伴い不要なコードを削除しました。

function escapePostData 内での無意味な分岐を削除しました。 これによりHTMLエスケープの処理方法がhtmlspecialcharsに一本化されました。 それに伴い不要なコードを削除しました。
kenmo-melon commented 2023-10-11 11:09:23 +09:00
First-time contributor
Copy Link

詳細な確認ありがとうございます！絵文字禁止はそんなに優先度高くないと思うので、とりあえず今回はこれでいくのがベターかなと思います。

詳細な確認ありがとうございます！絵文字禁止はそんなに優先度高くないと思うので、とりあえず今回はこれでいくのがベターかなと思います。
konkon-fox marked this conversation as resolved
konkon-fox added 1 commit 2023-10-11 19:23:59 +09:00
konkon-fox changed title from WIP: postDataのエスケープ処理方法等の修正 to postDataのエスケープ処理方法等の修正 2023-10-11 23:15:11 +09:00
konkon-fox commented 2023-10-11 23:20:58 +09:00
Author
Contributor
Copy Link

kenmo-melonさんにもレビューいただき、大きな問題は無いようなのでWIP: を外しました。
とはいえまだレビューは受け付けますのでphpが読める方はぜひご協力よろしくお願いします。

kenmo-melonさんにもレビューいただき、大きな問題は無いようなのでWIP: を外しました。 とはいえまだレビューは受け付けますのでphpが読める方はぜひご協力よろしくお願いします。
konkon-fox added 1 commit 2023-10-12 14:07:09 +09:00
stat2 merged commit 08b6747264 into main 2023-10-12 14:09:47 +09:00
stat2 commented 2023-10-12 14:11:15 +09:00
Owner
Copy Link

ありがとうございました！
プルリクエストをマージしました。

ありがとうございました！ プルリクエストをマージしました。
konkon-fox commented 2023-10-12 16:00:13 +09:00
Author
Contributor
Copy Link

プルリクエストのマージ対応ありがとうございます。
今後もバグの修正や機能の追加などの機会があればよろしくお願いします。

プルリクエストのマージ対応ありがとうございます。 今後もバグの修正や機能の追加などの機会があればよろしくお願いします。
❤️ 2
Sign in to join this conversation.
Reviewers
No reviewers
kenmo-melon
Labels
Clear labels   
bug

Something is not working

  
discussion

   
documentation

Documentation changes

  
duplicate

This issue or pull request already exists

  
enhancement

Improve existing functionality

  
feature

New functionality

  
help wanted

Need some help

  
invalid

Something is wrong

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
discussion
documentation
duplicate
enhancement
feature
help wanted
invalid
Priority
High
Priority
Low
Priority
Medium
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: stat2/delightly-v2fork#24
No description provided.
Delete Branch "konkon-fox/delightly-v2fork:main"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?