From 37cbe4b9cc7e9affb49a9087452989da14f06403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski-Owczarek?= Date: Tue, 14 Jan 2025 11:37:07 +0100 Subject: [PATCH] Data: Patch & warn about props from Object.prototype in data objects Ref gh-559 --- src/jquery/data.js | 86 ++++++++++++++++++++++++++++++++++++++++ src/migrate.js | 1 + test/data/testinit.js | 1 + test/unit/jquery/data.js | 21 ++++++++++ warnings.md | 9 +++++ 5 files changed, 118 insertions(+) create mode 100644 src/jquery/data.js create mode 100644 test/unit/jquery/data.js diff --git a/src/jquery/data.js b/src/jquery/data.js new file mode 100644 index 00000000..be86342f --- /dev/null +++ b/src/jquery/data.js @@ -0,0 +1,86 @@ +import { migratePatchFunc, migrateWarn } from "../main.js"; + +function patchDataProto( original, options ) { + var i, + apiName = options.apiName, + isInstanceMethod = options.isInstanceMethod, + + // `Object.prototype` keys are not enumerable so list the + // official ones here. An alternative would be wrapping + // data objects with a Proxy but that creates additional issues + // like breaking object identity on subsequent calls. + objProtoKeys = [ + "__proto__", + "__defineGetter__", + "__defineSetter__", + "__lookupGetter__", + "__lookupSetter__", + "hasOwnProperty", + "isPrototypeOf", + "propertyIsEnumerable", + "toLocaleString", + "toString", + "valueOf" + ], + + // Use a null prototype at the beginning so that we can define our + // `__proto__` getter & setter. We'll reset the prototype afterwards. + intermediateDataObj = Object.create( null ); + + for ( i = 0; i < objProtoKeys.length; i++ ) { + ( function( key ) { + Object.defineProperty( intermediateDataObj, key, { + get: function() { + migrateWarn( "data-null-proto", + "Accessing properties from " + apiName + + " inherited from Object.prototype is removed" ); + return ( key + "__cache" ) in intermediateDataObj ? + intermediateDataObj[ key + "__cache" ] : + Object.prototype[ key ]; + }, + set: function( value ) { + migrateWarn( "data-null-proto", + "Setting properties from " + apiName + + " inherited from Object.prototype is removed" ); + intermediateDataObj[ key + "__cache" ] = value; + } + } ); + } )( objProtoKeys[ i ] ); + } + + Object.setPrototypeOf( intermediateDataObj, Object.prototype ); + + return function jQueryDataProtoPatched() { + var result = original.apply( this, arguments ); + + if ( arguments.length !== ( isInstanceMethod ? 0 : 1 ) || result === undefined ) { + return result; + } + + // Insert an additional object in the prototype chain between `result` + // and `Object.prototype`; that intermediate object proxies properties + // to `Object.prototype`, warning about their usage first. + Object.setPrototypeOf( result, intermediateDataObj ); + + return result; + }; +} + +// Yes, we are patching jQuery.data twice; here & above. This is necessary +// so that each of the two patches can be independently disabled. +migratePatchFunc( jQuery, "data", + patchDataProto( jQuery.data, { + apiName: "jQuery.data()", + isPrivateData: false, + isInstanceMethod: false + } ), + "data-null-proto" ); +migratePatchFunc( jQuery.fn, "data", + patchDataProto( jQuery.fn.data, { + apiName: "jQuery.fn.data()", + isPrivateData: true, + isInstanceMethod: true + } ), + "data-null-proto" ); + +// TODO entry in warnings.md diff --git a/src/migrate.js b/src/migrate.js index ef81d526..6272946d 100644 --- a/src/migrate.js +++ b/src/migrate.js @@ -6,6 +6,7 @@ import "./jquery/selector.js"; import "./jquery/ajax.js"; import "./jquery/attributes.js"; import "./jquery/css.js"; +import "./jquery/data.js"; import "./jquery/effects.js"; import "./jquery/event.js"; import "./jquery/manipulation.js"; diff --git a/test/data/testinit.js b/test/data/testinit.js index a138bc58..6d4193a6 100644 --- a/test/data/testinit.js +++ b/test/data/testinit.js @@ -62,6 +62,7 @@ "unit/jquery/ajax.js", "unit/jquery/attributes.js", "unit/jquery/css.js", + "unit/jquery/data.js", "unit/jquery/deferred.js", "unit/jquery/effects.js", "unit/jquery/event.js", diff --git a/test/unit/jquery/data.js b/test/unit/jquery/data.js new file mode 100644 index 00000000..97f00f25 --- /dev/null +++ b/test/unit/jquery/data.js @@ -0,0 +1,21 @@ +QUnit.module( "data" ); + +QUnit.test( "properties from Object.prototype", function( assert ) { + assert.expect( 6 ); + + var div = jQuery( "
" ).appendTo( "#qunit-fixture" ); + + div.data( "foo", "bar" ); + + expectNoMessage( assert, "Regular properties", function() { + assert.strictEqual( div.data( "foo" ), "bar", "data access" ); + assert.strictEqual( jQuery.data( div[ 0 ], "foo" ), "bar", "data access (static method)" ); + } ); + + expectMessage( assert, "Properties from Object.prototype", 2, function() { + assert.ok( div.data().hasOwnProperty( "foo" ), + "hasOwnProperty works" ); + assert.ok( jQuery.data( div[ 0 ] ).hasOwnProperty( "foo" ), + "hasOwnProperty works (static method)" ); + } ); +} ); diff --git a/warnings.md b/warnings.md index f371cba9..b41ebe9c 100644 --- a/warnings.md +++ b/warnings.md @@ -144,6 +144,15 @@ This is _not_ a warning, but a console log message the plugin shows when it firs **Solution:** Always pass string values to `.css()`, and explicitly add units where required. For example, use `$.css("line-height", "2")` to specify 200% of the current line height or `$.css("line-height", "2px")` to specify pixels. When the numeric value is in a variable, ensure the value is converted to string, e.g. `$.css("line-height", String(height))` and `$.css("line-height", height+"px")`. +### \[data-null-proto\] Accessing properties from jQuery.data() inherited from Object.prototype is removed +### \[data-null-proto\] Setting properties from jQuery.data() inherited from Object.prototype is removed +### \[data-null-proto\] Accessing properties from jQuery.fn.data() inherited from Object.prototype is removed +### \[data-null-proto\] Setting properties from jQuery.fn.data() inherited from Object.prototype is removed + +**Cause:** As of jQuery 4.0.0, data objects no longer inherit from `Object.prototype`. This includes properties like `__proto__` or `hasOwnProperty`. + +**Solution:** Don't use properties inherited from `Object.prototype` on data objects. Instead of `jQuery.data( node ).hasOwnProperty( "foo" )` use `Object.hasOwn( jQuery.data( node ), "foo" )` or, if you need to support older browsers like IE 11, use `Object.prototype.hasOwnProperty.call( jQuery.data( node ), "foo" )`. + ### \[self-closed-tags\] JQMIGRATE: HTML tags must be properly nested and closed: _(HTML string)_ **Cause:** jQuery 3.5.0 changed the way it processes HTML strings. Previously, jQuery would attempt to fix self-closed tags like `` that the HTML5 specification says are not self-closed, turning it into ``. This processing can create a [security problem](https://nvd.nist.gov/vuln/detail/CVE-2020-11022) with malicious strings, so the functionality had to be removed.