Bug 748309 - Structured clone should check for duplicates of all objects. r=jorendorff
authorSteve Fink <sfink@mozilla.com>
Tue, 04 Sep 2012 14:09:48 -0700
changeset 107768 f0e182ab06a9a0aa65825544ac44135e9e67e42a
parent 107767 ea2e059c896b86d863e318e62311c2ff2d617346
child 107769 2c9976725a5779075efb21a087ccc644eac0684c
push id23509
push userryanvm@gmail.com
push dateSat, 22 Sep 2012 12:28:38 +0000
treeherdermozilla-central@b461a7cd250e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs748309
milestone18.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 748309 - Structured clone should check for duplicates of all objects. r=jorendorff
js/src/jsclone.cpp
js/src/jsclone.h
--- a/js/src/jsclone.cpp
+++ b/js/src/jsclone.cpp
@@ -444,33 +444,37 @@ bool
 JSStructuredCloneWriter::writeArrayBuffer(JSHandleObject obj)
 {
     ArrayBufferObject &buffer = obj->asArrayBuffer();
     return out.writePair(SCTAG_ARRAY_BUFFER_OBJECT, buffer.byteLength()) &&
            out.writeBytes(buffer.dataPointer(), buffer.byteLength());
 }
 
 bool
-JSStructuredCloneWriter::startObject(JSHandleObject obj)
+JSStructuredCloneWriter::startObject(JSHandleObject obj, bool *backref)
 {
-    JS_ASSERT(obj->isArray() || obj->isObject());
-
     /* Handle cycles in the object graph. */
     CloneMemory::AddPtr p = memory.lookupForAdd(obj);
-    if (p)
+    if ((*backref = p))
         return out.writePair(SCTAG_BACK_REFERENCE_OBJECT, p->value);
     if (!memory.add(p, obj, memory.count()))
         return false;
 
     if (memory.count() == UINT32_MAX) {
         JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL,
                              JSMSG_NEED_DIET, "object graph to serialize");
         return false;
     }
 
+    return true;
+}
+
+bool
+JSStructuredCloneWriter::traverseObject(JSHandleObject obj)
+{
     /*
      * Get enumerable property ids and put them in reverse order so that they
      * will come off the stack in forward order.
      */
     size_t initialLength = ids.length();
     if (!GetPropertyNames(context(), obj, JSITER_OWNONLY, &ids))
         return false;
     jsid *begin = ids.begin() + initialLength, *end = ids.end();
@@ -506,29 +510,36 @@ JSStructuredCloneWriter::startWrite(cons
 
         // The object might be a security wrapper. See if we can clone what's
         // behind it. If we can, unwrap the object.
         obj = UnwrapObjectChecked(context(), obj);
         if (!obj)
             return false;
 
         AutoCompartment ac(context(), obj);
+
+        bool backref;
+        if (!startObject(obj, &backref))
+            return false;
+        if (backref)
+            return true;
+
         if (obj->isRegExp()) {
             RegExpObject &reobj = obj->asRegExp();
             return out.writePair(SCTAG_REGEXP_OBJECT, reobj.getFlags()) &&
                    writeString(SCTAG_STRING, reobj.getSource());
         } else if (obj->isDate()) {
             double d = js_DateGetMsecSinceEpoch(context(), obj);
             return out.writePair(SCTAG_DATE_OBJECT, 0) && out.writeDouble(d);
-        } else if (obj->isObject() || obj->isArray()) {
-            return startObject(obj);
         } else if (obj->isTypedArray()) {
             return writeTypedArray(obj);
         } else if (obj->isArrayBuffer() && obj->asArrayBuffer().hasData()) {
             return writeArrayBuffer(obj);
+        } else if (obj->isObject() || obj->isArray()) {
+            return traverseObject(obj);
         } else if (obj->isBoolean()) {
             return out.writePair(SCTAG_BOOLEAN_OBJECT, obj->asBoolean().unbox());
         } else if (obj->isNumber()) {
             return out.writePair(SCTAG_NUMBER_OBJECT, 0) &&
                    out.writeDouble(obj->asNumber().unbox());
         } else if (obj->isString()) {
             return writeString(SCTAG_STRING_OBJECT, obj->asString().unbox());
         }
@@ -816,59 +827,68 @@ JSStructuredCloneReader::startRead(Value
         break;
       }
 
       case SCTAG_ARRAY_OBJECT:
       case SCTAG_OBJECT_OBJECT: {
         JSObject *obj = (tag == SCTAG_ARRAY_OBJECT)
                         ? NewDenseEmptyArray(context())
                         : NewBuiltinClassInstance(context(), &ObjectClass);
-        if (!obj || !objs.append(ObjectValue(*obj)) ||
-            !allObjs.append(ObjectValue(*obj)))
+        if (!obj || !objs.append(ObjectValue(*obj)))
             return false;
         vp->setObject(*obj);
         break;
       }
 
       case SCTAG_BACK_REFERENCE_OBJECT: {
         if (data >= allObjs.length()) {
             JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL,
                                  JSMSG_SC_BAD_SERIALIZED_DATA,
-                                 "invalid input");
+                                 "invalid back reference in input");
+            return false;
         }
         *vp = allObjs[data];
-        break;
+        return true;
       }
 
       case SCTAG_ARRAY_BUFFER_OBJECT:
-        return readArrayBuffer(data, vp);
+        if (!readArrayBuffer(data, vp))
+            return false;
+        break;
 
       default: {
         if (tag <= SCTAG_FLOAT_MAX) {
             double d = ReinterpretPairAsDouble(tag, data);
             if (!checkDouble(d))
                 return false;
             vp->setNumber(d);
             break;
         }
 
-        if (SCTAG_TYPED_ARRAY_MIN <= tag && tag <= SCTAG_TYPED_ARRAY_MAX)
-            return readTypedArray(tag, data, vp);
+        if (SCTAG_TYPED_ARRAY_MIN <= tag && tag <= SCTAG_TYPED_ARRAY_MAX) {
+            if (!readTypedArray(tag, data, vp))
+                return false;
+            break;
+        }
 
         if (!callbacks || !callbacks->read) {
             JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL, JSMSG_SC_BAD_SERIALIZED_DATA,
                                  "unsupported type");
             return false;
         }
         JSObject *obj = callbacks->read(context(), this, tag, data, closure);
         if (!obj)
             return false;
         vp->setObject(*obj);
       }
     }
+
+    if (vp->isObject() && !allObjs.append(*vp))
+        return false;
+
     return true;
 }
 
 bool
 JSStructuredCloneReader::readId(jsid *idp)
 {
     uint32_t tag, data;
     if (!in.readPair(&tag, &data))
--- a/js/src/jsclone.h
+++ b/js/src/jsclone.h
@@ -129,18 +129,19 @@ struct JSStructuredCloneWriter {
 
   private:
     JSContext *context() { return out.context(); }
 
     bool writeString(uint32_t tag, JSString *str);
     bool writeId(jsid id);
     bool writeArrayBuffer(JSHandleObject obj);
     bool writeTypedArray(JSHandleObject obj);
-    bool startObject(JSHandleObject obj);
+    bool startObject(JSHandleObject obj, bool *backref);
     bool startWrite(const js::Value &v);
+    bool traverseObject(JSHandleObject obj);
 
     inline void checkStack();
 
     js::SCOutput &out;
 
     // Vector of objects with properties remaining to be written.
     //
     // NB: These can span multiple compartments, so the compartment must be