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

X-Requested-With header is incorrect in FetchHttpClient #44933

Closed
1 task done
val1984 opened this issue Nov 8, 2022 · 5 comments
Closed
1 task done

X-Requested-With header is incorrect in FetchHttpClient #44933

val1984 opened this issue Nov 8, 2022 · 5 comments
Labels
area-signalr Includes: SignalR clients and servers Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity

Comments

@val1984
Copy link

val1984 commented Nov 8, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

FetchHttpClient.ts sets X-Requested-With header to XMLHttpRequest which makes it harder than needed to know which HTTP client was used by @microsoft/signalr JS library.

Expected Behavior

X-Requested-With header should be set to Fetch as it was in the initial implementation.

Steps To Reproduce

Make use of @microsoft/signalr JS library and check the headers sent by the lib when calling the negotiate route.

Exceptions (if any)

No response

.NET Version

No response

Anything else?

@microsoft/signalr: ^6.0.9

@javiercn javiercn added the area-signalr Includes: SignalR clients and servers label Nov 8, 2022
@BrennanConroy
Copy link
Member

BrennanConroy commented Nov 8, 2022

This header is intentionally set so that Cookie authentication will avoid redirecting to a login page, if it actually did the redirect, it would result in an ugly, hard to diagnose error on the client side.

if (IsAjaxRequest(context.Request))
{
context.Response.Headers.Location = context.RedirectUri;

private static bool IsAjaxRequest(HttpRequest request)
{
return string.Equals(request.Query[HeaderNames.XRequestedWith], "XMLHttpRequest", StringComparison.Ordinal) ||
string.Equals(request.Headers.XRequestedWith, "XMLHttpRequest", StringComparison.Ordinal);

This behavior is customizable by setting the header yourself when creating the connection.

new signalR.HubConnectionBuilder()
    .withUrl("/chathub", { headers: { "X-Requested-With": "Fetch" } })
    .build();

@val1984
Copy link
Author

val1984 commented Nov 8, 2022

What is the point of it if it's always set to "XMLHttpRequest", even when the FetchHttpClient is used?

Shouldn't CookieAuthenticationEvents.cs be modified to also accept "Fetch"?

@BrennanConroy
Copy link
Member

which HTTP client was used by @microsoft/signalr JS library

Why do you need to know this information?

This behavior is customizable by setting the header yourself when creating the connection.

Is this customization not usable for your scenario?

Shouldn't CookieAuthenticationEvents.cs be modified to also accept "Fetch"?

No. Using "Fetch" as the header value is not a common pattern, whereas XMLHttpRequest is a very well known pattern that has been used for years across multiple technology stacks.

@rafikiassumani-msft rafikiassumani-msft added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Nov 15, 2022
@ghost
Copy link

ghost commented Nov 15, 2022

Hi @val1984. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@ghost
Copy link

ghost commented Nov 21, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Nov 24, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-signalr Includes: SignalR clients and servers Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. Status: No Recent Activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants