Skip to content

Commit

Permalink
Data:Event:Manipulation: Prevent collisions with Object.prototype
Browse files Browse the repository at this point in the history
Make sure events & data keys matching Object.prototype properties work.
A separate fix for such events on cloned elements was added as well.

Fixes gh-3256
Closes gh-4603
  • Loading branch information
mgol committed Mar 2, 2020
1 parent 358b769 commit 9d76c0b
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/data/Data.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Data.prototype = {

// If not, create one
if ( !value ) {
value = {};
value = Object.create( null );

// We can accept data for non-element nodes in modern browsers,
// but we should not, see #8335.
Expand Down
6 changes: 4 additions & 2 deletions src/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ jQuery.event = {

// Init the element's event structure and main handler, if this is the first
if ( !( events = elemData.events ) ) {
events = elemData.events = {};
events = elemData.events = Object.create( null );
}
if ( !( eventHandle = elemData.handle ) ) {
eventHandle = elemData.handle = function( e ) {
Expand Down Expand Up @@ -294,7 +294,9 @@ jQuery.event = {
// Make a writable jQuery.Event from the native event object
event = jQuery.event.fix( nativeEvent ),

handlers = ( dataPriv.get( this, "events" ) || {} )[ event.type ] || [],
handlers = (
dataPriv.get( this, "events" ) || Object.create( null )
)[ event.type ] || [],
special = jQuery.event.special[ event.type ] || {};

// Use the fix-ed jQuery.Event rather than the (read-only) native event
Expand Down
4 changes: 3 additions & 1 deletion src/event/trigger.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ jQuery.extend( jQuery.event, {
special.bindType || type;

// jQuery handler
handle = ( dataPriv.get( cur, "events" ) || {} )[ event.type ] &&
handle = (
dataPriv.get( cur, "events" ) || Object.create( null )
)[ event.type ] &&
dataPriv.get( cur, "handle" );
if ( handle ) {
handle.apply( cur, data );
Expand Down
8 changes: 3 additions & 5 deletions src/manipulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,19 @@ function restoreScript( elem ) {
}

function cloneCopyEvent( src, dest ) {
var i, l, type, pdataOld, pdataCur, udataOld, udataCur, events;
var i, l, type, pdataOld, udataOld, udataCur, events;

if ( dest.nodeType !== 1 ) {
return;
}

// 1. Copy private data: events, handlers, etc.
if ( dataPriv.hasData( src ) ) {
pdataOld = dataPriv.access( src );
pdataCur = dataPriv.set( dest, pdataOld );
pdataOld = dataPriv.get( src );
events = pdataOld.events;

if ( events ) {
delete pdataCur.handle;
pdataCur.events = {};
dataPriv.remove( dest, "handle events" );

for ( type in events ) {
for ( i = 0, l = events[ type ].length; i < l; i++ ) {
Expand Down
15 changes: 15 additions & 0 deletions test/unit/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -998,3 +998,18 @@ QUnit.test( ".data(prop) does not create expando", function( assert ) {
}
}
} );

QUnit.test( "keys matching Object.prototype properties (gh-3256)", function( assert ) {
assert.expect( 2 );

var div = jQuery( "<div/>" );

assert.strictEqual( div.data( "hasOwnProperty" ), undefined,
"hasOwnProperty not matched (before forced data creation)" );

// Force the creation of a data object for this element.
div.data( { foo: "bar" } );

assert.strictEqual( div.data( "hasOwnProperty" ), undefined,
"hasOwnProperty not matched (after forced data creation)" );
} );
43 changes: 43 additions & 0 deletions test/unit/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,49 @@ QUnit.test( "jQuery.off using dispatched jQuery.Event", function( assert ) {
.remove();
} );

QUnit.test( "events with type matching an Object.prototype property (gh-3256)", function( assert ) {
assert.expect( 1 );

var elem = jQuery( "<div/>" ),
eventFired = false;

elem.appendTo( "#qunit-fixture" );

try {
elem
.one( "hasOwnProperty", function() {
eventFired = true;
} )
.trigger( "hasOwnProperty" );
} finally {
assert.strictEqual( eventFired, true, "trigger fired without crashing" );
}
} );

QUnit.test( "events with type matching an Object.prototype property, cloned element (gh-3256)",
function( assert ) {
assert.expect( 1 );

var elem = jQuery( "<div/>" ),
eventFired = false;

elem.appendTo( "#qunit-fixture" );

try {
// Make sure the original element has some event data.
elem.on( "click", function() {} );

elem
.clone( true )
.one( "hasOwnProperty", function() {
eventFired = true;
} )
.trigger( "hasOwnProperty" );
} finally {
assert.strictEqual( eventFired, true, "trigger fired without crashing" );
}
} );

// selector-native does not support scope-fixing in delegation
QUnit[ QUnit.jQuerySelectors ? "test" : "skip" ]( "delegated event with delegateTarget-relative selector", function( assert ) {
assert.expect( 3 );
Expand Down

0 comments on commit 9d76c0b

Please sign in to comment.