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 107766 f0e182ab06a9a0aa65825544ac44135e9e67e42a
parent 107765 ea2e059c896b86d863e318e62311c2ff2d617346
child 107767 2c9976725a5779075efb21a087ccc644eac0684c
push id1708
push userakeybl@mozilla.com
push dateMon, 19 Nov 2012 21:10:21 +0000
treeherdermozilla-esr52@2704e441363f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs748309
milestone18.0a1
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