Home > Latest topics

Latest topics > 拡張機能におけるeval()の5つの間違った使い方

宣伝。日経LinuxにてLinuxの基礎?を紹介する漫画「シス管系女子」を連載させていただいています。 以下の特設サイトにて、単行本まんがでわかるLinux シス管系女子の試し読みが可能! シス管系女子って何!? - 「シス管系女子」特設サイト

拡張機能におけるeval()の5つの間違った使い方 - Nov 28, 2009

以下、Adblock Plus and (a little) more: Five wrong reasons to use eval() in an extensionのいいかげんな訳です。XUL/Migemoのバージョンアップ時のエディタによるレビューのコメントで「今回は公開を承認するけど、次からはeval()はなるべく減らすように。詳しくはこれを読んで。」と指摘されたので、自分が読むために訳してみました。誤訳があったら指摘して。一部のサンプルコードは見やすさのためにインデントを勝手に加えてます。

ちなみに、僕は5番目の点(こういう用途でeval()を使うなという話)については反対の立場です。拡張機能同士を協調して動作させたいなら、むしろeval()を使って関数を書き換えるやり方を使う方が望ましいとすら考えています。なので参考のために、似たような立場と思われるSimon氏・Dorando氏のコメントも訳しています。

2010年2月8日追記。このエントリで述べられている内容に対する反論を公開しました。できればそちらも併せてご覧下さい。


過剰に使われているJavaScriptの機能のひとつに、eval()関数があります。私はそれが非常に多くの拡張機能で利用されているのを見てきましたが、そのうちのごく一部だけが正しい使われ方をしています。ですので、eval()のすべての間違った使い方について説明したいと思います。

1. JSON形式のデータのパースのため

今日において、JSONはデータを保存するためのポピュラーな形式となっています。その最大の特長は、パースが非常に簡単であるということです。単に data = eval(json) という風に書けば、それだけで事足ります。

うまい話には裏があるんじゃないの? その通り。このjsonという変数は {foo: "bar" + alert(Components.classes)} のような内容が含まれるかもしれず、このようなJavaScriptのコードを実行してしまうと、あなたが意図していなかった結果になってしまうでしょう。このように、信頼できない情報源からやってきたデータをJSONとしてパースする用途にはeval()は全く不向きです。それがFirefoxの拡張機能であるなら、どんなWebサーバから送られてくるデータも信頼できません。もしそれがあなたのWebサーバであっても、ハックされて(※訳註:原文ではhacked)いるかもしれませんし、ユーザへの通信経路上で情報が改竄されているかもしれません(特に、暗号化されていない接続では)。あなたは、ユーザを危険な状況に晒したくはないでしょう。

それだけではありません。そのデータが拡張機能自身によって(例えば、ブラウザ終了時の状態を保存するためなどの目的で)書き出されたデータであっても、常に信頼できるとは限りません。その中にはひょっとしたら、Webから受け取ったデータが含まれるかもしれません。もしJSONを書き出す処理にバグがあって、JavaScriptの文字列として書き出すべき物が文字列になっていなければ、それをJSONとして解釈しようとした時、知らない間にJavaScriptのコードとして実行されてしまうでしょう。これが、JSON処理専用の機能を必ず使うようにした方が良い理由です。JSON処理専用の機能は、不正なデータを受け取った時にもJavaScriptのコードとして実行してしまわないので安全です。

2. プロパティ名が動的に変化する時に、そのオブジェクトのプロパティにアクセスするため

obj.fooNNnという変数の値である、というプロパティにアクセスしたいとき、どんなコードを書けばよいでしょうか? これは、あなたがアクセスしなければいけないプロパティの名前が事前には分からなくて、動的に決定されるものであるという場合のことです。拡張機能の中には、これを eval("obj.foo" + n) のようなやり方で解決しているものがあります。この時、その拡張機能はnの値の中に危険な内容が含まれていないかを検証する必要があるでしょう――でも、どうやって?

幸いにも、この質問の答えを考える必要はありません。もっといい方法があります。JavaScriptではすべてのオブジェクトが連想配列である(※訳註:原文ではassociative arrays)ことを思い出してください。言い換えると、obj.fooobj["foo"] は全く同じ意味で、すべてのプロパティは配列の要素としてアクセスできるのです。ですから、前述のような問題を解決するには単に obj["foo" + n] とだけ書けばよく、この操作は、何も他の余計なことをすることなく常にそのプロパティにアクセスするでしょう。

では、メソッド(関数)の場合は? JavaScriptではメソッドも、値が関数オブジェクトであるという違いがあるだけのただのプロパティです。その関数を this が正しい値を示すようにして呼び出すために、Function.call() というメソッドが利用できます:

var method = obj["foo" + n];
method.call(obj, param1, param2);

あるいは簡潔にこう書くこともできます:

obj["foo" + n](param1, param2);

同じアプローチが、グローバル変数やグローバルな関数に対しても使えます。「グローバルオブジェクト」のすべてのプロパティはwindowのプロパティとして参照できます。window.foowindow["foo"] は、グローバル変数fooの値を返すでしょう。(※訳註:JavaScriptコードモジュールなどwindowが使えない変数スコープでも、 (function() { return this; })() とすればその実行時の変数スコープのグローバルオブジェクトを取得できます。)

3. 関数に対して、その関数が処理を終えたあとに何をするべきかを示すため

私が時々見かけるひとつのパターンは、このような関数の呼び出し方です:

foo("window.close()");

その関数は他の場面で、異なるJavaScriptのコードをパラメータとして渡されていました。そして関数が処理を終えた後で、パラメータとして渡された内容を動作の指定として eval() で実行するようになっていました。

どう見ても、ここにはセキュリティ上の問題はありません(※訳註:もちろん、パラメータで渡す内容にWebから取ってきたデータが含まれる可能性がある時は問題外ですよ!)。では、このアプローチの一体どこが間違っているのでしょうか? 実際には、以下のような問題があります:

  • このコードはeval()が呼ばれるまでコンパイルされないでしょう。これは、それ以外の部分のコードについてはスクリプトが読み込まれた時にすぐにJavaScriptインタープリタが文法エラーを報告するのに対して、関数のパラメータとして渡されたコードの文法エラーは後になってからしか報告されないため、そのコードが実行されるような経路を辿る操作をあなたがテストしなかった場合に、問題が見過ごされてしまうだろうということを意味します。
  • もう1つの問題は、そのコードの中で起こったエラーに対して、JavaScriptインタープリタが正しいソースファイルと行番号を報告することができず、どこで問題が起こったのかを知ることができないという点です。このようなエラーのデバッグはとても面倒です。
  • 付け加えると、foo()に対して実行して欲しいコードをパラメータとして渡すというのは普通のやり方ではなく、見苦しい回避方法を色々と必要とします。(※訳註:ダブルクォーテーションをエスケープしないといけない、など。)

幸いにも、クロージャを使うことによってそれらの問題は解決できます(※訳註:この例はクロージャではなく関数リテラルとか関数オブジェクトとかその辺の話だと思うんですが……)。以下は、前述のコードを少し書き換えた例です:

foo(function(error)
{
  alert(error);
  window.close();
});

そしてfoo()という関数の内容は以下のようになるでしょう:

function foo(callback)
{
  ...
  callback("Full success");
}

4. HTMLやXULにインラインで記述されたイベントハンドラを実行するため

以下のようなボタンがあると仮定しましょう:

<button id="button" oncommand="doSomething();"/>

このイベントハンドラを実行するために eval(document.getElementById("button").getAttribute("oncommand")) としてはいけないのは何故でしょうか? その要素が実際にクリックされたなどの場合以外の所でイベントハンドラを実行するための方法として、拡張機能の中ではしばしばこのようなやり方が用いられます。しかしながら実は、commandイベントを生成する方法のほうがもっと簡単で、しかもイベントハンドラがどのように定義されていようとも正常に動作することが期待できます:

document.getElementById("button").doCommand();

doCommand()というメソッドは、すべてのXUL要素で利用可能です。他のイベントに対しては、document.createEvent()を使って本当のイベントオブジェクトを生成する方がよいでしょう――何故なら、イベントハンドラがそれを期待しているでしょうから。例えば:

var event = document.createEvent("MouseEvents");
event.initMouseEvent("click", true, true, window,
                     0, 0, 0, 0, 0,
                     false, false, false, false,
                     0, null);
document.getElementById("button").dispatchEvent(event);

では、あなたが「onfooaction」という風な独自の属性を定義していて、それがいかなる実際のイベントとも関連付けられていない場合には? このような場面でも、eval()を使うのは最良の選択とは言えません。何故なら、eval()を呼び出した関数の実行コンテキストにおいてコードが実行されてしまうからです。もしそのイベントハンドラがfooというグローバル変数を参照したとして、あなたがそのイベントハンドラを呼んでいる関数の中にfooという名前のローカルな変数があったら――そのイベントハンドラは意図しないままにそのローカル変数にアクセスしてしまうでしょう。そしてもちろん、その時イベントハンドラに対してパラメータを渡すこともできません。よりベターな解決策は、そのイベントハンドラのための関数を作る事でしょう:

var handler = new Function("param1", "param2",
                           document.getElementById("button")
                                   .getAttribute("onfooaction"));
handler("foo", "bar");

このシナリオでは、このイベントハンドラは「foo」をparam1という名前の引数として、「bar」をparam2として受け取るでしょう。(これは、よくあるインラインで記述されたイベントハンドラに対してeventというパラメータを渡す時にも使えます。)

5. ブラウザの関数を書き換えるため

以下のようなことをしているコードをよく見かけます:

gBrowser.foo = eval(gBrowser.foo
                            .toString()
                            .replace("foo", "bar"));

このようなやり方でブラウザの関数を書き換えている人は、公の場所でおしりペンペンされることをお勧めします。それは、ブラウザの関数を新しい関数で単に置き換えてしまうだけの拡張機能に比べてほんのちょっとだけマシであるに過ぎません。どちらの場合も、書き換えられようとしているコードが変化しないことを前提にしていますが――しかし、もしそれが起こったら? 最良のケースでは、その拡張機能は大きな損害を与えることなく単に動作しなくなるでしょう。しかしひょっとすると、ブラウザを壊してしまうかもしれません。あるいは、ブラウザのその関数がセキュリティ上の問題の修正のために書き換えられたとしたら、その拡張機能は同じセキュリティ上の問題をまた持ち込んでしまうかもしれません。

言い換えると――この使い方はしてはいけません。ほとんどの場合で、このアイデアはブラウザの関数の動作を変えるのではなく、その関数の前後に追加の処理を挿入するために使われています。幸いなことに、それをするためのもっと危険でないやり方として、ここでもクロージャを使えば元の関数を単純にあなたの関数で包み込むことができます:

var origFoo = gBrowser.foo;
gBrowser.foo = function(param1, param2)
{
  if (param1 == "top secret")
    doSomethingBeforeFoo();
  var result = origFoo.apply(this, arguments);
  if (result == null)
    doSomethingAfterFoo();
  return result;
}

元の関数にすべてのパラメータを渡すためにFunction.apply()を使うことに注意してください。その関数が今現在2つだけパラメータを受け取っているとしても、将来のバージョンのブラウザでは変わるかもしれません。あなたの拡張機能は新しいパラメータに対して何をすればよいのか知っているかもしれませんが、元の関数の動作を壊さないために、それらの新しいパラメータはそのまま元の関数へ引き渡しましょう。

eval()の正しい使い方とは?

私は、eval()関数の有効な使い方はそれほど無いと思っています。拡張機能の中にはユーザに対して、評価可能なJavaScriptのコードを入力させることを許しているものもあります。そのスクリプトが関数のパラメータとして値を受け取る必要があり、関数を作り変数を渡すのにFunction()コンストラクタを使うことが依然として望ましいとしても、これはeval()の妥当な使い方でしょう。

もう1つのeval()の使い方として、状況に応じて定数を宣言するためにも使えます:

if (typeof MY_CONSTANT == "undefined")
  eval("const MY_CONSTANT = 'foo'");

こうすることによって、もし他のスクリプトで同じ名前の定数が定義されていても、あなたは文法エラーを目にしなくて済むでしょう。しかしながら、私はこれはその場しのぎのやり方だと思います。もし同じ名前空間で実行される未知のスクリプトと衝突することを恐れているのなら、あなたは他のスクリプトが使わないような一意な名前を定数(グローバル変数も)に与えるように気をつけるべきです。また、あなた自身のスクリプトについても、定数宣言を含んでいるスクリプトを複数回読み込まないように気をつけてください。

最後に、実行時にそれ自身のコードを生成するためにeval()を大量に使う、分かりにくい・「圧縮」されたスクリプトもあります。Webにおいては「圧縮された」スクリプトに価値があることは認めますが、拡張機能の中で同じ事をやることにはほとんど意味がありません。拡張機能は1度だけしかダウンロードされませんから、ダウンロードにかかる時間をたった2秒だけ節約できても、誰も喜ばないでしょう。また、「圧縮」されたスクリプトはロードされ実行される度に毎回、処理に余計な時間を食うことでしょう。

エントリにつけられたコメント

1. Simonによるコメント

5番目について。関数の中のコードに手を入れる(単にコードの前後に処理を付け加えるだけでは実現不可能で、関数全体を書き換えないといけないような場合)方法として、どんなやり方なら勧められるというのでしょうか? また、ある1つの拡張機能があなたの推奨しているやり方を実行したら、関数の中身を書き換えるタイプの他のすべての拡張機能の動作が妨げられ、それらの拡張機能はすべてのコードを書き直すことを強いられるので、拡張機能同士が円満に共存できなくなるでしょう。

私は元々はあなたが勧めているようなやり方を取っていましたが、しかしeval()を使うことによってしか解決できないような非互換性の問題に何度か遭遇してきました。より良いやり方があるのなら私は喜んでそれを採用するつもりですが、残念ながら、私にはあなたが勧めているやり方が問題を解決してくれるようには見えません……

4番目について。new Function("some code")eval("some code")と比べてどのように安全なのでしょうか? bug 477380であなたはeval()を禁止することを提案していますが、new Function()も禁止しないと無意味なのではないでしょうか。

Wladimir Palantによる返信:

あなたが書いたわけではない関数の中身に対して変更を行う事は、一般的に言ってよいアイデアではありません。あなたがどんな風にそれをやったとしても、必ず酷い結果になるでしょう。他の手段(例えばObject.watch()など)であなたがやりたいことを実現できないのであれば、多分あなたはそれをするべきではないのでしょう。

new Function()について。それはeval()に比べて本質的に安全であるとは言えませんが、それはたいていの場合静的なコードのために使われ、それほど多くの問題を持ち込みません。これは、トラブルを抱え込まないようにするための良いコーディングの習慣ということです。

2. Mookによるコメント

特権が無いWebページの場合でも同じ事が言えるのでしょうか? Gecko 1.9.1はグローバルなJSONオブジェクトを実装するそうですが、evalInSandboxと同等のものはあるのでしょうか?

evalInSandboxを使うためにUniversalXPConnect権限が必要なのは困ります……

Wladimir Palantによる返信:

クロスサイトスクリプティングを防ぐことについて話しているのだと思いますが――スクリプトのためのサンドボックスは十分な対策とは言えません。これについてはbug 341604(※訳註:IEの独自拡張であるiframe要素のsecurity属性の実装についてのバグ)とWHATWGのWeb Appsの仕様のiframe要素のsandbox属性を参照してください。

5. Dorandoによるコメント

5番目について、例えば元の関数 gBrowser.foo について以下の場合を仮定しましょう(似たようなコードはMozillaのコードベースから簡単に見つけられます。例えばtabbrowser.xmlで「.isTrusted」や「permitUnload」を検索してみてください):

gBrowser.foo = function(param1, param2)
{
  if(!gBrowser.isItSaveToDoSomethingBeforeFoo())
    return;

  if (param1 == "do_nothing")
    return;

  var color = "red";
  doSomething(color, param2.length);

  if (param2 == "some_extensions_want_to_prevent_this")
    doSomethingAnnoying("dark"+color);
}
  1. 関数内でdoSomething()が実行された時に、常にdoSomethingAfterFoo()を実行するようにしたい、という時はどうすればいいのでしょうか?
  2. 関数内でdoSomething()が実行されるであろう時に、常にdoSomethingBeforeFoo()を実行するようにしたい、という時はどうすればいいのでしょうか? 元のコードのうちいくらかを複製することはできますが、それは既に修正されたセキュリティの問題をまた持ち込みかねず、関数を単に書き換えるだけにしない事のメリットを打ち消してしまいかねません。関数に対して行われる変更を常に参照し続けることは、いくつかのシチュエーションでは上手く行くでしょうが、私に言わせてもらうとそれは大変すぎます。(そしておそらくコードをいくらか遅くさせるでしょう。)
  3. gBrowser.foodoSomethingAnnoying()を実行するのを食い止めたい時はどうすればいいのでしょうか?
  4. gBrowser.fooが使う色をgreenに変えたい時はどうすればいいのでしょうか? 追加の関数を置き換えてその呼び出し元関数を確認するようにするという手もありますが、私にとってはそれは何かを壊してしまう可能性を増大させるものでしかないです。関数に本来の処理を行わせた後でその変更を後から取り消すというやり方も、可能ではありますが、良いユーザ体験をもたらすものとは思えません。

どちらの場合も、書き換えられようとしているコードが変化しないことを前提にしていますが――しかし、もしそれが起こったら?

maxVersionはそのために存在しています。

あるいは、ブラウザのその関数がセキュリティ上の問題の修正のために書き換えられたとしたら、その拡張機能は同じセキュリティ上の問題をまた持ち込んでしまうかもしれません。

isItSaveToDoSomethingBeforeFooが導入されて、doSomethingBeforeFoo();eval()でパッチを当てられた場合と同様の内容を含むようになったとしたら、(※訳註:ここで推奨されているやり方の方の)関数の置き換えでも同じ問題が起こり得ます。

その一方で、拡張機能の作者はeval()によって、(セキュリティに関するものも含めて)バグがFirefox本体側で修正されるよりも前にパッチを適用することができます。そのバグが本体側で修正された後は、書き換え対象のコードが見つからなくなるので、eval()によるパッチ適用は望ましい形で失敗する(※訳註:何の変化ももたらさず悪影響も及ぼさないということ)でしょう。

あなたが書いたわけではない関数の中身に対して変更を行う事は、一般的に言ってよいアイデアではありません。

もちろんです、それは組み込みのどんな機能の挙動を変えることについても常に言えることです。しかし拡張機能の作者にとっては、やりたいことを実現するための組み込みのインターフェースや関数が存在しない時に、それを実現するための唯一の手段がこれであるという場合もあります。

Simonが既に指摘していることの繰り返しになりますが、関数へパッチを当てるのではなく関数を丸ごと置き換えるという(※訳註:ここで推奨されている)やり方は、私が述べたようなことをやろうとしているすべての拡張機能に対してそれを完全に妨げてしまいます。ですからどうか、あなたが代替になる方法を提案できないのなら、このような手法を推奨しないでください。

あなたがどんな風にそれをやったとしても、必ず酷い結果になるでしょう。

(以下翻訳中)

今まで私がみてきた限りでは、ほとんどの最小の変更がそれを壊しうるという事実にもかかわらず、(APIの変更を含む)他の種類の変更によってコードが動かなくなるケースの方が、eval()によるパッチの適用が原因で動かなくなるケースよりも多く見られました。

Wladimir Palantによる返信:
  1. あなたはgBrowser.foowindow.doSomethingの両方を拡張する必要があります。gBrowser.fooの開始時にフラグ変数をfalseにセットして、window.doSomethingがそれをtrueにする、doSomethingAfterFooはそのフラグ変数がtrueになった時にだけ呼ばれる、という具合です。
  2. 私はあなたが、予知能力を持ったモジュールを必要としているとは思いません――あなたがやりたいのは、gBrowser.foowindow.doSomethingを呼んだ時にdoSomethingBeforeFooを呼ぶ、という事でしょう(window.doSomethingが他の場所から呼ばれた場合を除いて)。このような場合、やはり両方の関数を拡張するべきです。gBrowser.fooの開始時にフラグ変数をtrueにセットして、終了時にそれをfalseにセットする。window.doSomethingの開始時に、そのフラグ変数の値がtrueであるならdoSomethingBeforeFooを実行する。という具合です。
  3. bのやり方と同じ手法でgBrowser.fooを拡張し、doSomethingAnnoyingについてもそのフラグ変数がtrueな時は何もしないように拡張することで、可能でしょう。ただし、あなたが本当にそれをする必要があって、それが何をどのように壊すのかについては、改めてよく考えてください。
  4. bのやり方と同じ手法でgBrowser.fooを拡張し、doSomethingについてもそのフラグ変数がtrueで且つarguments[0]"red"である場合にarguments[0]"green"に置き換えるように拡張すれば、それも可能でしょう。ただしその場合も、やる前にその影響をよく考えてください。

maxVersionの仕組みについては、大抵の拡張機能は「バージョン3.0.*に対して互換性がある」という風に指定されているので、セキュリティの修正やマイナーリリースでの変更をキャッチアップしてくれません。

その一方で、拡張機能の作者はeval()によって、(セキュリティに関するものも含めて)バグがFirefox本体側で修正されるよりも前にパッチを適用することができます。――このような方法でバグにパッチを当てるのは、実に宜しくないアイデアです。Bugzillaにバグを報告して、ちゃんとしたパッチを書いてください。それが確かに明らかにバグで、安全に修正できるのであれば、その修正はマイナーリリースに取り込まれるでしょう。もしあなたが間違ったことをしていれば、フィードバックを得られるでしょう――それは、誰にも何も尋ねずに「パッチ適用」をしてブラウザの機能を壊してしまうよりも良いことです。

6. Dorandoによるコメント

doSomething(あるいは他の関数)は、 Components.classes[""].createInstance().doSomething(); という風な(※訳註:置き換え不能なネイティブのメソッドを呼ぶ)コードかもしれませんし、関数ではなくべた書きされたコードのブロックかもしれませんし、あるいは、その処理がとても頻繁に呼ばれる機能である場合は置き換え後の関数によって度し難いほどの余計な処理時間がかかるかもしれません(特に、複数の拡張機能がそういうことをするのなら)。

maxVersionの仕組みについては、大抵の拡張機能は「バージョン3.0.*に対して互換性がある」という風に指定されているので、セキュリティの修正やマイナーリリースでの変更をキャッチアップしてくれません。

これは「必ず酷い結果になる」という未来予測への反論ですよ。あなたが言っているのは実際には拡張機能の作者の過ちです。また、私に言わせれば、既存のコードを動かなくしてしまうような変更がMozillaのセキュリティアップデートで行われることがあまりに多すぎます(私が知っている最近の例はBug 442333(※訳註:eval()の第2引数の廃止を決定したバグ)です)。

このような方法でバグにパッチを当てるのは、実に宜しくないアイデアです。Bugzillaにバグを報告して、ちゃんとしたパッチを書いてください。

既にBugzillaに報告されていたり、次のメジャーリリースに修正が取り込まれることが確定していたりするなら、同じコードを使えるでしょう。そのバグが外部の(※訳註:拡張機能などの)コードに対してのみ影響を与えるものである場合は特に、(パッチが提出されていても)そのバグが実際に修正されるまでには何ヶ月もかかることがあります。

それが確かに明らかにバグで、安全に修正できるのであれば、その修正はマイナーリリースに取り込まれるでしょう。

その修正が次のマイナーリリースに取り込まれたと仮定しても、それには最低でも1ヶ月はかかります。その修正内容をバックポートすることはそれほど害を及ぼさないでしょう。

~誰にも何も尋ねずに「パッチ適用」をしてブラウザの機能を壊してしまうよりも良いことです。

もしその拡張機能の意図するところが、そのシチュエーションにおいて行われる通常の挙動を変更する事なのであれば、それは初期状態の挙動を壊してしまおうとするもののように見えるでしょう。

7. Simonのコメント

ちなみに、異なる実行コンテキストでコードを動的に実行することは、eval()のもう1つの妥当な使い方です。

eval("code to inject", context);

あるいは

domWindow.eval("code to inject");

これらは、コードを一時ファイルに書き出して、サブスクリプトローダーで読み込ませ、一時ファイルを削除する、という風なやり方によってのみ置き換えることができます。なぜなら、サブスクリプトローダーは依然としてdata: URIの読み込みを許可していないからです。(この回避策は実に面倒です。テンポラリファイルを使うことは他の問題を引き起こしかねませんから。)

Wladimir Palantによる返信:

「異なる実行コンテキスト」というのは、「特権がないコンテキスト」という意味ですか? それは別物です、そのコードはChrome権限を得ることはないでしょう。

8. Simonのコメント

「異なる実行コンテキスト」というのは、「特権がないコンテキスト」という意味ですか?

そうとは限りません。nsIWindowMediatorによって返されたDOMWindowのコンテキストは、Chrome権限のあるオブジェクトと同じになり得ます。(サブスクリプトローダーを使って読み込ませたスクリプトや、XBLのバインディングの中に書かれたスクリプトのようにwindowオブジェクトを汚染することなく動的に使われるスクリプトにおいては。)

最初の影響を見るには、以下のコードをエラーコンソールで実行してみてください:

Components.classes["@mozilla.org/appshell/window-mediator;1"]
          .getService(Components.interfaces.nsIWindowMediator)
          .getMostRecentWindow("navigator:browser")
          .eval("location");
分類:Mozilla > XUL, , , , , , , , 時刻:03:39 | Comments/Trackbacks (5) | Edit

Comments/Trackbacks

typo

あクエス -> アクセス

Commented by kou at 2009/11/28 (Sat) 05:20:33

no title

ガノタということがバレバレなtypo!

Commented by Piro at 2009/11/28 (Sat) 06:36:48

各項目に対するコメント

1 番:これは正しい。このようなjsonのパースは脆弱。
2 番:これは可読性やデバッグ容易性の問題であって、即危険というわけではない。でも従えば可読性やデバッグ容易性は向上する。
3 番:2 番と同じ。
4 番:これは逆に「なんでdoSomethingを直接コールしないの?なんでそんな面倒なことをするの?」という疑問がわく。
5 番:これはブラウザ内の関数のオーバーライドの否定ですな。それはオブジェクト指向の否定になるのでは?危険かどうかコードを読めばわかるはずだけど。これを正しいとするなら C++ や Java は非常に危険な言語となってしまう。ロジック構造を抽象化する C の STL も否定されてしまう。

 どうも eval() を悪用するケースが多いためか(そもそも本当に多いのか?)、"eval" というラベルを見たら危険を感じる、という条件反射が形成されているようにしか読めない。
 というわけで概ね Piro さんに正当性があると思えます。

Commented by mitsugu at 2010/02/08 (Mon) 09:06:46

追記

 Oracle の PL/SQL では、トリガーの結果実行されるストアド・プロシジャ内で、動的に SQL 文を生成しないとなにもできない、というケースが多いけど、それも否定されるのだろうか。
 言語が違うという反論も想定できるけど、それって単にコードの査読の手を抜きたい、という欲求を表明しているようにしか読めない。

 というわけでやはり Piro さんの論旨の方が望ましいと思うけど。

Commented by mitsugu at 2010/02/08 (Mon) 09:12:37

no title

> 4 番:これは逆に「なんでdoSomethingを直接コールしないの?なんでそんな面倒なことをするの?」という疑問がわく。

このエントリで挙げられている例だけを見ればそうなのですが、evalを使いたくなるのは、これを一般化する場合の話ですね。
Firefoxの場合、ツールバー上のボタンやメニュー項目をミドルクリックした際にその結果を新しいタブで開く、という挙動を実現するにあたって、oncommandに指定された内容を「ミドルクリックされた」という情報付きで呼び出し、実際の挙動の切り替えは呼び出されたイベントハンドラ側で行う、という実装がなされています。
http://hg.mozilla.org/mozilla-central/file/b27f05c8743b/browser/base/content/browser.xul#l341
http://hg.mozilla.org/mozilla-central/file/b27f05c8743b/browser/base/content/utilityOverlay.js#l265


>言語が違うという反論も想定できるけど、それって単にコードの査読の手を抜きたい、という欲求を表明しているようにしか読めない。

アドオンの数が膨大になってきて、システムが破綻しているのだと思います。
これは、「マニフェストに使用APIを記述し、マニフェストで宣言された範囲外のことはできなくする、そうすればレビュー担当者はレビューの手間を軽減できる」「レビューが早く通るように、コードを書く人間はきちんとマニフェストを書いた方が得になるという圧力が働く」というような仕組みをJetpackに入れる必要があって、rawのようなむき出しの機能は載せるべきではない、という話にも繋がります。
http://piro.sakura.ne.jp/latest/blosxom/mozilla/extension/2010-01-26_jetpack.htm

ただ、そういうことができるのはJetpackでそういう仕組みが入ってからの話であって、それまでは嫌でも現状の仕組みを前提にしなければならない(バックドア等なんでもありな事を前提に、きちんとコードレビューする必要がある)のですよね。
自分も含めてエディタ権限を持った人間が、いいかげんなレビューをしていい理由にはなりません。

Commented by Piro at 2010/02/08 (Mon) 10:10:13

TrackBack ping me at


の末尾に2020年11月30日時点の日本の首相のファミリーネーム(ローマ字で回答)を繋げて下さい。例えば「noda」なら、「2009-11-28_five-wrong-reasons-to-use-eval-in-an-extension.trackbacknoda」です。これは機械的なトラックバックスパムを防止するための措置です。

Post a comment

writeback message: Ready to post a comment.

2020年11月30日時点の日本の首相のファミリーネーム(ひらがなで回答)

Powered by blosxom 2.0 + starter kit
Home

カテゴリ一覧

過去の記事

1999.2~2005.8

最近のコメント

最近のつぶやき