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

Possible confusion in SignalR.Client.Transports.AutoTransport when overriding the default transport order #1884

Closed
abillingsley opened this Issue Apr 16, 2013 · 2 comments

Comments

Projects
None yet
5 participants

The AutoTransport negotiation expects a specific transport order but provides an api to allow a custom transport order. This could cause some unexpected results in certain environments.

Given the code snippets below it would be possible for a transport to be skipped if a custom list of transports is passed in.

public AutoTransport(IHttpClient httpClient, IList<IClientTransport> transports)
public Task<NegotiationResponse> Negotiate(IConnection connection)
{
    var task = _httpClient.GetNegotiationResponse(connection);
#if NET45
    return task.Then(response =>
    {
        if (!response.TryWebSockets)
        {
            _startIndex = 1;
        }

         return response;
        });
#else
            return task;
#endif
}

NOTE I made these up and have not actually built a minimal repo project that demos the following

Repo Steps 1

  1. On a Server built against .NET40
  2. On a Client built against .NET45
  3. Define AutoTransport with Transport Order (SSE, LP)
  4. NegotiationResponse.TryWebSocket will be false
  5. SSE will be skipped and LP will be attempted

Expect SSE to be attempted first before falling back to LP

Repo Steps 2

  1. On a Server built against .NET40
  2. On a Client built against .NET45
  3. Define AutoTransport with Transport Order (SSE)
  4. NegotiationResponse.TryWebSocket will be false
  5. SSE will be skipped and IClientTransport transport = _transports[index]; will throw

In general if the server returns false for TryWebSockets and a custom transport order was passed into the constructor for example (SSE, LP) or if WS is not at index 0 above that the client will have some unexpected behavior.

@ghost ghost assigned abnanda1 May 22, 2013

@abnanda1 abnanda1 added a commit that referenced this issue Jul 16, 2013

@abnanda1 abnanda1 Made small code style changes e7a9a31

@abnanda1 abnanda1 added a commit that referenced this issue Jul 16, 2013

@abnanda1 abnanda1 Made small code style changes eab108e

@ghost ghost assigned muratg Jul 18, 2013

Owner

davidfowl commented Jul 22, 2013

@abnanda1 Why is this still in review?

@ghost ghost assigned gustavo-armenta Jul 22, 2013

Contributor

gustavo-armenta commented Jul 23, 2013

validated setting on server payload.TryWebSocket = false

Then, on client tried and it connected with LongPolling
transports = new List
{
new WebSocketTransport(client),
new LongPollingTransport(client),
new WebSocketTransport(client),
new ServerSentEventsTransport(client),
new WebSocketTransport(client),
};
AutoTransport auto = new AutoTransport(client, transports);
await hubConnection.Start(auto);

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