Skip to main content

Bug Tracker

Side navigation

#8108 closed bug (fixed)

Opened January 31, 2011 11:39PM UTC

Closed February 14, 2011 10:23PM UTC

Last modified March 13, 2012 02:40PM UTC

jQuery metadata is exposed on plain JS objects when serializing with JSON.stringify

Reported by: BorisMoore Owned by: dmethvin
Priority: blocker Milestone: 1.5.1
Component: data Version: 1.5
Keywords: Cc:
Blocked by: Blocking:
Description

New event binding code in 1.5 is adding jQuery expandos to plain objects that are not of type function, and therefore show up on JSON serialized objects.

See comment and suggested fix:

https://github.com/jquery/jquery/commit/8e59a99e0ade75dec434f246f52e8b3f7393f359#src/data.js-P71

Attachments (0)
Change History (21)

Changed January 31, 2011 11:40PM UTC by BorisMoore comment:1

component: unfileddata
keywords: → Events

Changed January 31, 2011 11:47PM UTC by john comment:2

milestone: 1.next1.5.1
priority: undecidedblocker

Changed January 31, 2011 11:48PM UTC by rwaldron comment:3

owner: → BorisMoore
status: newpending

Please provide a reduced jsFiddle test case to help us assess your ticket

Changed February 01, 2011 12:15AM UTC by jitter comment:4

Changed February 01, 2011 12:18AM UTC by dmethvin comment:5

ha you beat me jitter, here's my test: http://jsfiddle.net/FFgdz/

Changed February 01, 2011 06:05PM UTC by BorisMoore comment:6

status: pendingnew

This was assigned to me, but I think it would be better assigned to snover or similar, since I am not set up for committing fixes to jQuery itself.

I have made all necessary changes to DataLink to work against 1.5, given the new behavior of .data(). This bug concerns serialization of plain objects in general, and is a problem in the data component. It has no specific bearing on data link, and data link does not directly depend on this fix.

Should I simply reassign?

Changed February 06, 2011 04:01PM UTC by snover comment:7

keywords: Events
owner: BorisMooresnover
status: newassigned

Changed February 07, 2011 04:52PM UTC by Colin Snover comment:8

resolution: → fixed
status: assignedclosed

Update $.data to use a function instead of an object when attaching to JS objects in order to hide attached metadata from JSON.stringify. Remove event.js code that was doing this before specifically for events, which is now redundant. Fixes #8108. 1.5-stable

Changeset: 7acb141ed7f2dedd950bb65acf878098640d081e

Changed February 08, 2011 05:55PM UTC by jitter comment:9

resolution: fixed
status: closedreopened

Reopening to make sure we don't ship with this, as this fix fails the test suite in several browsers including:

  • Opera 11.01, 11.00, 10.63, 10.54
  • FF 3.5.11, 3.0.19, 2.0.0.20
  • Safari 4.0.5, 3.2.3, 3.1.2

Not all of those are officially supported (anymore) but many are.

Special attention is needed for the fail in Safari 4.0.5 which don't seem to related to prototype being an enumerable property. Safari 4.0.5 fails the test included for this ticket data: JSON serialization (#8108) -> Expando is hidden from JSON.stringify which probably means there is bug in the JSON.stringify implementation in Safari 4.0.5

Changed February 08, 2011 05:55PM UTC by jitter comment:10

status: reopenedopen

Changed February 09, 2011 03:02AM UTC by snover comment:11

resolution: → wontfix
status: openclosed

There’s no way to do this reliably; ES3 exposes enumerable properties by default on function instances. ES5 does not, but not all supported browsers have been updated to support ES5. Users that want to serialize JS objects that have been modified using $.data should pass a replacer function that filters out jQuery.expando properties by returning undefined.

Changed February 09, 2011 05:39AM UTC by BorisMoore comment:12

In 1.4.4, before the change to the .data implementation, those test cases were presumably passing, right? At that point, events were being added to plain objects, as an expando of type function. If that is the case, then I have not understood why a modified .data implementation can't also work in the same way without breaking test cases.

Also I have not understood why a function instance exposing properties in ES3 is a problem. We want a plain object, which is itself a 'real' object, (not a function with added expando properties), to be able to have expandos of type property added to it, and for those expandos not to show up in the JSON serialization. Are you saying that stringify in Safari 4.0.5, does not hide expandos of type function from the serialization?

Changed February 09, 2011 02:37PM UTC by snover comment:13

There were no test cases that applied in 1.4.4. JSON serialization was never tested explicitly. The only internal property that was hidden in this way was the internal events property, which was far from the only property that was potentially added to objects. The only test that existed in the 1.4.4 suite was “is events a function when added to a plain JS object?”. There was never an explicit test to see what a serialized object actually looked like until I added it along with this patch, and then it turned out that it only passed in IE and browsers that had started implementing ES5 (which changed the property to DontEnum).

It’s a problem for three reasons.

1. ES3 exposes a prototype property by default as a non-deletable, enumerable value of the function instance. This means that it’s impossible to have a truly empty function instance in an ES3 environment: iterating through the instance always shows it as having a prototype property that cannot be deleted. This means that an “empty” data structure actually has properties, which means that a bunch of other tests start to fail, and the only way to work around those is to add more hacks in jQuery to deal with and ignore the potential existence of a prototype property.

2. Using a function instance ignores the very real possibility of people extending Function.prototype with their own shit which ends up being enumerable on function instances, further breaking the ability to determine whether or not the data structure is empty. Unlike Object.prototype, modifying Function.prototype is not verboten, and occurs with relatively high frequency, especially with polyfills for ES5.

3. Safari 4’s native JSON.stringify treats a Function like an Object and serializes it anyway.

There is a simple and much more viable alternative for people that want to serialize objects with attached metadata (replacer function). Please use it.

Changed February 11, 2011 02:41AM UTC by BorisMoore comment:14

I am fine with using replacer in my code - that is not the issue :). DataLink does not use stringify. The issue is that if one plugin uses .data on plain objects, or binds events to plain objects - as datalink does, then every user of that plugin who wants to serialize objects needs to know that they should provide a replacer functions, and needs to implement that function.

Better have it work per the JSON spec with all browsers which comply to that spec, and with JSON2, than have it break for all browsers. In the not to far future, JSON implementations which are not compliant, and incorrectly serialize field of type function will be less and less relevant...

Why would we prefer to break serialization on all browsers, rather than breaking serialization on only a small percentage of browsers?

Changed February 11, 2011 03:58AM UTC by snover comment:15

_comment0: Users can ignore the metadata that shows up, or they can use a replacer function to remove it—there really is no third option here. This feature is impossible to implement across all browsers that jQuery supports and actively breaks ES3-conformant browsers. Trying to hide metadata using using a function instance is a bad hack that will never work sufficiently robustly. \ \ If there is confusion among anyone about what this strange property is that shows up on their plain JS objects, then we can consider adding some documentation somewhere to address that confusion once it comes up.1297396968828869

Users can ignore the metadata that shows up, or they can use a replacer function to remove it—there really is no third option here. This feature is impossible to implement across all browsers that jQuery supports and breaks completely in standards-compliant browsers that implement ES3. Trying to hide metadata using using a function instance is a bad hack that will never work sufficiently robustly.

If there is confusion among anyone about what this strange property is that shows up on their plain JS objects, then we can consider adding some documentation somewhere to address that confusion once it comes up.

Changed February 11, 2011 07:19PM UTC by snover comment:16

summary: JSON serialization broken: Event binding code expandos added to plain objects should be of type function.jQuery metadata is exposed on plain JS objects when serializing with JSON.stringify

Changed February 11, 2011 08:36PM UTC by dmethvin comment:17

resolution: wontfix
status: closedreopened

I'm going to look at the trick that jitter suggested, override toJSON() on the cache object so it returns undefined.

Changed February 11, 2011 08:36PM UTC by dmethvin comment:18

owner: snoverdmethvin
status: reopenedassigned

Changed February 11, 2011 08:39PM UTC by BorisMoore comment:19

Great, thanks very much Dave. Sounds very promising.

Also, I agree with the modified description from Colin. The problem is not specific to events - it is with stringify and jQuery metadata in general...

Changed February 14, 2011 10:23PM UTC by Colin Snover comment:20

resolution: → fixed
status: assignedclosed

Hide metadata when serializing JS objects using JSON.stringify via a toJSON hack. Fixes #8108.

Changeset: 2ed81708bdacfd4b97b77baef67ad8b75205dd20

Changed April 12, 2011 04:30AM UTC by jeresig comment:21

Removing un-needed frameElement check as discussed in #8018. Fixes #8108.

Changeset: 7bfb6a7dd315f0eda3e5227f7d41e38f66f46549