Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix to protect against potential JArray foreach issue #2691

Closed
JamesNK opened this Issue Nov 7, 2013 · 6 comments

Comments

Projects
None yet
6 participants

JamesNK commented Nov 7, 2013

A couple of months ago to fixed an issue in json.net I change GetEnumerator on JArray from an explicit implementation to public.

Unfortunately because of the way foreach will silently add a cast to IEnumerable if needed, the change made it so json.net errors when it has been compiled with a reference to the newest version of json.net but for some reason or another at runtime it uses a json.net assembly that is version 5.0.4 or older.

The only place in SignalR that is effected is TransportHelper that does a foreach over a JArray:

To prevent anyone running into this issue you can either cast the JArray to IEnumerable before doing the foreach or just change the foreach to a for loop.

Contributor

akoeplinger commented Nov 7, 2013

This seems to be the cause for #2669.

JamesNK commented Nov 9, 2013

Yes I think so

JamesNK commented Nov 9, 2013

What version of json.net is SignalR compiled against? The source code appears to say 4.5.11 - https://github.com/SignalR/SignalR/blob/master/src/Microsoft.AspNet.SignalR.Client/Microsoft.AspNet.SignalR.Client.csproj - but this error should only become a problem if it is compiled against 5.0.5 or higher.

Edit: Ah, signalr 2.0 is on the release branch

@ghost ghost assigned abnanda1 Nov 22, 2013

Owner

davidfowl commented Nov 22, 2013

@abnanda1 Can you look into this?

@ghost ghost assigned halter73 Nov 27, 2013

halter73 added a commit that referenced this issue Nov 27, 2013

Fix to protect against potential JArray foreach issue
- This issue is most likely to occur when an older version of Json.Net
  (<= 5.0.4) is GACed

#2669
#2691

halter73 added a commit that referenced this issue Nov 27, 2013

Fix to protect against potential JArray foreach issue
- This issue is most likely to occur when an older version of Json.Net
  (<= 5.0.4) is GACed

#2669
#2691

halter73 added a commit that referenced this issue Dec 5, 2013

Ensure that the "Messages" portion of a PersistentResponse is an array
- .NET client
- Previously it was possible to iterate over an object instead
- Thanks to @JamesNK for pointing it out

c20d822#commitcomment-4729636

#2691

halter73 added a commit that referenced this issue Dec 5, 2013

Ensure that the "Messages" portion of a PersistentResponse is an array
- .NET client
- Previously it was possible to iterate over an object instead
- Thanks to @JamesNK for pointing it out

c20d822#commitcomment-4729636

#2691

JamesNK commented Dec 12, 2013

Do you guys have an estimate of when 2.0.2 will be released? I'd like to release 6.0 of Json.NET soon but I'd rather wait until after 2.0.2 because of the breaking change.

@ghost ghost assigned gustavo-armenta Dec 15, 2013

Contributor

gustavo-armenta commented Dec 18, 2013

tested I get an error when using SignalR 2.0 and Json 4.5.11. Error goes away when keeping same Json version and upgrading Client to "2.1.0-pre-131218-b313"

18:52:07.4501610 - 87fb2b44-c540-46be-8502-aa27df354010 - OnError(System.MissingMethodException: Method not found: 'System.Collections.Generic.IEnumerator`1<Newtonsoft.Json.Linq.JToken> Newtonsoft.Json.Linq.JArray.GetEnumerator()'.
at Microsoft.AspNet.SignalR.Client.Transports.TransportHelper.ProcessResponse(IConnection connection, String response, Boolean& shouldReconnect, Boolean& disconnected, Action onInitialized)
at Microsoft.AspNet.SignalR.Client.Transports.WebSocketTransport.OnMessage(String message)
at Microsoft.AspNet.SignalR.WebSockets.WebSocketHandler.d__e.MoveNext())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment