Bug #11386
closedtaint flag about rb_fstring()
Description
r51261以降、mswinのtest-allでfailureが出るようになった件を調査していて発見したのですが、
rb_fstring()にはtaintフラグを保存しないという問題があります。
原因は2つあって、
- sharedなStringオブジェクトを登録する際にtaintフラグをコピーしないバグがある
- 既に同じバイト列・エンコーディングで表現可能なStringオブジェクトが登録されている場合それを返すが、
引数のtaintフラグと返すオブジェクトのtaintフラグの違いを評価していない
というものです。
前者は以下のパッチで直るのでどうでもいいんですが、後者はrb_fstring()の仕様がどうなのか、
という問題であると思います。
r51261より前のように、SymbolやRubyレベルでは見えないStringオブジェクトのみを扱っているのならば特に問題ではないのでr51261のような使い方を禁止するべきなのか、
rb_fstring()を変更してtaintフラグを適切に扱うようにすべきなのか、どちらでしょうか?
Index: string.c
===================================================================
--- string.c (リビジョン 51334)
+++ string.c (作業コピー)
@@ -245,8 +245,10 @@ fstr_update_callback(st_data_t *key, st_data_t *va
}
else {
if (STR_SHARED_P(str)) { /* str should not be shared */
- str = rb_enc_str_new(RSTRING_PTR(str), RSTRING_LEN(str), STR_ENC_GET(str));
- OBJ_FREEZE(str);
+ VALUE newstr = rb_enc_str_new(RSTRING_PTR(str), RSTRING_LEN(str), STR_ENC_GET(str));
+ OBJ_INFECT(newstr, str);
+ OBJ_FREEZE(newstr);
+ str = newstr;
}
else {
str = rb_str_new_frozen(str);
Files
Updated by usa (Usaku NAKAMURA) over 9 years ago
- Backport changed from 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED to 2.0.0: DONTNEED, 2.1: REQUIRED, 2.2: REQUIRED
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
- File 0001-string.c-keep-taintedness.patch 0001-string.c-keep-taintedness.patch added
- File 0002-tests-for-fstring-taintedness.patch 0002-tests-for-fstring-taintedness.patch added
tainted/untaintedのfstringを分けるようにするパッチです。
Updated by usa (Usaku NAKAMURA) over 9 years ago
なるほど、とは思ったのですが、
「次にぼくは『特異メソッドを定義したStringオブジェクトをrb_fstring()に渡すと問題がある』と言うッ!」
という感じです。(そして次はインスタンス変数)
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
ほいさ
Updated by usa (Usaku NAKAMURA) over 9 years ago
むむ、早い。
で、このパッチだと、Stringのサブクラスのインスタンスだったり特異メソッドがあったりインスタンス変数を持ってたりするとfstringにならなくなってるわけですが、そもそもtaintフラグが立ってるものもfstringにならなくてもいいんじゃない? 的なことも思ったりします。
というわけで、やっぱり、「fstringって本質的にはなんなの?」という仕様の問題に立ち返る気がするのですが、これを説明できるのは誰でしょう?
最初は笹田さんかと思ってたんですが、実は元々これを持ってきたのはちゃりさむ?
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
昨日笹田さんとも話した結果、「内容だけをfstringに保存する」という方針がいいのではないか、ということになりました。
Updated by usa (Usaku NAKAMURA) over 9 years ago
やっぱそうなりますよねー。
ぼくもそれがいいと思います。
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
- Status changed from Open to Closed
Applied in changeset r51360.
string.c: pool only bare strings in fstring
- string.c (fstr_update_callback): pool bare strings only.
- string.c (rb_fstring): return the original string with sharing a
fstring if it has extra attributes, not the fstring itself.
[ruby-dev:49188] [Bug #11386]
Updated by naruse (Yui NARUSE) almost 8 years ago
- Related to Bug #12923: Accessing singleton_class of fstring cause assertion failure added