Protect against IJsonWritable instances (PersistentResponse, ..) being improperly serialized by custom IJsonSerializer implementations #954

Closed
KrzysFR opened this Issue Nov 12, 2012 · 0 comments

2 participants

@KrzysFR

I have a scenario where I need to have a custom JSON serializer (not based on JSON.Net) to serialize custom payloads (for performance reasons), so I have implemented a custom IJsonSerializer that I register with the DependencyResolver:

resolver.Register(typeof(IJsonSerializer), () => new MySuperAwesomeSerializer(someSettings));

This works well, and my objects are serialized correctly, but I can see that the serializer instance is also being called to serialize objetcts like PersistentResponse, that are internal to SignalR.

These objects are custom serialized (via the IJsonWritable.WriteJson(TextWriter) in PersistentResponse) using Json.Net, and expose this fact by implementing the IJsonWritable interface.

Anyone doing a naive implementation of IJsonSerializer, and forgetting to test for this interface, will end up serializing the PersistentResponse incorrectly. Or he would need to copy/paste the implementation of PersistentResponse.WriteJson(...) and port it to whatever library he is using, exposing himself to API breakage if you change the wire format protocol (like it happened with v1.1).

Also, buggy implementation could break SignalR in interesting ways if they bork the wire format by, for example, lower-casing all property names (typical if someone wants to camelCase all hub methods/args).

One way to protect against this, would be to add an extension method that would check the type of objects, and call .WriteJson(..) on them if they are IJsonWritable, or pass them to the registered IJsonSerializer if not.

Basically, taking the following code, and converting it to a static extension method called for all serializers, instead of only in JsonNetSerializer : https://github.com/SignalR/SignalR/blob/master/src/Microsoft.AspNet.SignalR.Core/Json/JsonNetSerializer.cs#L58

        public static void SerializeSafe(this IJsonSerializer serializer, object value, TextWriter writer)
        {
            var selfSerializer = value as IJsonWritable;
            if (selfSerializer != null)
            { // this type is internal to SignalR and must not be custom serialized
                selfSerializer.WriteJson(writer);
            }
            else
            { // this type is provided by the application and can be custom serialized
                serializer.Serialize(writer, value);
            }
        }

tl;dr Guard against naive/buggy IJsonSerializer implementations breaking the wire format by having SignalR always calling IJsonWritable.WriteJson(...) directly, and only calling IJsonSerialize.Serialize(...) for other types.

@davidfowl davidfowl closed this May 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment