Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hub calls should fail if request's response is 3xx redirect #2106

Closed
SteveSyfuhs opened this issue May 30, 2013 · 10 comments
Closed

Hub calls should fail if request's response is 3xx redirect #2106

SteveSyfuhs opened this issue May 30, 2013 · 10 comments
Assignees
Milestone

Comments

@SteveSyfuhs
Copy link

The client library should treat requests with responses with redirect codes (e.g. 302/301) as failed.

This is problematic in situations where browser sessions expire but the client library is still trying to call back -- e.g. via timer

setInterval(function() { // trivial example
     $.connection.myHub.server.getLatest().done(function(someResp) {
          updateSomething(someResp);
     }).fail(function() { 
          alert('bang'); 
     });
}, 90000); // 90 seconds

The observed behavior is that the request is made to the server, the server responds 302, the browser then does the redirect and requests the login page, but neither done() or fail() are called.

@ghost ghost assigned davidfowl Jun 25, 2013
@davidfowl
Copy link
Member

We'll fix this for 2.0

@ghost ghost assigned NTaylorMullen Jun 27, 2013
@NTaylorMullen
Copy link
Contributor

I feel that a better approach would be to fail IF we are unable to parse the response body. This alternative is more of a general fail condition where if we're redirected to a page that is not valid JSON we can fail appropriately.

NTaylorMullen added a commit that referenced this issue Jul 8, 2013
…hen an error occurs, instead of throwing to the window object we now handle it appropriately.

#2106
NTaylorMullen added a commit that referenced this issue Jul 8, 2013
…rifying parse responses is handled correctly when it throws.

#2106
NTaylorMullen added a commit that referenced this issue Jul 15, 2013
NTaylorMullen added a commit that referenced this issue Jul 16, 2013
…hen an error occurs, instead of throwing to the window object we now handle it appropriately.

#2106
NTaylorMullen added a commit that referenced this issue Jul 16, 2013
…rifying parse responses is handled correctly when it throws.

#2106
NTaylorMullen added a commit that referenced this issue Jul 16, 2013
@ghost ghost assigned muratg Jul 16, 2013
@kbirger
Copy link

kbirger commented Jul 17, 2013

Whatever was changed between 1.1.2 and 2.0.0 beta has caused some of the service resources like /xxxhub/signalr/connect to trigger forms authentication (without consulting the authorizeattribute on the hub -- which seems wrong to me). I'm not sure what it was, and I think it would be better if this was handled with a 401 and not a 302. One reason is that it seems to break IE9. Right now, IE is propagating the 302 to the entire page and redirecting me away from the page I'm trying to view.

@ghost ghost assigned NTaylorMullen Jul 18, 2013
@NTaylorMullen
Copy link
Contributor

Moving this back into the working state, we need to special case requests that occur while in the connecting state to ensure if a redirect or some other parse error occurs during that time period that we fall back and not error->stop.

NTaylorMullen added a commit that referenced this issue Jul 18, 2013
- If transports fail to parse a response while in connecting they will now fall back.
- Centralized this logic in the common.js

#2106
NTaylorMullen added a commit that referenced this issue Jul 18, 2013
NTaylorMullen added a commit that referenced this issue Jul 18, 2013
- If transports fail to parse a response while in connecting they will now fall back.
- Centralized this logic in the common.js

#2106
NTaylorMullen added a commit that referenced this issue Jul 18, 2013
@ghost ghost assigned gustavo-armenta Jul 18, 2013
@NTaylorMullen
Copy link
Contributor

@kbirger the latest change should fix your issue. As for the IE9 redirection I wasn't able to reproduce that issue.

@gustavo-armenta
Copy link
Contributor

Verified OnError event is raised when request is redirected and the response content is not valid json

LP-Connect
SignalR: No transport could be initialized successfully. Try specifying a different transport or none at all for auto initialization.
LP-Poll
SignalR: failed at parsing response: . With error: Unexpected token <
LP-Send
SyntaxError: Unexpected token <

@davidfowl
Copy link
Member

@gustavo-armenta did you test any auth scenarios?

@gustavo-armenta
Copy link
Contributor

@davidfowl
I added a middleware to simulate invalid basic auth credentials during "/poll" and "/send"

map.Use((context, next) =>
{
    if (context.Request.Path.Contains("/poll") || context.Request.Path.Contains("/send"))
    {        
        if (++requestNumber % 2 > 0)
        {
            context.Request.Headers["Authorization"] = "Basic " + Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("user:wrong_password"));
        }
    }

    return next.Invoke();
});

We are sending a response with 401, and the user is asked to input credentials again

HTTP/1.1 401 Unauthorized
Cache-Control: private
Transfer-Encoding: chunked
Content-Type: text/html; charset=utf-8
Server: Microsoft-IIS/8.0
WWW-Authenticate: Basic
X-AspNet-Version: 4.0.30319
X-SourceFiles: =?UTF-8?B?RDpcc2lnbmFsclxkZXZcc2FtcGxlc1xNaWNyb3NvZnQuQXNwTmV0LlNpZ25hbFIuU2FtcGxlc1xiYXNpY2F1dGhcc2lnbmFsclxwb2xs?=
X-Powered-By: ASP.NET
Date: Tue, 23 Jul 2013 04:27:06 GMT

67
<html><head><title>Authentication Failed</title></head><body>Invalid username or password</body></html>
0

@davidfowl
Copy link
Member

Very nice. Can you try forms auth as well?

@gustavo-armenta
Copy link
Contributor

I tried forms auth:
After signout, a websocket connection can still send messages successfully while the rest of transports fail to send message, get 302 redirection to login page, fail to parse JSON content as it is HTML, and disconnect the signalr client

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

No branches or pull requests

6 participants