Skip to main content
added 46 characters in body
Source Link
Blindman67
  • 22.3k
  • 2
  • 16
  • 40

Normally this would make the question "off topic", but unless you are experienced with JS this type of bug is very often overlooked or even considered in normal testing regimes.

Normally this would make the question "off topic", but unless you are experienced with JS this type of bug is very often overlooked.

Normally this would make the question "off topic", but unless you are experienced with JS this type of bug is very often overlooked or even considered in normal testing regimes.

Source Link
Blindman67
  • 22.3k
  • 2
  • 16
  • 40

Serious BUG!

Normally this would make the question "off topic", but unless you are experienced with JS this type of bug is very often overlooked.

The bug is very dangerous, that will either throw a call stack overflow or timeout the page.

You do not check created proxy objects for cyclic references.

As proxies are slow and depending on the structure of the proxy the page will often timeout before the call stack overflows.

Demo the bug

For example your testing code can be changed to

  data.a.b = {c: true};      // adds new proxy
  data.a.b.c = data.a;       // cyclic proxy reference
  data = unbuildProxy(data); // call stack overflow (or page timeout)

The call chain is not straight forward as its the unbuildProxy call that starts the infinite call chain.

See snippet below for example.

Fix

Most common solution is to use a WeakSet (only keeps the hash of objects) to track objects you create a proxy for. If the object exists in the map don't build a new proxy for it.

Another solution is to count the cycle depth and throw an error if it gets too deep. (Note at all ideal)

Example of bug

The snippet shows the bug by throwing an error when the call depth to buildProxy is > 50.

Open the console [hit F12] to see the trace when the snippet throws the error.

Note I made some minor mods (before I noticed the bug). Changes do not change codes behaviour (Apart from (fixed) unguarded for in loop in original code).

function isObject(value) {
  return value !== null && typeof value === "object" && ["Array", "Object"].includes(value.constructor.name);
}
function buildProxy(poj, callback = console.log, tree = [], count = 0) {
  if (count > 50) { throw new Error("Cycle depth too deep: " + count)      }
  const getPath = (prop) => tree.concat(prop).join(".");
  return new Proxy(poj, {
    get(target, prop) {
      const value = Reflect.get(target, prop);
      if (isObject(value)) { return buildProxy(value, callback, tree.concat(prop), count + 1); }
      return value;
    },
    set(target, prop, value) {
      callback({
        action: "set",
        path: getPath(prop),
        newValue: value,
        preValue: Reflect.get(target, prop, value),
        stack: new Error().stack.split("\n")[1].trim()
      });
      return Reflect.set(target, prop, value);
    },
    deleteProperty(target, prop) {
      callback({
        action: "delete",
        path: getPath(prop),
        stack: new Error().stack.split("\n")[1].trim()
      });
      return Reflect.deleteProperty(target, prop);
    },
  });
}
const unbuildProxy = (value) => {
  if (isObject(value)) {
    value = Array.isArray(value)? [...value] : {...value};

    // Do not use unguarded for in loops. BM67 Replaced with for of loop
    for (const key of Object.keys(value)) { value[key] = unbuildProxy(value[key]) }
  }
  return value;
}

var data = buildProxy({a: {b: true}});
data.a.b = {c: true};
data.a.b.c = data.a;
data = unbuildProxy(data);