🥴

オブジェクトを連想配列として使うと痛い目にあいますよ

2021/03/24に公開
9

2億回くらい口をすっぱくして言ったような気がします。(嘘)

というくらいに過去に何度も語られて語りつくされた話題なので、雑にまとめておきます。

JavaScript でオブジェクトを連想配列として使っちゃだめです。
だめ。全然だめ。絶対だめ。

こういう、『なんか工夫して使ったら使えちゃったから便利。』だけどあとからひどい目にあうという変な構文やテクニックがやたらおおくてカオスなのがJavaScriptの難しさです。気をつけましょう。

const inputValue = 'a';

const object1 = {
  a: 'Aです',
  b: 'Bです',
  c: 'Cです',
}

const isUndefined = value => typeof value === 'undefined';

if (isUndefined(object1[inputValue])) {
  console.log('処理がみつかりません');
} else {
  console.log(object1[inputValue]);
}

これで、inputValueがa,b,c,d など【さまざまな値】であっても、出力結果は次のどれか【のみ】なる。そのようにプログラムは読めますよね。

  Aです / Bです / Cです / 処理がみつかりません

ちがいます。全然ちがいます。絶対ちがいます。

inputValue に、'toString' や、 'constructor' を代入して調べてみてください。

  function toString() { [native code] }

とか

  function Object() { [native code] }

と出力されます。

object1 は、自分で定義してなくても、constructor や toString というプロパティを最初から持っているのです。

なので、このようなオブジェクトを連想配列扱いするようなプロパティに対する文字アクセスで分岐処理をするような使い方はしてはいけません。

そんな使い方は想定していないから大丈夫。

大丈夫なわけねーだろ。

という実例を教えていただいたので引用させていただきます。

https://twitter.com/kymn_/status/1297171791962546178

「以下では、バグの原因について報告します。 パフォーマンスをを計算する部分のコードについて、以下のようなコードがありました。(単純化しています。) rate = rates[user] ? rates[user] : default; ratesは内部レートを格納するJSONをパースしたObject型、userは参加者のIDです。」

ratesには「一度以上参加した参加者のレート」が格納されています。そのため、userが初参加ならばrates[user]は偽として評価され、rateにはdefaultが格納されるはずでした。
しかし、JavaScriptのオブジェクトはデフォルトで複数のメンバを持っています。例えば、"toString"や"constructor"です。

ここで、このような名前を持つユーザーが参加した場合を考えます。
当然rates["constructor"]はundefinedではないため、真として評価されます。結果として、rateに数値でない値が入ってしまったためにその後の計算でNaNが発生し、連鎖的に様々なところが破壊されました。
これが今回起きたことです。

こわっ、、、こわすぎる。。。

追記

t12uさんから良いコメントを頂きましたのでコメント欄も読まれることをおすすめします。

キーと値のペアなデータ構造としてオブジェクトを使わないように、という事ではなく、純粋な連想配列だと思っているとそうではないのでトラブルの原因になる場合があるので、使い方に気をつけていきましょう。

より安全なプログラムを目指してがんばっていきましょう!

Discussion

t12ut12u

JavaScript の擁護をすると笑、Object.prototype.hasOwnProperty でオブジェクト自身が持つプロパティかどうかを検証できるので、オブジェクトを安全に連想配列として使うことができます。

const object1 = {
  a: 'Aです',
  b: 'Bです',
  c: 'Cです',
}

Object.prototype.hasOwnProperty.call(object1, 'a') // true
Object.prototype.hasOwnProperty.call(object1, 'toString') // false
standard softwarestandard software

オブジェクトを扱うときは必須ですよね。hasOwnProperty。

ただ、constructorとかtoStringという文字列を扱えない時点で連想配列とは呼べないと思います。

例にあげた、ユーザー名にconstructorという文字列を使った時の不具合を防止できるものでもないでしょうし、

また、IT用語辞典サイトなんて作ってみて、constructorだけは登録できません、とか、あるいは登録はできてDBに書き込めたけど、それが読込時に落ちてサイトが動かないとかなると、アジャパーとなります。

hasOwnPropertyで防御するくらいならMap使いましょう、というか、Mapってそういう用途にも使える用に作られたんだと思います。

他には愚直に

const array1 = [
  [文字,],
  [文字,],
  [文字,],
]

こんなデータ構造を自作しても文字列をキーにした連想配列は作れるのでオブジェクト連想配列として使っちゃいかんかなと。🥴

t12ut12u

constructorとかtoStringという文字列を扱えない時点で連想配列とは呼べない

constructor や toString はオブジェクト自身ではなく Object.prototype のプロパティなので、key が constructor や toString であっても以下のように使えますが、そういう意味ではありませんか?
この記事で提起されている問題であればこれで十分対応可能だと思います。

const o1 = {
  constructor: "hey",
  toString: "yo"
}

Object.prototype.hasOwnProperty.call(o1, "constructor") // true
Object.prototype.hasOwnProperty.call(o1, "toString") // true

const o2 = {}

Object.prototype.hasOwnProperty.call(o2, "constructor") // false
Object.prototype.hasOwnProperty.call(o2, "toString") // false

Map を使うべきというのは全く同意です。

追記:もしかしたら in 演算子の挙動と勘違いされているかもしれません。
in 演算子はプロトタイプチェーンをさかのぼってプロパティをチェックします。

"toString" in {} // true
standard softwarestandard software

なるほどー!!!上書きすると、そんな動きするんですね。知らなかったです。
すいません、理解が不足していました。
理解できました。ありがとうございます。

これもうまく動きますね。

const o2 = {}
o2.constructor = 'test'
o2.toString = 'test'

console.log(
  Object.prototype.hasOwnProperty.call(o2, "constructor") // true
)
console.log(
  Object.prototype.hasOwnProperty.call(o2, "toString") // true
)

toStringをウワがいてしまうのは、MDNのObject.create(null)したら、デバッグで困ったりするよ問題(下記リンク)にぶつかりそうなので、こわいもんですが、勉強になりました。ありがとうございます。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Object/create

constructorとかtoStringとか、どんな文字列がアウトなのか疑わしくていろいろドキドキしそうですが、hasOwnでチェックすればまあ、なんとかなるんですね。


in についてありがとうございます。in は理解していたのですが、constructor や toString を上書きできるということがすっかり抜けていました。

t12ut12u

よかったです!

実際、どんな文字列がアウトなのか疑わしくていろいろ怖い

Object.prototype に存在するようなプロパティは暗黙に呼び出されることがあるので混乱を呼ぶという話ですね。

これはプロトタイプチェーン上に存在するものでなければ問題ないと思います。
プロトタイプチェーン上のプロパティを取得する関数を書いてみたので、使ってみてください。

const getAllProperties = (o = Object.create(null), result = []) => {
  const prototype = Object.getPrototypeOf(o)
  if (!prototype) {
    return result
  }
  const keys = Object.getOwnPropertyNames(prototype)
  return getAllProperties(prototype, [...result, ...keys])
}

console.log(getAllProperties([]))
`*
["length", "constructor", "concat", "copyWithin", "fill", "find", "findIndex", "lastIndexOf", "pop", "push", "reverse", "shift", "unshift", "slice", "sort", "splice", "includes", "indexOf", "join", "keys", "entries", "values", "forEach", "filter", "flat", "flatMap", "map", "every", "some", "reduce", "reduceRight", "toLocaleString", "toString", "constructor", "__defineGetter__", "__defineSetter__", "hasOwnProperty", "__lookupGetter__", "__lookupSetter__", "isPrototypeOf", "propertyIsEnumerable", "toString", "valueOf", "__proto__", "toLocaleString"]
*`
standard softwarestandard software

以前の記事ですが、
t12uさんのコードとほぼ同じだと思いますが

下記の記事でコードを書いたことがありました。

[JavaScript] Class の継承元をたどって property を全て列挙する - Qiita
https://qiita.com/standard-software/items/e907563d10c43025b18a

t12uさんに改めて感謝しております。
クラスメソッドの列挙属性の有無とか、知らなかったです。

michiharumichiharu

名前空間がクリーンではないリスクに鈍感なエンジニア、いますよね。

古参メンバーが構築したオレオレAPIフレームワークが使われているプロジェクトに参画しました。
そのフレームワークはレスポンスBodyに"status"が含められません。(含めてもステータスコード200とかで上書きされてしまう。)もちろん、名前空間が汚染されていることに気がつけるヒントはフレームワークの実装以外にありません。

同じエンジニアが最近axiosinterceptorsに、名前空間を汚染するミドルウェア的なものを実装し、リクエストBodyにも含められない名前がある環境になってしまいました。

「〇〇という名前は使っちゃダメ」という脳内メモリーが解放できないと苦しいですね。

standard softwarestandard software

他の人が構築したものを引き継ぐのって、めちゃくちゃ難しいですよね。
プログラムは書くより読む方が難しいとは10年も前から言われていることですが、これを知り、読みやすく書ける人のなんと少ないことかと思ったりします。

エラーハンドリングとか、API側の都合とかを含めていろいろやらなければいけないので、上手ではない人の共通化コードを紐解くのは、すごい足ひっぱられるなー、と思うことは過去いろいろありましたが、

最近は、そういうのを紐解いてリファクタリングしたり、
過去コードはそのままにして、似た機能の別の新しい共通化関数を作ったりとか、
そういう所が自分の腕の見せどころかなー、と思うようにしています。