Skip to content

Fix SignalR typescript tests with Chrome SameSite reaction #25283

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

Merged
merged 2 commits into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/SignalR/clients/ts/FunctionalTests/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,21 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, ILogger<
{
if (context.Request.Path.Value.Contains("/negotiate"))
{
context.Response.Cookies.Append("testCookie", "testValue");
context.Response.Cookies.Append("testCookie2", "testValue2");
context.Response.Cookies.Append("expiredCookie", "doesntmatter", new CookieOptions() { Expires = DateTimeOffset.Now.AddHours(-1) });
var cookieOptions = new CookieOptions();
var expiredCookieOptions = new CookieOptions() { Expires = DateTimeOffset.Now.AddHours(-1) };
if (context.Request.IsHttps)
{
cookieOptions.SameSite = Microsoft.AspNetCore.Http.SameSiteMode.None;
cookieOptions.Secure = true;

expiredCookieOptions.SameSite = Microsoft.AspNetCore.Http.SameSiteMode.None;
expiredCookieOptions.Secure = true;
}
context.Response.Cookies.Append("testCookie", "testValue", cookieOptions);
context.Response.Cookies.Append("testCookie2", "testValue2", cookieOptions);

cookieOptions.Expires = DateTimeOffset.Now.AddHours(-1);
context.Response.Cookies.Append("expiredCookie", "doesntmatter", expiredCookieOptions);
}

await next.Invoke();
Expand Down
12 changes: 12 additions & 0 deletions src/SignalR/clients/ts/FunctionalTests/ts/Common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ console.log(`Using SignalR HTTPS Server: '${ENDPOINT_BASE_HTTPS_URL}'`);
console.log(`Jasmine DEFAULT_TIMEOUT_INTERVAL: ${jasmine.DEFAULT_TIMEOUT_INTERVAL}`);

export const ECHOENDPOINT_URL = ENDPOINT_BASE_URL + "/echo";
export const HTTPS_ECHOENDPOINT_URL = ENDPOINT_BASE_HTTPS_URL + "/echo";

export function getHttpTransportTypes(): HttpTransportType[] {
const transportTypes = [];
Expand Down Expand Up @@ -131,3 +132,14 @@ export function eachHttpClient(action: (transport: HttpClient) => void) {
return action(t);
});
}

// Run test in Node or Chrome, but not on macOS
export const shouldRunHttpsTests =
// Need to have an HTTPS URL
!!ENDPOINT_BASE_HTTPS_URL &&

// Run on Node, unless macOS
(process && process.platform !== "darwin") &&

// Only run under Chrome browser
(typeof navigator === "undefined" || navigator.userAgent.search("Chrome") !== -1);
14 changes: 8 additions & 6 deletions src/SignalR/clients/ts/FunctionalTests/ts/ConnectionTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// tslint:disable:no-floating-promises

import { HttpTransportType, IHttpConnectionOptions, TransferFormat } from "@microsoft/signalr";
import { DEFAULT_TIMEOUT_INTERVAL, eachHttpClient, eachTransport, ECHOENDPOINT_URL } from "./Common";
import { DEFAULT_TIMEOUT_INTERVAL, eachHttpClient, eachTransport, ECHOENDPOINT_URL, HTTPS_ECHOENDPOINT_URL, shouldRunHttpsTests } from "./Common";
import { TestLogger } from "./TestLogger";

// We want to continue testing HttpConnection, but we don't export it anymore. So just pull it in directly from the source file.
Expand All @@ -15,6 +15,8 @@ import "./LogBannerReporter";

jasmine.DEFAULT_TIMEOUT_INTERVAL = DEFAULT_TIMEOUT_INTERVAL;

const USED_ECHOENDPOINT_URL = shouldRunHttpsTests ? HTTPS_ECHOENDPOINT_URL : ECHOENDPOINT_URL;

const commonOptions: IHttpConnectionOptions = {
logMessageContent: true,
logger: TestLogger.instance,
Expand All @@ -23,7 +25,7 @@ const commonOptions: IHttpConnectionOptions = {
describe("connection", () => {
it("can connect to the server without specifying transport explicitly", (done) => {
const message = "Hello World!";
const connection = new HttpConnection(ECHOENDPOINT_URL, {
const connection = new HttpConnection(USED_ECHOENDPOINT_URL, {
...commonOptions,
});

Expand Down Expand Up @@ -53,7 +55,7 @@ describe("connection", () => {
const message = "Hello World!";
// the url should be resolved relative to the document.location.host
// and the leading '/' should be automatically added to the url
const connection = new HttpConnection(ECHOENDPOINT_URL, {
const connection = new HttpConnection(USED_ECHOENDPOINT_URL, {
...commonOptions,
httpClient,
transport: transportType,
Expand Down Expand Up @@ -83,7 +85,7 @@ describe("connection", () => {
const message = "Hello World!";

// DON'T use commonOptions because we want to specifically test the scenario where logMessageContent is not set.
const connection = new HttpConnection(ECHOENDPOINT_URL, {
const connection = new HttpConnection(USED_ECHOENDPOINT_URL, {
httpClient,
logger: TestLogger.instance,
transport: transportType,
Expand Down Expand Up @@ -119,7 +121,7 @@ describe("connection", () => {
const message = "Hello World!";

// DON'T use commonOptions because we want to specifically test the scenario where logMessageContent is set to true (even if commonOptions changes).
const connection = new HttpConnection(ECHOENDPOINT_URL, {
const connection = new HttpConnection(USED_ECHOENDPOINT_URL, {
httpClient,
logMessageContent: true,
logger: TestLogger.instance,
Expand Down Expand Up @@ -167,7 +169,7 @@ describe("connection", () => {
const message = "Hello World!";

// The server will set some response headers for the '/negotiate' endpoint
const connection = new HttpConnection(ECHOENDPOINT_URL, {
const connection = new HttpConnection(USED_ECHOENDPOINT_URL, {
...commonOptions,
httpClient,
transport: transportType,
Expand Down
18 changes: 4 additions & 14 deletions src/SignalR/clients/ts/FunctionalTests/ts/HubConnectionTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { AbortError, DefaultHttpClient, HttpClient, HttpRequest, HttpResponse, H
import { MessagePackHubProtocol } from "@microsoft/signalr-protocol-msgpack";
import { getUserAgentHeader, Platform } from "@microsoft/signalr/dist/esm/Utils";

import { DEFAULT_TIMEOUT_INTERVAL, eachTransport, eachTransportAndProtocolAndHttpClient, ENDPOINT_BASE_HTTPS_URL, ENDPOINT_BASE_URL } from "./Common";
import { DEFAULT_TIMEOUT_INTERVAL, eachTransport, eachTransportAndProtocolAndHttpClient, ENDPOINT_BASE_HTTPS_URL, ENDPOINT_BASE_URL, shouldRunHttpsTests } from "./Common";
import "./LogBannerReporter";
import { TestLogger } from "./TestLogger";

Expand All @@ -18,6 +18,7 @@ import * as RX from "rxjs";

const TESTHUBENDPOINT_URL = ENDPOINT_BASE_URL + "/testhub";
const TESTHUBENDPOINT_HTTPS_URL = ENDPOINT_BASE_HTTPS_URL ? (ENDPOINT_BASE_HTTPS_URL + "/testhub") : undefined;
const HTTPORHTTPS_TESTHUBENDPOINT_URL = shouldRunHttpsTests ? TESTHUBENDPOINT_HTTPS_URL : TESTHUBENDPOINT_URL;

const TESTHUB_NOWEBSOCKETS_ENDPOINT_URL = ENDPOINT_BASE_URL + "/testhub-nowebsockets";
const TESTHUB_REDIRECT_ENDPOINT_URL = ENDPOINT_BASE_URL + "/redirect?numRedirects=0&baseUrl=" + ENDPOINT_BASE_URL;
Expand All @@ -28,17 +29,6 @@ const commonOptions: IHttpConnectionOptions = {
logMessageContent: true,
};

// Run test in Node or Chrome, but not on macOS
const shouldRunHttpsTests =
// Need to have an HTTPS URL
!!TESTHUBENDPOINT_HTTPS_URL &&

// Run on Node, unless macOS
(process && process.platform !== "darwin") &&

// Only run under Chrome browser
(typeof navigator === "undefined" || navigator.userAgent.search("Chrome") !== -1);

function getConnectionBuilder(transportType?: HttpTransportType, url?: string, options?: IHttpConnectionOptions): HubConnectionBuilder {
let actualOptions: IHttpConnectionOptions = options || {};
if (transportType) {
Expand Down Expand Up @@ -599,7 +589,7 @@ describe("hubConnection", () => {
}

it("preserves cookies between requests", async (done) => {
const hubConnection = getConnectionBuilder(transportType).build();
const hubConnection = getConnectionBuilder(transportType, HTTPORHTTPS_TESTHUBENDPOINT_URL).build();
await hubConnection.start();
const cookieValue = await hubConnection.invoke<string>("GetCookie", "testCookie");
const cookieValue2 = await hubConnection.invoke<string>("GetCookie", "testCookie2");
Expand All @@ -610,7 +600,7 @@ describe("hubConnection", () => {
});

it("expired cookies are not preserved", async (done) => {
const hubConnection = getConnectionBuilder(transportType).build();
const hubConnection = getConnectionBuilder(transportType, HTTPORHTTPS_TESTHUBENDPOINT_URL).build();
await hubConnection.start();
const cookieValue = await hubConnection.invoke<string>("GetCookie", "expiredCookie");
expect(cookieValue).toBeNull();
Expand Down