Enable to insert unblessed values with strict-mode SQL::Maker #132

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

nobuoka commented Jul 10, 2014

Currently, Teng can be used with strict-mode SQL::Maker.

When we try to insert unblessed values using Teng->insert method, the SQL::Maker instance throws following error.

cannot pass in an unblessed ref as an argument in strict mode at /home/nobuoka/Documents/perl/p5-Teng/lib/Teng.pm line 330.

This is because

In order to enable to insert unblessed values with strict-mode SQL::Maker, I think Teng#do_insert method should set an blessed value (instead of arrayref) to a value of hashref for the second argument of SQL::Maker#insert method.

How about this? Thanks.

Collaborator

cho45 commented Jul 10, 2014

ちょっと完全にコード読めてないですが、自動的にブレスするようにすると、結局 JSON そのまま渡して脆弱性、にならないですか?

Contributor

nobuoka commented Jul 10, 2014

脆弱性になりうる例は、次のような場合ですよね?

# ユーザー入力として文字列を期待しているのに hashref を受け取ってしまった。
my $user_input_value = { xxx => "..." };
$teng->insert($table_name, {
    name => $user_input_value,
});

この場合、lib/Teng.pm の 311 行目 ( https://github.com/nekokak/p5-Teng/pull/132/files#diff-da7a9c9199ccf861828a0cc132c71c8fR311 ) において、$args->{$col} にユーザー入力値 (hashref) が渡ってくるので、条件演算子の条件部が真になり、bless される方には来ません。

$bind_args->{$col} = ref $args->{$col} ? $args->{$col} : bless([ $args->{$col}, $table->get_sql_type($col) ], 'Teng::_ValueTypePair');

なので、文字列を期待しているユーザー入力に JSON (arreyref や hashref) が渡されても、今のコードであれば大丈夫だと思います。

今回修正したいのは次のような問題です。

# reference でない文字列や数値を insert する。
my $value = "...";
$teng->insert($table_name, {
    name => $value,
});

この場合、変更前は lib/Teng.pm の 310 行目 ( https://github.com/nekokak/p5-Teng/pull/132/files#diff-da7a9c9199ccf861828a0cc132c71c8fL310 ) の条件演算子が偽になり、$bind_args の値が arrayref になります。

そして、$bind_args は lib/Teng.pm の 331 行目 ( https://github.com/nobuoka/p5-Teng/blob/fix/insert_normal_value_with_strict_sql_builder/lib/Teng.pm#L331 ) で $self->{sql_builder}->insert メソッドの第 2 引数として渡されますので、$self->{sql_builder} が strict モードの SQL::Maker であった場合に 『cannot pass in an unblessed ref as an argument in strict mode』 のエラーが発生してしまいます。

今回の変更は、このエラーを解消するのが目的です。

Collaborator

cho45 commented Jul 10, 2014

ありがとうございます。要は Teng 自体が strict でも bless されてない arrayref 作って渡してるのがエラーになってるてことですね。

strict => 1

  1. $value が reference の場合
    • そのまま渡される → SQL::Maker 側で blessed でないならはじかれるので安全
  2. $value がただの値の場合
    • bless されて渡される → SQL::Maker 側で通るけどただの値なので安全

strict => 0 (互換性)

  1. $value が reference の場合
    • 何も変わってないので問題なし
  2. $value がただの値の場合
    • bless されて渡される → SQL::Maker 側では素通り → Teng#execute への変更で元と同じ挙動

なので大丈夫であってますか?

Contributor

nobuoka commented Jul 10, 2014

わかりやすくまとめてくださってありがとうございます。 仰る通りの挙動になるはずなので、大丈夫だという認識です。

strict モードのエラー回避のためだけに bless するのは微妙だとは思うので他に良い方法があれば良いのですが、特に思いつきませんでした。

Collaborator

cho45 commented Jul 10, 2014

@tokuhirom なんかいい方法ありますか? SQL::Maker 側で何かできることでもないし、こうするしかない感じではありますが…

Collaborator

tokuhirom commented Jul 10, 2014

これ、SQL::Maker の strict mode で、今までできていたことができなくなっているということであって、SQL::Maker 側が治すべきかと考えますが、どういう風に対応するのがよいかがちょっと思いつかない。
Cc @kazuho

Contributor

kazuho commented Jul 11, 2014

変数を型を指定してバインドするにあたっては、sql_type という関数を SQL::Maker が提供しているので、こちらを使うことできれいに書けるのではないでしょうか。

Contributor

kazuho commented Jul 11, 2014

PS. SQL::Maker が提供している sql_type はまともに動いていなさそうなので、まずそちらを直します。

Collaborator

cho45 commented Jul 11, 2014

😵

Contributor

kazuho commented Jul 11, 2014

とくひろむと相談した結果 sql_type のインターフェイスがイケてないのはそのままにするということで、現状で動く方式のPRを #133 として起こしました。動作確認の上 merge していただければと思います。

Contributor

nobuoka commented Jul 11, 2014

なるほど。 sql_type というのが SQL::Maker にあるのですね。 気づいていませんでした。

変更ありがとうございます! < #133

@cho45 cho45 closed this in 526002e Jul 14, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment