[ruby-dev:40467] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40467] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

NARUSE, Yui-2
成瀬です。

(2010/02/24 0:51), [hidden email] wrote:

> knu 2010-02-24 00:51:01 +0900 (Wed, 24 Feb 2010)
>
>   New Revision: 26739
>
>   http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=26739
>
>   Log:
>     * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the
>       OpenSSL::Digest class in place of where either an instance of
>       the class or the algorithm name was demanded.  For example,
>       OpenSSL::HMAC.digest(OpenSSL::Digest::SHA1, key, data) is now
>       accepted as well as the usual
>       OpenSSL::HMAC.digest(OpenSSL::Digest::SHA1.new, key, data) and
>       OpenSSL::HMAC.digest("SHA1", key, data).
>
>   Modified files:
>     trunk/ChangeLog
>     trunk/ext/openssl/ossl_digest.c


議論なしのAPI変更に加えて、ext/openssl への変更なのであまり感心しないんですが、

とりあえず、
irb(main):002:0> OpenSSL::HMAC.digest(1,"foo","bar")
TypeError: wrong argument (Fixnum)! (Expected kind of OpenSSL::Digest)
だったエラーが、
TypeError: can't convert Fixnum into String
というエラーに変わっているのは意図していますか。

また、API変更をよしとするならば、テストも追加するべきじゃありませんか。

--
NARUSE, Yui  <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40468] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

Akinori MUSHA
At Wed, 24 Feb 2010 01:28:24 +0900,
NARUSE, Yui wrote:
> 議論なしのAPI変更に加えて、ext/openssl への変更なのであまり感心しないんですが、

 メンテナ不在だと思っていたので気軽にいじってしまいました。

> とりあえず、
> irb(main):002:0> OpenSSL::HMAC.digest(1,"foo","bar")
> TypeError: wrong argument (Fixnum)! (Expected kind of OpenSSL::Digest)
> だったエラーが、
> TypeError: can't convert Fixnum into String
> というエラーに変わっているのは意図していますか。

 そういう変化をもたらすことを予想していたかと言われればイエスです。

 もしご存じでしたら、そのテストの意図を教えてください。

> また、API変更をよしとするならば、テストも追加するべきじゃありませんか。

 そもそも今のAPIに対するテストがなかったので追加しました。

--
Akinori MUSHA / http://akinori.org/

attachment0 (203 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40469] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

NARUSE, Yui-2
(2010/02/24 1:57), Akinori MUSHA wrote:
> At Wed, 24 Feb 2010 01:28:24 +0900,
> NARUSE, Yui wrote:
>> 議論なしのAPI変更に加えて、ext/openssl への変更なのであまり感心しないんですが、
>
>  メンテナ不在だと思っていたので気軽にいじってしまいました。

メンテナの不在は自由に仕様変更が出来る事を意味しないはずです。
そもそも、仕様変更に寛容だった昔の牧歌的なノリは困ります。
今は RubySpec や各種ドキュメントなど影響範囲が大きいのですから。

また、別の Digest の継承もそうなんですが、ext/openssl への変更はおっかないので
先にMLに差分を投げてレビューを貰って欲しいです。

>> とりあえず、
>> irb(main):002:0> OpenSSL::HMAC.digest(1,"foo","bar")
>> TypeError: wrong argument (Fixnum)! (Expected kind of OpenSSL::Digest)
>> だったエラーが、
>> TypeError: can't convert Fixnum into String
>> というエラーに変わっているのは意図していますか。
>
>  そういう変化をもたらすことを予想していたかと言われればイエスです。
>
>  もしご存じでしたら、そのテストの意図を教えてください。

ext/openssl でバグが出ると嫌なので差分を見て気付きました。

>> また、API変更をよしとするならば、テストも追加するべきじゃありませんか。
>
>  そもそも今のAPIに対するテストがなかったので追加しました。

そのテストはどこですか?

--
NARUSE, Yui  <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40470] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

Akinori MUSHA
At Wed, 24 Feb 2010 02:07:00 +0900,
NARUSE, Yui wrote:

> (2010/02/24 1:57), Akinori MUSHA wrote:
> > At Wed, 24 Feb 2010 01:28:24 +0900,
> > NARUSE, Yui wrote:
> >> 議論なしのAPI変更に加えて、ext/openssl への変更なのであまり感心しないんですが、
> >
> >  メンテナ不在だと思っていたので気軽にいじってしまいました。
>
> メンテナの不在は自由に仕様変更が出来る事を意味しないはずです。
> そもそも、仕様変更に寛容だった昔の牧歌的なノリは困ります。
> 今は RubySpec や各種ドキュメントなど影響範囲が大きいのですから。
 わかりました。一度戻します。テストがうまくコミットできなかった
ようなので併せて更新します。


 OpenSSL::Digest については前から関心があり、最初の変更は無事に
入って喜んだものの、しばらくして次の変更ができたときにはメンテナ
不在状態でメールの反応がもらえなくなってお蔵入りになっていたものを、
現実逃避中に発掘したのでつい入れてしまったというところです。

> また、別の Digest の継承もそうなんですが、ext/openssl への変更はおっかないので
> 先にMLに差分を投げてレビューを貰って欲しいです。

 聞くばかりですみませんが、議論の場所はここでいいのでしょうか。

--
Akinori MUSHA / http://akinori.org/

attachment0 (203 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40472] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

Hiroshi Nakamura-3
2010/2/24 Akinori MUSHA <[hidden email]>:
> OpenSSL::Digest については前から関心があり、最初の変更は無事に
> 入って喜んだものの、しばらくして次の変更ができたときにはメンテナ
> 不在状態でメールの反応がもらえなくなってお蔵入りになっていたものを、
> 現実逃避中に発掘したのでつい入れてしまったというところです。

もったいないですねえ。メンテナ問題、どうしたものやら。

>> また、別の Digest の継承もそうなんですが、ext/openssl への変更はおっかないので
>> 先にMLに差分を投げてレビューを貰って欲しいです。
>
>  聞くばかりですみませんが、議論の場所はここでいいのでしょうか。

議論の場所が決まったら教えてください。議論に参加希望です。

暗号処理の分野では、Mac is-a
MessageDigestは成り立たないので、サブクラス化に反対する予定です。できればDigest(非OpenSSL版)も戻したい。

Reply | Threaded
Open this post in threaded view
|

[ruby-dev:40473] Re: [ruby-cvs:33954] Ruby:r26739 (trunk): * ext/openssl/ossl_digest.c (GetDigestPtr): Allow to pass the

Akinori MUSHA
At Wed, 24 Feb 2010 06:06:13 +0900,
NAKAMURA, Hiroshi wrote:
> >> また、別の Digest の継承もそうなんですが、ext/openssl への変更はおっかないので
> >> 先にMLに差分を投げてレビューを貰って欲しいです。
> >
> >  聞くばかりですみませんが、議論の場所はここでいいのでしょうか。
>
> 議論の場所が決まったら教えてください。議論に参加希望です。
>
> 暗号処理の分野では、Mac is-a MessageDigestは成り立たないので、サブク
> ラス化に反対する予定です。できればDigest(非OpenSSL版)も戻したい。

 そういえば ruby_1_8 にバックポートせずに trunk に置いたまま、
いつか議論しようと思っているうちに 1.9.1 が出てしまったのでした。

 私の主眼は実装の共有にあるのですが、どのようなモジュール構成が
いいと思いますか?

--
Akinori MUSHA / http://akinori.org/

attachment0 (203 bytes) Download Attachment