Bug #650

Marshal.load raises RegexpError

Added by shyouhei (Shyouhei Urabe) over 3 years ago. Updated about 1 year ago.

[ruby-dev:36750]
Status:Closed Start date:10/15/2008
Priority:Normal Due date:
Assignee:matz (Yukihiro Matsumoto) % Done:

100%

Category:M17N
Target version:2.0.0
ruby -v:ruby 1.9.2dev

Description

以下のように、以前のバージョンのRubyで正しくdumpしたはずの文字列をtrunkでloadできません。

 % ruby1.8 -e 'Marshal.dump(/C:\Documents and Settings\urabe/, STDOUT)' | ruby1.8 -ve 'p Marshal.load(STDIN)'
 ruby 1.8.7 (2008-10-11 revision 17572) [x86_64-linux]
 /C:\Documents and Settings\urabe/

 % ruby1.8 -e 'Marshal.dump(/C:\Documents and Settings\urabe/, STDOUT)' | ruby1.9 -ve 'p Marshal.load(STDIN)'
 ruby 1.9.0 (2008-10-13  revision 17576) [x86_64-linux]
 -e:1:in `load': invalid Unicode escape: /C:\Documents and Settings\urabe/ (RegexpError)
         from -e:1:in `<main>'

Associated revisions

Revision 23999
Added by naruse (Yui NARUSE) almost 3 years ago

* marshal.c (r_object0): replace \u by u when the regexp is made by Ruby 1.8. [ruby-dev:36750]

History

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36750] [Bug #650] Marshal.load raises RegexpError"
    on Wed, 15 Oct 2008 22:59:59 +0900, Shyouhei Urabe <redmine@ruby-lang.org> writes:

|以下のように、以前のバージョンのRubyで正しくdumpしたはずの文字列をtrunkでloadできません。
|
| % ruby1.8 -e 'Marshal.dump(/C:\Documents and Settings\urabe/, STDOUT)' | ruby1.8 -ve 'p Marshal.load(STDIN)'
| ruby 1.8.7 (2008-10-11 revision 17572) [x86_64-linux]
| /C:\Documents and Settings\urabe/

1.9では正規表現に\uが増えたからですね。

厳密に言うとmarshalに上位互換性がないのでmarshalのメジャーバー
ジョンを変化させるべきなのかもしれませんが、メリットよりもデ
メリットの方が大きいのでそれはしない方向を考えてます。

で、1.8の方のregexpに手を入れて「\u」は「u」に正規化しようと
思います。そうすると、上のだと

  C:\Documents and Settingsurabe

になってしまうわけですが、もともと\Dは[0-9]という意味で、上の
正規表現はもともとパスにはマッチしないんで、これはこういうも
のだと思ってください。バックスラッシュを含むパターンはちゃん
と\\に変換してくださいということで。

                                まつもと ゆきひろ /:|)

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

Yukihiro Matsumoto さんは書きました:
> 1.9では正規表現に\uが増えたからですね。
>
> 厳密に言うとmarshalに上位互換性がないのでmarshalのメジャーバー
> ジョンを変化させるべきなのかもしれませんが、メリットよりもデ
> メリットの方が大きいのでそれはしない方向を考えてます。
>   

ちなみにどういうデメリットですか?

> で、1.8の方のregexpに手を入れて「\u」は「u」に正規化しようと
> 思います。そうすると、上のだと
>
>   C:\Documents and Settingsurabe
>
> になってしまうわけですが、もともと\Dは[0-9]という意味で、上の
> 正規表現はもともとパスにはマッチしないんで、これはこういうも
> のだと思ってください。バックスラッシュを含むパターンはちゃん
> と\\に変換してくださいということで。

今後Marshal.loadされる場合に関しては特に問題が思い浮かばないのでいいん
じゃないかと思いますが、すでに作ってしまったPStoreデータベースが困りま
す、というか、手元で読めなくて若干困っているわけです。なにか(スクリプト
側ででもかまわないので)既存のMarshal済みデータに対するworkaroundみたいな
ものはないでしょうか。

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36771] Re: [Bug #650] Marshal.load raises RegexpError"
    on Sat, 18 Oct 2008 01:05:25 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|> 厳密に言うとmarshalに上位互換性がないのでmarshalのメジャーバー
|> ジョンを変化させるべきなのかもしれませんが、メリットよりもデ
|> メリットの方が大きいのでそれはしない方向を考えてます。

|ちなみにどういうデメリットですか?

1.8と1.9でmarshalデータが全くやりとりできないというデメリッ
トです。

|今後Marshal.loadされる場合に関しては特に問題が思い浮かばないのでいいん
|じゃないかと思いますが、

実はMarshal.dumpしたものをloadしても等しくないという問題がな
いわけではないんですが。

|すでに作ってしまったPStoreデータベースが困りま
|す、というか、手元で読めなくて若干困っているわけです。なにか(スクリプト
|側ででもかまわないので)既存のMarshal済みデータに対するworkaroundみたいな
|ものはないでしょうか。

これからコミットする新しい1.8で古いmarshalデータを一度読み込
んで、ふたたび書き込むことで1.9でも読めるようにできます。って、
そういうことじゃないのかな。

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

Yukihiro Matsumoto さんは書きました:
> まつもと ゆきひろです
>
> In message "Re: [ruby-dev:36771] Re: [Bug #650] Marshal.load raises RegexpError"
>     on Sat, 18 Oct 2008 01:05:25 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:
>
> |> 厳密に言うとmarshalに上位互換性がないのでmarshalのメジャーバー
> |> ジョンを変化させるべきなのかもしれませんが、メリットよりもデ
> |> メリットの方が大きいのでそれはしない方向を考えてます。
>
> |ちなみにどういうデメリットですか?
>
> 1.8と1.9でmarshalデータが全くやりとりできないというデメリッ
> トです。
>   

まったくということはないでしょう。
1.8で作ったデータを1.9が読めように1.9を作るのは可能なはずです。
# 今でも、古いmarshal formatのデータを読もうとすると、警告が出るが処理自
体はできるはず。

> |すでに作ってしまったPStoreデータベースが困りま
> |す、というか、手元で読めなくて若干困っているわけです。なにか(スクリプト
> |側ででもかまわないので)既存のMarshal済みデータに対するworkaroundみたいな
> |ものはないでしょうか。
>
> これからコミットする新しい1.8で古いmarshalデータを一度読み込
> んで、ふたたび書き込むことで1.9でも読めるようにできます。って、
> そういうことじゃないのかな。
>   

気づいたんですけど、それだと

% ruby1.9 -e 'Marshal.dump(/\u0001/, STDOUT)' | \
  ruby1.8 -e 'Marshal.dump(Marshal.load(STDIN), STDOUT)' | \
  ruby1.9 -e 'p Marshal.load(STDIN)'

とかがroud-tripしなくなると思う(いまはする)んですけど、平気なんですかね。

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

Tanaka Akira さんは書きました:
> syntax と semantics のどちらを保存するかというと、semantics
> のほうがいいんじゃないですかね。
>
> 今は以下のように、マッチするものが変化するわけで、それが変化
> しないようになるほうがまだマシなんじゃないでしょうか。
>   

semanticsのほうが保存されてたほうがいいのには納得しましたが、寝不足の頭
で考えたところによると、それって無理じゃない?

1.8と1.9は正規表現エンジンが違うわけで、たとえば /(?<foo>bar)/ とかは1.8
に持っていくことはできませんよね。

まあつまり、結局のところMarshalデータを1.8で作って1.9で読むのはひょっと
して頑張れば可能かもしれないけど、1.9で作って1.8で読むのはあからさまに不
可能なので、頑張れるにしても上位互換くらいが関の山で、ってことはMarshal
のバージョンが同じであることに固執しても意味ないんじゃないかという気がし
てきたんですが、どうでしょう。

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36773] Re: [Bug #650] Marshal.load raises RegexpError"
    on Sat, 18 Oct 2008 07:05:07 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|> |> 厳密に言うとmarshalに上位互換性がないのでmarshalのメジャーバー
|> |> ジョンを変化させるべきなのかもしれませんが、メリットよりもデ
|> |> メリットの方が大きいのでそれはしない方向を考えてます。
|>
|> |ちなみにどういうデメリットですか?
|>
|> 1.8と1.9でmarshalデータが全くやりとりできないというデメリッ
|> トです。
|
|まったくということはないでしょう。
|1.8で作ったデータを1.9が読めように1.9を作るのは可能なはずです。
|# 今でも、古いmarshal formatのデータを読もうとすると、警告が出るが処理自
|体はできるはず。

Marshalファイルフォーマットには、メジャーバージョンとマイナー
バージョンの情報が付加されており、マイナーバージョンの違いは
「上位互換性がある」ので「古いmarshal formatのデータを読もう
とすると、警告が出るが処理自体はできる」、メジャーバージョン
の違いは「互換性はないので古いデータは読めない」とする仕組み
があります。

今回の場合は、古いデータが新しいMarshalで読めない(Marshal以
外の部分の変更に影響を受けた)というものなので、Marshal側でで
きる対応は

 (1) 杓子定規に考えて、メジャーバージョンをあげる(1.8と1.9が
     通信できなくなる、うれしくない)

 (2) ささいな違いなので気にしない

 (3) 1.8で正規化する(すでに書き込んだデータは救済できないし、
     副作用もある)

 (4) 1.9側に1.8正規表現かどうか判別して1.9正規表現に変換する
     (おそらくは巨大な)ルーチンを追加する(苦労の割に得るもの
     が少ない、うれしくない)

くらいではないかと思います。現状のリソースから考えると、2か3
くらいがせいぜいではないかと思います。

                                まつもと ゆきひろ /:|)

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

Yukihiro Matsumoto さんは書きました:
>  (1) 杓子定規に考えて、メジャーバージョンをあげる(1.8と1.9が
>      通信できなくなる、うれしくない)
>
>  (2) ささいな違いなので気にしない
>
>  (3) 1.8で正規化する(すでに書き込んだデータは救済できないし、
>      副作用もある)
>
>  (4) 1.9側に1.8正規表現かどうか判別して1.9正規表現に変換する
>      (おそらくは巨大な)ルーチンを追加する(苦労の割に得るもの
>      が少ない、うれしくない)
>   

(5) 1.9でRegexp#_dumpとRegexp#_loadを定義する
ってのはどうでしょうね。すると
* TYPE_REGEXPなデータは1.8のRegexp
* TYPE_USERDEFなデータ(でklass==rb_cRegexp)は1.9のRegexp
という割と簡単な判定でいけそうな気がするんですが。


# おもいつきレベルだけど。

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36785] Re: [Bug #650] Marshal.load raises RegexpError"
    on Sun, 19 Oct 2008 02:13:16 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|Yukihiro Matsumoto さんは書きました:
|>  (1) 杓子定規に考えて、メジャーバージョンをあげる(1.8と1.9が
|>      通信できなくなる、うれしくない)
|>
|>  (2) ささいな違いなので気にしない
|>
|>  (3) 1.8で正規化する(すでに書き込んだデータは救済できないし、
|>      副作用もある)
|>
|>  (4) 1.9側に1.8正規表現かどうか判別して1.9正規表現に変換する
|>      (おそらくは巨大な)ルーチンを追加する(苦労の割に得るもの
|>      が少ない、うれしくない)
|
|(5) 1.9でRegexp#_dumpとRegexp#_loadを定義する
|ってのはどうでしょうね。すると
|* TYPE_REGEXPなデータは1.8のRegexp
|* TYPE_USERDEFなデータ(でklass==rb_cRegexp)は1.9のRegexp
|という割と簡単な判定でいけそうな気がするんですが。

判別はできますが、その後の対応はどうするんでしょうね。
1.9に1.8のregex.cを導入するのも、1.8に鬼車を導入するのも現実
的ではないと思いますから、判別できても完全な互換性を維持する
のは困難ではないでしょうか。

さらに言うと、この件についてなんらかの対応したとしても、救済
できるのは、ドキュメントに掲載されていないし、サポートされて
いると明言されたこともない\uのようなメタキャラクタでないアル
ファベットの前にバックスラッシュがついた正規表現だけなので、
コストの割に得るものが少ない気がします。

考えるほど、ここは(2)かなあ、という気がしてきました。という
か、むしろundocumentedなので対応すべきでないような。

                                まつもと ゆきひろ /:|)

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

Yukihiro Matsumoto さんは書きました:
> まつもと ゆきひろです
>
> In message "Re: [ruby-dev:36785] Re: [Bug #650] Marshal.load raises RegexpError"
>     on Sun, 19 Oct 2008 02:13:16 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:
>
> |Yukihiro Matsumoto さんは書きました:
> |>  (1) 杓子定規に考えて、メジャーバージョンをあげる(1.8と1.9が
> |>      通信できなくなる、うれしくない)
> |>
> |>  (2) ささいな違いなので気にしない
> |>
> |>  (3) 1.8で正規化する(すでに書き込んだデータは救済できないし、
> |>      副作用もある)
> |>
> |>  (4) 1.9側に1.8正規表現かどうか判別して1.9正規表現に変換する
> |>      (おそらくは巨大な)ルーチンを追加する(苦労の割に得るもの
> |>      が少ない、うれしくない)
> |
> |(5) 1.9でRegexp#_dumpとRegexp#_loadを定義する
> |ってのはどうでしょうね。すると
> |* TYPE_REGEXPなデータは1.8のRegexp
> |* TYPE_USERDEFなデータ(でklass==rb_cRegexp)は1.9のRegexp
> |という割と簡単な判定でいけそうな気がするんですが。
>
> 判別はできますが、その後の対応はどうするんでしょうね。
> 1.9に1.8のregex.cを導入するのも、1.8に鬼車を導入するのも現実
> 的ではないと思いますから、判別できても完全な互換性を維持する
> のは困難ではないでしょうか。
>   

「1.8から来た正規表現に\uが含まれていたら1.9ではuのこととして扱う」でい
いんじゃないですかね。
1.8が1.9から来る正規表現を読めるようになることは期待してません。

> さらに言うと、この件についてなんらかの対応したとしても、救済
> できるのは、ドキュメントに掲載されていないし、サポートされて
> いると明言されたこともない\uのようなメタキャラクタでないアル
> ファベットの前にバックスラッシュがついた正規表現だけなので、
> コストの割に得るものが少ない気がします。
>
> 考えるほど、ここは(2)かなあ、という気がしてきました。という
> か、むしろundocumentedなので対応すべきでないような。
>   

undocumentedなのが理由になれるのはちゃんとドキュメントがあるときだけでに
しましょうね。
たとえば/\n/や/\t/あたりもドキュメントされてませんけど、まさか今後の動作
を保証しないとか言い出しますか?

ところでなんでこんなにしつこく対応すべきと主張しているかというと、今後同
様のことがまだ起こると思うからです。
どうせ1.9は1.8と互換性がないわけで、Marshalで非互換が見つかったり、あら
たに作られたりすることは今後も出てくることが強く予想されます。そのとき
に、毎回同様の判断を求められるでしょう。それを毎回「気にしない」とかいう
逃げかたをしていくと、どんどんMarshalでやりとりできる情報が少なくなって
いって用をなさなくなります。しかもPStoreの場合だと中のオブジェクトが一個
でも読めなくなると、DB全体を捨てないといけません。これはとても困ります。
今後「気にしない」をどんどん続けていくと、最終的には読めないDBしか残らな
くなるでしょう(さすがにそこまで到達する前になんか回避策は考えることにな
ると思いますが)。

せめて過去のバージョンで読めていたデータは取り出せるべきです。

Updated by naruse (Yui NARUSE) over 3 years ago

成瀬です。

On Sun, 19 Oct 2008 02:13:16 +0900, Urabe Shyouhei  
<shyouhei@ruby-lang.org> wrote:

> 卜部です。
>
> Yukihiro Matsumoto さんは書きました:
>>  (1) 杓子定規に考えて、メジャーバージョンをあげる(1.8と1.9が
>>      通信できなくなる、うれしくない)
>>
>>  (2) ささいな違いなので気にしない
>>
>>  (3) 1.8で正規化する(すでに書き込んだデータは救済できないし、
>>      副作用もある)
>>
>>  (4) 1.9側に1.8正規表現かどうか判別して1.9正規表現に変換する
>>      (おそらくは巨大な)ルーチンを追加する(苦労の割に得るもの
>>      が少ない、うれしくない)
>>
>
> (5) 1.9でRegexp#_dumpとRegexp#_loadを定義する
> ってのはどうでしょうね。すると
> * TYPE_REGEXPなデータは1.8のRegexp
> * TYPE_USERDEFなデータ(でklass==rb_cRegexp)は1.9のRegexp
> という割と簡単な判定でいけそうな気がするんですが。
>
>
> # おもいつきレベルだけど。

そこまでしなくても、encoding の有無で判別できるような気がするんですが、
わたしの理解が間違っているのかなぁ。

-- 
NARUSE, Yui  <naruse@airemix.jp>

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36794] Re: [Bug #650] Marshal.load raises RegexpError"
    on Mon, 20 Oct 2008 01:50:06 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|「1.8から来た正規表現に\uが含まれていたら1.9ではuのこととして扱う」でい
|いんじゃないですかね。
|1.8が1.9から来る正規表現を読めるようになることは期待してません。

|undocumentedなのが理由になれるのはちゃんとドキュメントがあるときだけでに
|しましょうね。
|たとえば/\n/や/\t/あたりもドキュメントされてませんけど、まさか今後の動作
|を保証しないとか言い出しますか?

あー、では「意味」と「意図」と言い換えましょう。\nや\tがどの
ような意味を持つかについて、ほとんどの人は一定の期待を持つで
しょうし、それに応えるつもりは十分にあります。しかし、一方、
\uはどうでしょう? これにたいしてほとんどの人が合意できる意味
は存在しないと思います。元の正規表現を書いた人がどのような期
待をしたのかはわかりませんが、すくなくとも卜部さんがあげてく
ださった例では、「期待してるであろう動作(バックスラッシュ+u)」
と「実際の動作(u)」は異なっていました。要するに元のプログラム
のバグです。

このような「間違った使い方しか存在しない表現」は積極的にサポー
トしない方がよいのではないかと思います。

|ところでなんでこんなにしつこく対応すべきと主張しているかというと、今後同
|様のことがまだ起こると思うからです。
|どうせ1.9は1.8と互換性がないわけで、Marshalで非互換が見つかったり、あら
|たに作られたりすることは今後も出てくることが強く予想されます。そのとき
|に、毎回同様の判断を求められるでしょう。それを毎回「気にしない」とかいう
|逃げかたをしていくと、どんどんMarshalでやりとりできる情報が少なくなって
|いって用をなさなくなります。しかもPStoreの場合だと中のオブジェクトが一個
|でも読めなくなると、DB全体を捨てないといけません。これはとても困ります。
|今後「気にしない」をどんどん続けていくと、最終的には読めないDBしか残らな
|くなるでしょう(さすがにそこまで到達する前になんか回避策は考えることにな
|ると思いますが)。
|
|せめて過去のバージョンで読めていたデータは取り出せるべきです。

無条件で? 今回の場合はバージョンが進んだおかげでバグが見つ
かったのだと私には見えますけど。

                                まつもと ゆきひろ /:|)

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

もとの正規表現にバグがあるのは認めますが、それに巻き込まれてでかいPStore
のデータがまるごと読めなくて困ってるんですってば。

じゃあ妥協案ですけど、Marshal.loadに invalid: :replace とか受け取れるよ
うにしません?

Yukihiro Matsumoto さんは書きました:
> まつもと ゆきひろです
>
> In message "Re: [ruby-dev:36794] Re: [Bug #650] Marshal.load raises RegexpError"
>     on Mon, 20 Oct 2008 01:50:06 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:
>
> |「1.8から来た正規表現に\uが含まれていたら1.9ではuのこととして扱う」でい
> |いんじゃないですかね。
> |1.8が1.9から来る正規表現を読めるようになることは期待してません。
>
> |undocumentedなのが理由になれるのはちゃんとドキュメントがあるときだけでに
> |しましょうね。
> |たとえば/\n/や/\t/あたりもドキュメントされてませんけど、まさか今後の動作
> |を保証しないとか言い出しますか?
>
> あー、では「意味」と「意図」と言い換えましょう。\nや\tがどの
> ような意味を持つかについて、ほとんどの人は一定の期待を持つで
> しょうし、それに応えるつもりは十分にあります。しかし、一方、
> \uはどうでしょう? これにたいしてほとんどの人が合意できる意味
> は存在しないと思います。元の正規表現を書いた人がどのような期
> 待をしたのかはわかりませんが、すくなくとも卜部さんがあげてく
> ださった例では、「期待してるであろう動作(バックスラッシュ+u)」
> と「実際の動作(u)」は異なっていました。要するに元のプログラム
> のバグです。
>
> このような「間違った使い方しか存在しない表現」は積極的にサポー
> トしない方がよいのではないかと思います。
>
> |ところでなんでこんなにしつこく対応すべきと主張しているかというと、今後同
> |様のことがまだ起こると思うからです。
> |どうせ1.9は1.8と互換性がないわけで、Marshalで非互換が見つかったり、あら
> |たに作られたりすることは今後も出てくることが強く予想されます。そのとき
> |に、毎回同様の判断を求められるでしょう。それを毎回「気にしない」とかいう
> |逃げかたをしていくと、どんどんMarshalでやりとりできる情報が少なくなって
> |いって用をなさなくなります。しかもPStoreの場合だと中のオブジェクトが一個
> |でも読めなくなると、DB全体を捨てないといけません。これはとても困ります。
> |今後「気にしない」をどんどん続けていくと、最終的には読めないDBしか残らな
> |くなるでしょう(さすがにそこまで到達する前になんか回避策は考えることにな
> |ると思いますが)。
> |
> |せめて過去のバージョンで読めていたデータは取り出せるべきです。
>
> 無条件で? 今回の場合はバージョンが進んだおかげでバグが見つ
> かったのだと私には見えますけど。
>
>                                 まつもと ゆきひろ /:|)
>
>
>
>   

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36830] Re: [Bug #650] Marshal.load raises RegexpError"
    on Tue, 21 Oct 2008 12:37:17 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|もとの正規表現にバグがあるのは認めますが、それに巻き込まれてでかいPStore
|のデータがまるごと読めなくて困ってるんですってば。

別に意地悪を言ってるわけではないんですが。
loadに渡すprocが使えるか、と一瞬思ったのですが、これは再生し
たオブジェクトを引数にするので再生そのものが失敗する今回のケー
スは使えませんねえ。

|じゃあ妥協案ですけど、Marshal.loadに invalid: :replace とか受け取れるよ
|うにしません?

基本的にはいいアイディアだと思います。この場合、APIは

  Marshal.load(src, invalid: :replace)

とかにするんですかね。その場合、再生に失敗したオブジェクトは
なにに置換したらよいのでしょうか。nil?

                                まつもと ゆきひろ /:|)

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

Yukihiro Matsumoto さんは書きました:
> |じゃあ妥協案ですけど、Marshal.loadに invalid: :replace とか受け取れるよ
> |うにしません?
>
> 基本的にはいいアイディアだと思います。この場合、APIは
>
>   Marshal.load(src, invalid: :replace)
>
> とかにするんですかね。その場合、再生に失敗したオブジェクトは
> なにに置換したらよいのでしょうか。nil?
>   

ユーザーアプリケーション側は変換に失敗したかどうかを==で判定するとおもわ
れるので、nilみたいなものよりも普通のObject.newのほうがいいんじゃないで
しょうか。

  # rb_define_const(rb_mMarshal, "Invalid", rb_obj_alloc(rb_cObject);
  obj = Marshal.load src, invalid: :replace
  obj == Marshal::Invalid and raise "invalid data"


ObjectのインスタンスをMarshal.load(Marshal.dump)したものはもとのインスタ
ンスとは==にならないからこれで問題ないとおもいます。

Updated by znz (Kazuhiro NISHIYAMA) over 3 years ago

西山和広です。

At Tue, 21 Oct 2008 16:30:57 +0900,
Urabe Shyouhei wrote:
> 
> ユーザーアプリケーション側は変換に失敗したかどうかを==で判定するとおもわ
> れるので、nilみたいなものよりも普通のObject.newのほうがいいんじゃないで
> しょうか。
> 
>   # rb_define_const(rb_mMarshal, "Invalid", rb_obj_alloc(rb_cObject);
>   obj = Marshal.load src, invalid: :replace
>   obj == Marshal::Invalid and raise "invalid data"
> 
> 
> ObjectのインスタンスをMarshal.load(Marshal.dump)したものはもとのインスタ
> ンスとは==にならないからこれで問題ないとおもいます。

drubyの場合はMarshal.loadに失敗したときに、例外と元のデータを持った
DRbUnknownオブジェクトになるので、同じように理由と読み込めなかったデータを
持つオブジェクトにするのはどうでしょうか?


-- 
|ZnZ(ゼット エヌ ゼット)
|西山和広(Kazuhiro NISHIYAMA)

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36836] Re: [Bug #650] Marshal.load raises RegexpError"
    on Tue, 21 Oct 2008 16:30:57 +0900, Urabe Shyouhei <shyouhei@ruby-lang.org> writes:

|ユーザーアプリケーション側は変換に失敗したかどうかを==で判定するとおもわ
|れるので、nilみたいなものよりも普通のObject.newのほうがいいんじゃないで
|しょうか。
|
|  # rb_define_const(rb_mMarshal, "Invalid", rb_obj_alloc(rb_cObject);
|  obj = Marshal.load src, invalid: :replace
|  obj == Marshal::Invalid and raise "invalid data"
|
|ObjectのインスタンスをMarshal.load(Marshal.dump)したものはもとのインスタ
|ンスとは==にならないからこれで問題ないとおもいます。

ふむ。この線で対応しようかなと思うのですが、まだもう少しつめ
きれていない気がします。以下にいろいろと考えを書き連ねておき
ますので、意見があれば聞かせてください。

  * オブジェクト再生時に例外が発生したらObject.newに置き換え
    る挙動はデフォルトか?
  * 切り替え可能か(いつも置き換えてはいけないか)
  * Object.newにはどのような情報を持たすべきか
    * 特定のマーカーになるオブジェクト(使いまわす)
    * 単にObject.new
    * 特定のクラス(or Struct)のインスタンス
      * 例外や失敗した復元情報を含む?

いろいろ難しいものだ。

                                まつもと ゆきひろ /:|)

Updated by keiju (Keiju Ishitsuka) over 3 years ago

けいじゅ@いしつかです.

In [ruby-dev :36863 ] the message: "[ruby-dev:36863] Re: [Bug #650]
Marshal.load raises RegexpError ", on Oct/22 17:37(JST) Yukihiro
Matsumoto writes:

>まつもと ゆきひろです

>ふむ。この線で対応しようかなと思うのですが、まだもう少しつめ
>きれていない気がします。以下にいろいろと考えを書き連ねておき
>ますので、意見があれば聞かせてください。

私は西山さんの:

>drubyの場合はMarshal.loadに失敗したときに、例外と元のデータを持った
>DRbUnknownオブジェクトになるので、同じように理由と読み込めなかったデータを
>持つオブジェクトにするのはどうでしょうか?

の方が良い気がします.

例えば, 配列の一要素が復元できなかった場合など, 複数のオブジェクトを再
生したとき,

>>   obj = Marshal.load src, invalid: :replace
>>   obj == Marshal::Invalid and raise "invalid data"

このやり方では, 実際にどのオブジェクトの再生に失敗したのか調べるのが難
しいと思います. まず例外を発生させて, 再生できなかったことを明示的に知
らせて, ではどこがおかしいのかな? って中身を調べられる方がよいのでは?

あと, この方法だと, オプションの指定も必要ないような気がします. 常にマー
カーに置き換えちゃえば良いので.


__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

石塚圭樹 さんは書きました:
> 例えば, 配列の一要素が復元できなかった場合など, 複数のオブジェクトを再
> 生したとき,
>
>   
>>>   obj = Marshal.load src, invalid: :replace
>>>   obj == Marshal::Invalid and raise "invalid data"
>>>       
>
> このやり方では, 実際にどのオブジェクトの再生に失敗したのか調べるのが難
> しいと思います. まず例外を発生させて, 再生できなかったことを明示的に知
> らせて, ではどこがおかしいのかな? って中身を調べられる方がよいのでは?
>   

いや、まず例外が起きるなら現状と何ら変わりませんがな。

失敗したやつはともかくとして、それ以外の部分データを救いたいのが動機です。
# つまりそのサンプルコードはちょっといけてない

> あと, この方法だと, オプションの指定も必要ないような気がします. 常にマー
> カーに置き換えちゃえば良いので.
>   

例外のほうがうれしい局面ってあるんでしょうかね。
invalid: :replaceはもちろんString#encodeから引っ張ってきたアイディアです
けど、String#encodeではなんでデフォルトが例外なんでしょうか。

Updated by keiju (Keiju Ishitsuka) over 3 years ago

けいじゅ@いしつかです.

In [ruby-dev :36865 ] the message: "[ruby-dev:36865] Re: [Bug #650]
Marshal.load raises RegexpError ", on Oct/22 20:31(JST) Urabe Shyouhei
writes:

>卜部です。

>いや、まず例外が起きるなら現状と何ら変わりませんがな。

パラメータを指定するなら, 例外の捕捉を書いてもあまり変わらないと思いま
す. 

>失敗したやつはともかくとして、それ以外の部分データを救いたいのが動機
>です。

そうすると, 1.8で作成したデータを1.9で使えるようにコンバートしたいって
用途だと考えてよいでしょうか?

文字列と違って, 相手はオブジェクトなので, 再生を失敗したオブジェクトも
それなりのものに再変換する必要があると思います. 

また, 例えば, PStoreに入っているオブジェクト群を変換することを考えると,
ほとんどのものは正常に再生できることが多いと思いますので, 変換に失敗し
たものだけが例外になってもらった方がコンバートもしやすいです.

あと, 気になっているのが, 

  [1, 2, 3, <再生不能オブジェクト>]

が dump されていたとして, これをloadすると 何が帰ってくるのでしょう?
私は, まず例外を発生させて,

  [1, 2, 4, markerオブジェクト]

になるのかな? と考えていました. 例外を捕捉してから, オブジェクト内をス
キャンしていくイメージです. 卜部さんの案はどのように返ってくるのでしょ
う?

ただ, スキャンするといっても, 任意のオブジェクトのツリーのスキャンを一
般のユーザにやってもらうのは難しい気がしてきました. で, こんなのはいか
がでしょう?

obj = Marshal.load(src, invalid: proc{|invalid_info| ...})

再生不能オブジェクトに当たったら, 指定したprocを呼び出し, それなりのオ
ブジェクトを返してもらって, それをloadの返り値にアサインするというもの
です. これだったら, 複雑なネットワーク状になったオブジェクト群をloadし
た場合でも, それなりに対応できそうです.

>例外のほうがうれしい局面ってあるんでしょうかね。

converter的な使い方をするのであれば, 例外である必要はない気がします. 

>invalid: :replaceはもちろんString#encodeから引っ張ってきたアイディアです
>けど、

そうだったんですか... 
bladeで調べたんですけど, 探し方が悪いのか本質的な議論は見つかりません
でした. 他で議論されたものです?

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

Updated by shyouhei (Shyouhei Urabe) over 3 years ago

卜部です。

ちょと混乱してきました。石塚さんがおっしゃっておられる「例外」ってのはな
んなんでしょう。
私の中ではRubyのException(つまりrb_raise()でトリガするあれ)のつもりだっ
たので、例外を発生させたところで処理が中断する前提でお話をしていたのです
が、「変換に失敗したものだけが例外になってもらった方が」のくだりで指して
おられる「例外」は、どうもそれとは違うようで。


石塚圭樹 さんは書きました:
>> いや、まず例外が起きるなら現状と何ら変わりませんがな。
>>     
>
> パラメータを指定するなら, 例外の捕捉を書いてもあまり変わらないと思いま
> す. 
>
>   
>> 失敗したやつはともかくとして、それ以外の部分データを救いたいのが動機
>> です。
>>     
>
> そうすると, 1.8で作成したデータを1.9で使えるようにコンバートしたいって
> 用途だと考えてよいでしょうか?
>
> 文字列と違って, 相手はオブジェクトなので, 再生を失敗したオブジェクトも
> それなりのものに再変換する必要があると思います. 
>
> また, 例えば, PStoreに入っているオブジェクト群を変換することを考えると,
> ほとんどのものは正常に再生できることが多いと思いますので, 変換に失敗し
> たものだけが例外になってもらった方がコンバートもしやすいです.
>
> あと, 気になっているのが, 
>
>   [1, 2, 3, <再生不能オブジェクト>]
>
> が dump されていたとして, これをloadすると 何が帰ってくるのでしょう?
> 私は, まず例外を発生させて,
>
>   [1, 2, 4, markerオブジェクト]
>
> になるのかな? と考えていました. 例外を捕捉してから, オブジェクト内をス
> キャンしていくイメージです. 卜部さんの案はどのように返ってくるのでしょ
> う?
>
> ただ, スキャンするといっても, 任意のオブジェクトのツリーのスキャンを一
> 般のユーザにやってもらうのは難しい気がしてきました. で, こんなのはいか
> がでしょう?
>
> obj = Marshal.load(src, invalid: proc{|invalid_info| ...})
>
> 再生不能オブジェクトに当たったら, 指定したprocを呼び出し, それなりのオ
> ブジェクトを返してもらって, それをloadの返り値にアサインするというもの
> です. これだったら, 複雑なネットワーク状になったオブジェクト群をloadし
> た場合でも, それなりに対応できそうです.
>
>   
>> 例外のほうがうれしい局面ってあるんでしょうかね。
>>     
>
> converter的な使い方をするのであれば, 例外である必要はない気がします. 
>
>   
>> invalid: :replaceはもちろんString#encodeから引っ張ってきたアイディアです
>> けど、
>>     
>
> そうだったんですか... 
> bladeで調べたんですけど, 探し方が悪いのか本質的な議論は見つかりません
> でした. 他で議論されたものです?
>
> __
> ---------------------------------------------------->> 石塚 圭樹 <<---
> ---------------------------------->> e-mail: keiju@ishitsuka.com <<---
>   

Updated by keiju (Keiju Ishitsuka) over 3 years ago

けいじゅ@いしつかです.

In [ruby-dev :36871 ] the message: "[ruby-dev:36871] Re: [Bug #650]
Marshal.load raises RegexpError ", on Oct/23 02:07(JST) Urabe Shyouhei
writes:

>卜部です。
>
>ちょと混乱してきました。石塚さんがおっしゃっておられる「例外」ってのはな
>んなんでしょう。
>私の中ではRubyのException(つまりrb_raise()でトリガするあれ)のつもりだっ
>たので、例外を発生させたところで処理が中断する前提でお話をしていたのです
>が、「変換に失敗したものだけが例外になってもらった方が」のくだりで指して
>おられる「例外」は、どうもそれとは違うようで。

確かに説明不足でした.

例外オブジェクトのアクセサで取りあえずの変換結果が取り出せるイメージで
した.

入力が:

  [1, 2, 3, <再生不能オブジェクト>, 5]

だったら, 例外から取り出せるのは

  [1, 2, 4, <markerオブジェクト>, 5]

という感じです. ここから, markerオブジェクトを探して, 適当に処理するイ
メージでした.

ただ, 今は, 前メイルの理由より, API的には

  obj = Marshal.load(src, invalid: proc{|invalid_info| ...})

の方がふさわしいと感じています. 

さらに, invalid_info は適当な(適当な情報を持った)例外がいいんじゃない
かと思っています. つまり, invalid: が指定されなかったとき発生するもの
と同じ例外です. procでは, 個々のオブジェクトの再生が失敗したという例外
を引数として受け取ってそれなりに処理する感じです. 

この問題は, 松本さんの[ruby-dev:36784]にあった:

> Marshalファイルフォーマットには、メジャーバージョンとマイナー
> バージョンの情報が付加されており、マイナーバージョンの違いは
> 「上位互換性がある」ので「古いmarshal formatのデータを読もう
> とすると、警告が出るが処理自体はできる」、メジャーバージョン
> の違いは「互換性はないので古いデータは読めない」とする仕組み
> があります。

でのメジャーバージョンを越えるための方法を考えることであり, 結構重要な
問題だと思っています. 卜部さんもその辺りを気にしているのではと感じてい
ます. Rubyも今となっては, メジャーバージョンが変わったので互換性がない
からあきらめてね, とはいえない状態だと考えています.

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:36872] Re: [Bug #650] Marshal.load raises RegexpError"
    on Thu, 23 Oct 2008 03:26:00 +0900, keiju@ishitsuka.com (石塚圭樹) writes:

|例外オブジェクトのアクセサで取りあえずの変換結果が取り出せるイメージで
|した.

例外は実行を中断しちゃうんで、「とりあえずの変換結果」ではか
なり情報が失われるような気がします。

|入力が:
|
|  [1, 2, 3, <再生不能オブジェクト>, 5]
|
|だったら, 例外から取り出せるのは
|
|  [1, 2, 4, <markerオブジェクト>, 5]
|
|という感じです. ここから, markerオブジェクトを探して, 適当に処理するイ
|メージでした.

この例だと「5の再生に届かない」って感じでしょうか。
5を復元するためには例外が発生した情報だけはとっておいてとり
あえず最後まで復元して、改めて例外を発生し直すとかいうことに
なりそうですが、複数の例外が発生したらどうするかとか悩ましい
限りです。

|ただ, 今は, 前メイルの理由より, API的には
|
|  obj = Marshal.load(src, invalid: proc{|invalid_info| ...})
|
|の方がふさわしいと感じています. 

こちらならcallbackとして呼ばれますから、複数の例外などの問題
はありませんね。

|さらに, invalid_info は適当な(適当な情報を持った)例外がいいんじゃない
|かと思っています. つまり, invalid: が指定されなかったとき発生するもの
|と同じ例外です. procでは, 個々のオブジェクトの再生が失敗したという例外
|を引数として受け取ってそれなりに処理する感じです. 

例外が良いかもしれないことを頭から否定するわけではありません
が、たとえばmarshal_loadメソッドから発生した例外とかは事前に
予測困難ですから、統一的な対応はなかなか難しいかもしれません。

|この問題は, 松本さんの[ruby-dev:36784]にあった:
|
|> Marshalファイルフォーマットには、メジャーバージョンとマイナー
|> バージョンの情報が付加されており、マイナーバージョンの違いは
|> 「上位互換性がある」ので「古いmarshal formatのデータを読もう
|> とすると、警告が出るが処理自体はできる」、メジャーバージョン
|> の違いは「互換性はないので古いデータは読めない」とする仕組み
|> があります。
|
|でのメジャーバージョンを越えるための方法を考えることであり, 結構重要な
|問題だと思っています. 卜部さんもその辺りを気にしているのではと感じてい
|ます. Rubyも今となっては, メジャーバージョンが変わったので互換性がない
|からあきらめてね, とはいえない状態だと考えています.

まあ、そうなんですが、今さらメジャーバージョンが変わることは
ないと思います。とりあえず不足はないと思うし。

気になっている点は

  * ユーザ定義の_load,_dump,marshal_dump,marshal_loadが変化
    したらどうするのか
  * markerとなるオブジェクトはどんなオブジェクトでどんな属性
    を持つのか
  * callbackにするのか、markerを埋め込んだオブジェクトを返す
    のか

などがあります。

                                まつもと ゆきひろ /:|)

Updated by keiju (Keiju Ishitsuka) over 3 years ago

けいじゅ@いしつかです.

In [ruby-dev :36882 ] the message: "[ruby-dev:36882] Re: [Bug #650]
Marshal.load raises RegexpError ", on Oct/23 17:39(JST) Yukihiro
Matsumoto writes:

>まつもと ゆきひろです
>
>例外は実行を中断しちゃうんで、「とりあえずの変換結果」ではか
>なり情報が失われるような気がします。

えー.

>|  [1, 2, 3, <再生不能オブジェクト>, 5]
>|  [1, 2, 4, <markerオブジェクト>, 5]
>
>この例だと「5の再生に届かない」って感じでしょうか。
>5を復元するためには例外が発生した情報だけはとっておいてとり
>あえず最後まで復元して、改めて例外を発生し直すとかいうことに
>なりそうですが、複数の例外が発生したらどうするかとか悩ましい
>限りです。

後者のイメージでした. 確に複数の例外の問題はありますね. <marker>に情報
を埋め込めばどうにかなるとは思いますが. 
とはいえ, こちらの案はもう良いです.


>|  obj = Marshal.load(src, invalid: proc{|invalid_info| ...})
>
>こちらならcallbackとして呼ばれますから、複数の例外などの問題
>はありませんね。

そうでもなくて, 1オブジェクトで2種類の例外はなくはないんですよね. 今回
の問題はRegexpの問題でしたが, また, 同じクラスで別の問題が発生する可能
性がなくもないです.

>例外が良いかもしれないことを頭から否定するわけではありません
>が、たとえばmarshal_loadメソッドから発生した例外とかは事前に
>予測困難ですから、統一的な対応はなかなか難しいかもしれません。

converterを作るって話であれば, 統一的な対応でなくても良い気がします.
というか, 将来どういう非互換が発生するかは読めないですからね.

>まあ、そうなんですが、今さらメジャーバージョンが変わることは
>ないと思います。とりあえず不足はないと思うし。

変えたくないって話なんですよね? でも, storeしたものが(何も対処しないで)
load出来ないってことは, 事実上メジャーバージョンが変わっていると思いま
すが?

>  * ユーザ定義の_load,_dump,marshal_dump,marshal_loadが変化
>    したらどうするのか

いろいろな変更があると思いますが, ユーザ定義でも互換性を維持して行うの
が当然ですので, 互換性を維持できない場合ですよね. 

marshal_load で invalidが指定されているときは, marshal_loadもinvalidを
指定して呼び出し, コールバックするなりmakerを生成するなりすれば良いの
では?

>  * markerとなるオブジェクトはどんなオブジェクトでどんな属性
>    を持つのか

これが問題ですよねぇ. 最低でもダンプしたイメージは欲しいと思いますが,
あとは, 推選の変換オブジェクトがあればそういうのもあると良い気がします.

marshal_load(obj)の場合は, objを渡すと良いですね. 一般的にもこういうの
があるといいんですが, あ, 問題が発生したときに考えれば良いのかな? 今回
はRegexp なので, Regexp.new(string, option, code)のstring, option,
codeを渡せばほとんど再生できます. 

あとは, 何の非互換性かを識別するIDなり例外クラスが欲しいですよね. 

今回の例をコールバックの例で考えると

  proc do |exp|
    case exp
    when MarshalInvalid_Regex_backslash_u
      newstr = exp.string.gsub("\\u", "u")
      Regex.new(newstr, exp.option, exp.code)
    :

みたいなかんじでしょうか.    

>  * callbackにするのか、markerを埋め込んだオブジェクトを返す
>    のか

これは, 後者の方が良いと思いますねぇ. maker埋め込みはあとで, スキャン
するのがたいへんそうです. 無限ループしたりね...

__
---------------------------------------------------->> 石塚 圭樹 <<---
---------------------------------->> e-mail: keiju@ishitsuka.com <<---

Updated by ko1 (Koichi Sasada) over 3 years ago

  • Assignee set to matz (Yukihiro Matsumoto)

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Target version set to 2.0.0
1.9.1には間に合わなそうです。直接的な解決はしません @ Ruby開発者会議

Updated by naruse (Yui NARUSE) almost 3 years ago

  • ruby -v set to ruby 1.9.2dev
修正案です。エンコーディングを見て、UTF-8でない場合は1.8からとみなして、s/\\u/u/gしています。

diff --git a/marshal.c b/marshal.c
index 0c44eba..d572ecd 100644
--- a/marshal.c
+++ b/marshal.c
@@ -1393,8 +1393,19 @@ r_object0(struct load_arg *arg, int *ivp, VALUE extmod)
        {   
            volatile VALUE str = r_bytes(arg);
            int options = r_byte(arg);
+           v = r_entry(rb_reg_new("", 0, options), arg);
+           if (ivp) {
+               r_ivar(v, arg);
+               rb_p(v);
+               *ivp = Qfalse;
+           }
+           rb_enc_copy(str, v);
+           if (rb_enc_get_index(str) != rb_utf8_encindex()) {
+#define f_gsub_bang(x,y,z) rb_funcall(x, rb_intern("gsub!"), 2, y, z)
+               f_gsub_bang(str, rb_reg_new("\\\\u", 3, 0), rb_usascii_str_new_cstr("u"));
+           }
            v = r_entry(rb_reg_new_str(str, options), arg);
-            v = r_leave(v, arg);
+           v = r_leave(v, arg);
        }
        break;

Updated by naruse (Yui NARUSE) almost 3 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
Applied in changeset r23999.

Also available in: Atom PDF