JSON.stringify doesn't work on IDispatch wrappers in JScript

[matthewgertner]

If I get an object in Internet Explorer's JScript engine from e.g. an add-on written in C++, I don't get a native JavaScript object. Instead, I get a wrapper that uses Microsoft's IDispatch interface to duplicate the behavior of a native JS object. For example, the hasOwnProperty method will use some IDispatch method (maybe GetIDsOfNames?) to return the correct value.

In your library, you cache the value of Object.prototype.hasOwnProperty in a variable called isProperty that is reused for all the objects that are passed into the stringify() method. As a result, an object received via IDispatch will use the wrong method (i.e. the default hasOwnProperty instead of the special IDispatch hasOwnProperty). This causes the library to think that it should serialize the object's "constructor" property since isProperty.call(object, "constructor") erroneously returns true. The end result is a type error since it thinks the object has a cyclic structure.

The right solution might be to check in forEach whether the object is an IDispatch wrapper and always use its hasOwnProperty method if it is. I'm not sure how to determine this, but if this sounds like a viable approach to you, I can have a look around and propose a patch.

[ghost]

That's quite a unique use case. Given that IDispatch wrappers are host objects, JSON 3 is technically permitted to throw an exception (serialization rules are only defined for native objects). However, that's obviously not what we want to happen here. If you have the time, I'd love to see a patch.

Out of interest, what does typeof wrapper.constructor return?

[matthewgertner]

Good call. It doesn't look like typeof(wrapper.constructor) is function for IDispatch wrappers so hopefully we can use that to differentiate. Will post a pull request.

[matthewgertner]

Oops, when I updated to the latest json3 it throws in the lexer in some obscure circumstances in our app. Very hard to debug because it's running in a background service and the problem doesn't seem to be entirely deterministic. I'll need to get to the bottom of that when I have time, so it'll a bit longer to get a pull request done. But I will do so.

[ghost]

Awesome. I'll try to make time this week to test out the constructor fix as well; I've just been terribly busy.

The only change I made to the parser in v3.2.3 was a bug fix: 629a94dc23345ecabf4945c26ed1efef077f74cf. In prior versions, JSON 3 would parse "[}" as []; this now correctly throws a SyntaxError.

[matthewgertner]

You can see the complete diff of your latest version and the one I am using here: https://gist.github.com/3755025. The IDispatch fix is in there (search for isPropertyFunc) and works. The problem I'm getting is that at some point Source is unexpectedly set to null. Obviously the Source stuff is all new code in v3.2.3. I've done no analysis on this yet so I'm assuming that we're doing something weird, or maybe there's a new bug related to the use of IDispatch host objects. I'll get back to you once I've had time to look into it.

[ghost]

@matthewgertner I just pushed a new release—v3.2.4—that fixed several critical bugs. It doesn't have your isPropertyFunc fix baked in, but I'd like to include it in the next release. When you have time, would you mind testing 3.2.4 and see if it's still throwing?

[ghost]

@matthewgertner Here's a drop-in debug build for you that contains the isPropertyFunc fix and throws detailed syntax errors. Let's see if we can track down the issue you're having with the parser.

[matthewgertner]

Thanks, Kit. Apologies for being slow... been finishing up a hair-on-fire project so very much in "if it ain't broke don't fix it" mode. I've got more breathing room now and I'll get you feedback early next week on this.

[ghost]

Not a problem! Whenever you have time...

[matthewgertner]

I dropped in your code and it seems to work fine. I'm not sure why I was having problems before (it's possible that my testing config was slightly different, or maybe it's due to changes you made in this new build). Anyway, I'll let you know if we have further problems, but based on my brief tests and eyeballing the changes you made to property enumeration code, I would say it's good to go.

[ghost]

This is now in v3.2.5.