search

inc2734 / mw-wp-form

69 stars 46 forks source link

4.4.1 以降、「URL引数を有効にする」のときに URL パラメータに全角文字が含まれている場合、フォームの入力画面で無限ループが発生する #133

Closed inc2734 closed 2 years ago

inc2734 commented 2 years ago

メールで報告をもらいました。

例） ・変数の内容が半角（test）の例 　/sample/?type=test 　→OK

・変数の内容が全角（あ）の例 　/sample/?type=%E3%81%82 　→無限ループ発生

（「入力画面URL」の項目は設定済み）

半角文字や空文字、「URL引数を有効にする」の機能自体を使用しない場合は問題無くアクセスできております。 4.4.0と4.4.1のソースを見比べてみたのですが、 classes/services/class.redirected.phpの174行目付近で、以下のような違いがございました。

4.4.1 return $url . '?' . build_query( $query_string );

4.4.0 return $url . '?' . http_build_query( $query_string, null, '&' );

4.4.1からbuild_queryになったようですが、これはURLエンコードが行われない物だったと思います。 この辺りが影響しているのでしょうか。 （実際に4.4.1をhttp_build_queryに変更してみると正しく入力画面が表示されました）

tomothumb commented 2 years ago

同様の症状でした。先ほどプルリク送りました。#135

URLのqueryで何らかの不具合が出ているものと思いますが、気づいたことが2点。

・本Issueでも指摘のある、build_query()は、 https://developer.wordpress.org/reference/functions/build_query/ _http_build_query()のラッパですが、 https://developer.wordpress.org/reference/functions/_http_build_query/ build_query()を使うと、_http_build_query()の最後の引数のurlencodeにfalseが渡されurlencodeされないため、 #124 のエラーが引き起こされてしまうものと思いますので、個人的にあまり推奨できないように思います。

・もう一点、 同様の箇所のテストコードに、with_asterisk=foo*barになるようassertかけられておりますが、 アスタリスクは、予約語であるため、URLに使用しないのがw3cのドキュメントで推奨されています。 https://www.w3.org/Addressing/URL/4_URI_Recommentations.html https://webmasters.stackexchange.com/questions/15884/can-i-use-asterisks-in-urls

Other reserved characters
The asterisk ("*", ASCII 2A hex) and exclamation mark ("!" , ASCII 21 hex) are reserved for use as having special signifiance within specific schemes.
Uns
Example 3
The URIs
fxqn:/us/va/reston/cnri/ietf/24/asdf%*.fred

and
news:12345667123%asdghfh@info.cern.ch

are illegal, as all % characters imply encodings, and there is no decoding defined for "%*" or "%as" in this recommendation.

Qiitaの記事では、冒頭部分に、はURLに使って良いような記述がありますが、 同記事の文中、RFC 3986の箇所の説明で、はreservedに含まれるので、やはりエンコードが推奨となっています。 https://qiita.com/aosho235/items/0581fc82f8ce2c5ac055

RFC 2396にはあったreservedという文言がなくなっているが、sub-delims、":"、"@"がreservedに相当する。よって!'()*の五文字が、以前はunreservedであったが現在はreservedになった、ということ。

どのRFCに従うかという話ではありますが、他のフレームワークになりますが、Laravelを参照するところ、RFC 3986準拠でURLエンコードがデフォルトのようです。 Wordpressの、build_query()および、_http_build_query()は、WordPress独自のパーサのようなので、どのような経緯で、独自アルゴリズムにしているのかはわかりませんが、 ひとまずPHPのhttp_build_query()にて、RFC 3986でエンコードした想定で、プルリクエストを送りました。

補足としまして、半角スペースを+と変換するか、%20と変換するか別の問題がありますが、%20としてます。 ご検討ください。

inc2734 commented 2 years ago

マージしました！ありがとうございます！