Don't copy object directly if internal properties are set

lib/Runtime/Library/JavascriptArray.cpp

@@ -12279,15 +12279,23 @@ Case0:
         JS_REENTRANCY_LOCK(jsReentLock, scriptContext->GetThreadContext());
         SETOBJECT_FOR_MUTATION(jsReentLock, originalArray);
 
-        if (JavascriptArray::IsNonES5Array(originalArray)
-            && !UnsafeVarTo<DynamicObject>(originalArray)->GetDynamicType()->GetTypeHandler()->GetIsNotPathTypeHandlerOrHasUserDefinedCtor()
-            && UnsafeVarTo<DynamicObject>(originalArray)->GetPrototype() == scriptContext->GetLibrary()->GetArrayPrototype()
-            && !scriptContext->GetLibrary()->GetArrayObjectHasUserDefinedSpecies())
+        auto* library = scriptContext->GetLibrary();
+
+        if (JavascriptArray::IsNonES5Array(originalArray))
         {
-            return nullptr;
+            auto* dynamicObject = UnsafeVarTo<DynamicObject>(originalArray);
+            auto* typeHandler = dynamicObject->GetDynamicType()->GetTypeHandler();
+
+            if (typeHandler->IsPathTypeHandler()
+                && !PathTypeHandlerBase::FromTypeHandler(typeHandler)->HasUserDefinedCtor()
+                && dynamicObject->GetPrototype() == library->GetArrayPrototype()
+                && !library->GetArrayObjectHasUserDefinedSpecies())
+            {
+                return nullptr;
+            }
         }
 
-        Var constructor = scriptContext->GetLibrary()->GetUndefined();
+        Var constructor = library->GetUndefined();
 
         JS_REENTRANT(jsReentLock, BOOL isArray = JavascriptOperators::IsArray(originalArray));
         if (isArray)
@@ -12305,7 +12313,7 @@ Case0:
                 {
                     if (constructorScriptContext->GetLibrary()->GetArrayConstructor() == constructor)
                     {
-                        constructor = scriptContext->GetLibrary()->GetUndefined();
+                        constructor = library->GetUndefined();
                     }
                 }
             }
@@ -12321,14 +12329,14 @@ Case0:
                     }
                     return nullptr;
                 }
-                if (constructor == scriptContext->GetLibrary()->GetNull())
+                if (constructor == library->GetNull())
                 {
-                    constructor = scriptContext->GetLibrary()->GetUndefined();
+                    constructor = library->GetUndefined();
                 }
             }
         }
 
-        if (constructor == scriptContext->GetLibrary()->GetUndefined() || constructor == scriptContext->GetLibrary()->GetArrayConstructor())
+        if (constructor == library->GetUndefined() || constructor == library->GetArrayConstructor())
         {
             if (length > UINT_MAX)
             {
@@ -12337,7 +12345,7 @@ Case0:
 
             if (nullptr == pIsIntArray)
             {
-                return scriptContext->GetLibrary()->CreateArray(static_cast<uint32>(length));
+                return library->CreateArray(static_cast<uint32>(length));
             }
             else
             {

lib/Runtime/Types/DynamicObject.cpp

@@ -866,11 +866,11 @@ namespace Js
             }
             return false;
         }
-        if (!from->GetTypeHandler()->IsPathTypeHandler())
+        if (!from->GetTypeHandler()->IsObjectCopyable())
         {
             if (PHASE_TRACE1(ObjectCopyPhase))
             {
-                Output::Print(_u("ObjectCopy: Can't copy: Don't have PathTypeHandler\n"));
+                Output::Print(_u("ObjectCopy: Can't copy: from obj does not have copyable type handler\n"));
             }
             return false;
         }
@@ -890,14 +890,6 @@ namespace Js
             }
             return false;
         }
-        if (PathTypeHandlerBase::FromTypeHandler(from->GetTypeHandler())->HasAccessors())
-        {
-            if (PHASE_TRACE1(ObjectCopyPhase))
-            {
-                Output::Print(_u("ObjectCopy: Can't copy: type handler has accessors\n"));
-            }
-            return false;
-        }
         if (this->GetPrototype() != from->GetPrototype())
         {
             if (PHASE_TRACE1(ObjectCopyPhase))
@@ -906,14 +898,6 @@ namespace Js
             }
             return false;
         }
-        if (!from->GetTypeHandler()->AllPropertiesAreEnumerable())
-        {
-            if (PHASE_TRACE1(ObjectCopyPhase))
-            {
-                Output::Print(_u("ObjectCopy: Can't copy: from obj has non-enumerable properties\n"));
-            }
-            return false;
-        }
         if (from->IsExternal())
         {
             if (PHASE_TRACE1(ObjectCopyPhase))

lib/Runtime/Types/PathTypeHandler.cpp

@@ -235,12 +235,20 @@ namespace Js
         DynamicTypeHandler(slotCapacity, inlineSlotCapacity, offsetOfInlineSlots, DefaultFlags | (isLocked ? IsLockedFlag : 0) | (isShared ? (MayBecomeSharedFlag | IsSharedFlag) : 0)),
         typePath(typePath),
         predecessorType(predecessorType),
-        successorInfo(nullptr)
+        successorInfo(nullptr),
+        hasUserDefinedCtor(false),
+        hasInternalProperty(false)
     {
         Assert(pathLength <= slotCapacity);
         Assert(inlineSlotCapacity <= slotCapacity);
         SetUnusedBytesValue(pathLength);
-        isNotPathTypeHandlerOrHasUserDefinedCtor = predecessorType == nullptr ? false : predecessorType->GetTypeHandler()->GetIsNotPathTypeHandlerOrHasUserDefinedCtor();
+
+        if (predecessorType != nullptr && predecessorType->GetTypeHandler()->IsPathTypeHandler())
+        {
+            auto* handler = PathTypeHandlerBase::FromTypeHandler(predecessorType->GetTypeHandler());
+            hasUserDefinedCtor = handler->hasUserDefinedCtor;
+            hasInternalProperty = handler->hasInternalProperty;
+        }
     }
 
     int PathTypeHandlerBase::GetPropertyCount()
@@ -2166,7 +2174,12 @@ namespace Js
 
             if (key.GetPropertyId() == PropertyIds::constructor)
             {
-                nextPath->isNotPathTypeHandlerOrHasUserDefinedCtor = true;
+                nextPath->hasUserDefinedCtor = true;
+            }
+
+            if (IsInternalPropertyId(key.GetPropertyId()))
+            {
+                nextPath->hasInternalProperty = true;
             }
 
 #ifdef PROFILE_TYPES

lib/Runtime/Types/PathTypeHandler.h

@@ -102,6 +102,8 @@ namespace Js
         Field(DynamicType*) predecessorType; // Strong reference to predecessor type so that predecessor types remain in the cache even though they might not be used
         Field(TypePath*) typePath;
         Field(PathTypeSuccessorInfo*) successorInfo;
+        Field(bool) hasUserDefinedCtor;
+        Field(bool) hasInternalProperty;
 
     public:
         DEFINE_GETCPPNAME();
@@ -119,6 +121,8 @@ namespace Js
             return nullptr;
         }
 
+        bool HasUserDefinedCtor() { return this->hasUserDefinedCtor; }
+
         virtual BOOL IsLockable() const override { return true; }
         virtual BOOL IsSharable() const override { return true; }
 
@@ -459,6 +463,8 @@ namespace Js
         DEFINE_VTABLE_CTOR_NO_REGISTER(PathTypeHandlerNoAttr, PathTypeHandlerBase);
 
     public:
+        virtual bool IsObjectCopyable() const override { return !this->hasInternalProperty; }
+
         static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, TypePath* typePath, uint16 pathLength, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots, bool isLocked = false, bool isShared = false, DynamicType* predecessorType = nullptr);
         static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, TypePath* typePath, uint16 pathLength, const PropertyIndex slotCapacity, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots, bool isLocked = false, bool isShared = false, DynamicType* predecessorType = nullptr);
         static PathTypeHandlerNoAttr * New(ScriptContext * scriptContext, PathTypeHandlerNoAttr * typeHandler, bool isLocked, bool isShared);
@@ -534,6 +540,7 @@ namespace Js
             return FindNextPropertyHelper(scriptContext, this->attributes, index, propertyString, propertyId, attributes, type, typeToEnumerate, flags, instance, info);
         }
         virtual BOOL AllPropertiesAreEnumerable() sealed override { return false; }
+        virtual bool IsObjectCopyable() const override { return false; }
 #if ENABLE_NATIVE_CODEGEN
         virtual bool IsObjTypeSpecEquivalent(const Type* type, const TypeEquivalenceRecord& record, uint& failedPropertyIndex) override;
         virtual bool IsObjTypeSpecEquivalent(const Type* type, const EquivalentPropertyEntry* entry) override;

lib/Runtime/Types/TypeHandler.cpp

@@ -77,7 +77,6 @@ using namespace Js;
                 ? RoundUpObjectHeaderInlinedInlineSlotCapacity(inlineSlotCapacity)
                 : RoundUpInlineSlotCapacity(inlineSlotCapacity);
         this->slotCapacity = RoundUpSlotCapacity(slotCapacity, inlineSlotCapacity);
-        this->isNotPathTypeHandlerOrHasUserDefinedCtor = true;
 
         Assert(IsObjectHeaderInlinedTypeHandler() == IsObjectHeaderInlined(offsetOfInlineSlots));
     }
@@ -884,6 +883,5 @@ using namespace Js;
         Output::Print(_u("%*sslotCapacity: %d\n"), fieldIndent, padding, this->slotCapacity);
         Output::Print(_u("%*sunusedBytes: %u\n"), fieldIndent, padding, this->unusedBytes);
         Output::Print(_u("%*sinlineSlotCapacty: %u\n"), fieldIndent, padding, this->inlineSlotCapacity);
-        Output::Print(_u("%*sisNotPathTypeHandlerOrHasUserDefinedCtor: %d\n"), fieldIndent, padding, static_cast<int>(this->isNotPathTypeHandlerOrHasUserDefinedCtor));
     }
 #endif

lib/Runtime/Types/TypeHandler.h

@@ -48,7 +48,6 @@ namespace Js
         Field(int) slotCapacity;
         Field(uint16) unusedBytes;             // This always has it's lowest bit set to avoid false references
         Field(uint16) inlineSlotCapacity;
-        Field(bool) isNotPathTypeHandlerOrHasUserDefinedCtor;
         Field(bool) protoCachesWereInvalidated;
 
     public:
@@ -58,7 +57,6 @@ namespace Js
             propertyTypes(typeHandler->propertyTypes),
             slotCapacity(typeHandler->slotCapacity),
             offsetOfInlineSlots(typeHandler->offsetOfInlineSlots),
-            isNotPathTypeHandlerOrHasUserDefinedCtor(typeHandler->isNotPathTypeHandlerOrHasUserDefinedCtor),
             unusedBytes(typeHandler->unusedBytes),
             protoCachesWereInvalidated(false),
             inlineSlotCapacity(typeHandler->inlineSlotCapacity)
@@ -427,7 +425,6 @@ namespace Js
         }
 
         BOOL Freeze(DynamicObject *instance, bool isConvertedType = false)  { return FreezeImpl(instance, isConvertedType); }
-        bool GetIsNotPathTypeHandlerOrHasUserDefinedCtor() const { return this->isNotPathTypeHandlerOrHasUserDefinedCtor; }
 
         virtual BOOL IsStringTypeHandler() const { return false; }
 
@@ -560,6 +557,8 @@ namespace Js
         virtual BOOL IsSimpleDictionaryTypeHandler() const {return FALSE; }
         virtual BOOL IsDictionaryTypeHandler() const {return FALSE;}
 
+        virtual bool IsObjectCopyable() const { return false; }
+
         static bool IsolatePrototypes() { return CONFIG_FLAG(IsolatePrototypes); }
         static bool ChangeTypeOnProto() { return CONFIG_FLAG(ChangeTypeOnProto); }
         static bool ShouldFixMethodProperties() { return !PHASE_OFF1(FixMethodPropsPhase); }

test/Object/assign.baseline

@@ -1,11 +1,11 @@
 ObjectCopy succeeded
-ObjectCopy: Can't copy: Don't have PathTypeHandler
-ObjectCopy: Can't copy: type handler has accessors
-ObjectCopy: Can't copy: type handler has accessors
+ObjectCopy: Can't copy: from obj does not have copyable type handler
+ObjectCopy: Can't copy: from obj does not have copyable type handler
+ObjectCopy: Can't copy: from obj does not have copyable type handler
 ObjectCopy: Can't copy: Prototypes don't match
-ObjectCopy: Can't copy: from obj has non-enumerable properties
+ObjectCopy: Can't copy: from obj does not have copyable type handler
 ObjectCopy succeeded
 ObjectCopy succeeded
 ObjectCopy: Can't copy: to obj has object array
-ObjectCopy: Can't copy: Don't have PathTypeHandler
+ObjectCopy: Can't copy: from obj does not have copyable type handler
 pass

test/es6/weakmap_functionality.js

@@ -501,6 +501,28 @@ var tests = [
         }
     },
 
+    {
+        name: "WeakMap internal property data is not copied by Object.assign",
+        body: function () {
+            var key1 = {};
+            var key2 = {};
+            var map = new WeakMap(); 
+
+            map.set(key1, 1);
+            map.delete(Object.assign(key2, key1));
+            assert.isFalse(map.has(key2));
+
+            key1 = {};
+            key2 = {};
+            map = new WeakMap(); 
+
+            map.set(key1, 1);
+            key1.a = 1;
+            map.delete(Object.assign(key2, key1));
+            assert.isFalse(map.has(key2));
+        }
+    }
+
 ];
 
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });