From 5e41934dec53818ef8cb10ff3b0a25933969dc8e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 7 Apr 2021 16:06:50 -0700 Subject: [PATCH 1/6] Still send 100 Continue with null MinRequestBodyDataRate (#31284) --- .../Core/src/Internal/Http/MessageBody.cs | 10 +++--- .../InMemory.FunctionalTests/RequestTests.cs | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs index bae364ca566e..b17272465007 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs @@ -154,13 +154,15 @@ protected void AddAndCheckConsumedBytes(long consumedBytes) protected ValueTask StartTimingReadAsync(ValueTask readAwaitable, CancellationToken cancellationToken) { - - if (!readAwaitable.IsCompleted && _timingEnabled) + if (!readAwaitable.IsCompleted) { TryProduceContinue(); - _backpressure = true; - _context.TimeoutControl.StartTimingRead(); + if (_timingEnabled) + { + _backpressure = true; + _context.TimeoutControl.StartTimingRead(); + } } return readAwaitable; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index fef0608c1c9e..a975fc89961b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -590,6 +590,42 @@ await connection.ReceiveEnd( } } + [Fact] + public async Task Expect100ContinueHonoredWhenMinRequestBodyDataRateIsDisabled() + { + var testContext = new TestServiceContext(LoggerFactory); + + // This may seem unrelated, but this is a regression test for + // https://github.com/dotnet/aspnetcore/issues/30449 + testContext.ServerOptions.Limits.MinRequestBodyDataRate = null; + + await using (var server = new TestServer(TestApp.EchoAppChunked, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Expect: 100-continue", + "Connection: close", + "Content-Length: 11", + "\r\n"); + await connection.Receive( + "HTTP/1.1 100 Continue", + "", + ""); + await connection.Send("Hello World"); + await connection.ReceiveEnd( + "HTTP/1.1 200 OK", + "Connection: close", + $"Date: {testContext.DateHeaderValue}", + "Content-Length: 11", + "", + "Hello World"); + } + } + } + [Fact] public async Task ZeroContentLengthAssumedOnNonKeepAliveRequestsWithoutContentLengthOrTransferEncodingHeader() { From cceefc7969e98b5197c5f6fcabcdaaa2f51de063 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 7 Apr 2021 16:07:34 -0700 Subject: [PATCH 2/6] [SignalR TS] Fix permanent Disconnecting state (#30948) (#31251) --- .../clients/ts/signalr/src/HttpConnection.ts | 6 +- .../clients/ts/signalr/src/HubConnection.ts | 6 +- .../ts/signalr/tests/HttpConnection.test.ts | 19 ++-- .../tests/HubConnection.Reconnect.test.ts | 93 +++++++++++++++++++ .../clients/ts/signalr/tests/TestWebSocket.ts | 54 +++++++++++ 5 files changed, 169 insertions(+), 9 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index 81f42d27bcb1..2389e02e7117 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -61,7 +61,7 @@ export class HttpConnection implements IConnection { private transport?: ITransport; private startInternalPromise?: Promise; private stopPromise?: Promise; - private stopPromiseResolver!: (value?: PromiseLike) => void; + private stopPromiseResolver: (value?: PromiseLike) => void = () => {}; private stopError?: Error; private accessTokenFactory?: () => string | Promise; private sendQueue?: TransportSendQueue; @@ -208,7 +208,6 @@ export class HttpConnection implements IConnection { this.transport = undefined; } else { this.logger.log(LogLevel.Debug, "HttpConnection.transport is undefined in HttpConnection.stop() because start() failed."); - this.stopConnection(); } } @@ -288,6 +287,9 @@ export class HttpConnection implements IConnection { this.logger.log(LogLevel.Error, "Failed to start the connection: " + e); this.connectionState = ConnectionState.Disconnected; this.transport = undefined; + + // if start fails, any active calls to stop assume that start will complete the stop promise + this.stopPromiseResolver(); return Promise.reject(e); } } diff --git a/src/SignalR/clients/ts/signalr/src/HubConnection.ts b/src/SignalR/clients/ts/signalr/src/HubConnection.ts index 62190c166cd8..5d55bedf03af 100644 --- a/src/SignalR/clients/ts/signalr/src/HubConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HubConnection.ts @@ -762,7 +762,11 @@ export class HubConnection { this.logger.log(LogLevel.Information, `Reconnect attempt failed because of error '${e}'.`); if (this.connectionState !== HubConnectionState.Reconnecting) { - this.logger.log(LogLevel.Debug, "Connection left the reconnecting state during reconnect attempt. Done reconnecting."); + this.logger.log(LogLevel.Debug, `Connection moved to the '${this.connectionState}' from the reconnecting state during reconnect attempt. Done reconnecting.`); + // The TypeScript compiler thinks that connectionState must be Connected here. The TypeScript compiler is wrong. + if (this.connectionState as any === HubConnectionState.Disconnecting) { + this.completeClose(); + } return; } diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index 8981475e43e6..96bd009db502 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts @@ -132,23 +132,30 @@ describe("HttpConnection", () => { it("can stop a starting connection", async () => { await VerifyLogger.run(async (logger) => { + const stoppingPromise = new PromiseSource(); + const startingPromise = new PromiseSource(); const options: IHttpConnectionOptions = { ...commonOptions, httpClient: new TestHttpClient() .on("POST", async () => { - await connection.stop(); + startingPromise.resolve(); + await stoppingPromise; return "{}"; - }) - .on("GET", async () => { - await connection.stop(); - return ""; }), logger, } as IHttpConnectionOptions; const connection = new HttpConnection("http://tempuri.org", options); - await expect(connection.start(TransferFormat.Text)) + const startPromise = connection.start(TransferFormat.Text); + + await startingPromise; + const stopPromise = connection.stop(); + stoppingPromise.resolve(); + + await stopPromise; + + await expect(startPromise) .rejects .toThrow("The connection was stopped during negotiation."); }, diff --git a/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts b/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts index 15e5e953a0d7..825fff89ace8 100644 --- a/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HubConnection.Reconnect.test.ts @@ -2,13 +2,17 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. import { DefaultReconnectPolicy } from "../src/DefaultReconnectPolicy"; +import { HttpConnection, INegotiateResponse } from "../src/HttpConnection"; import { HubConnection, HubConnectionState } from "../src/HubConnection"; +import { IHttpConnectionOptions } from "../src/IHttpConnectionOptions"; import { MessageType } from "../src/IHubProtocol"; import { RetryContext } from "../src/IRetryPolicy"; import { JsonHubProtocol } from "../src/JsonHubProtocol"; import { VerifyLogger } from "./Common"; import { TestConnection } from "./TestConnection"; +import { TestHttpClient } from "./TestHttpClient"; +import { TestEvent, TestMessageEvent, TestWebSocket } from "./TestWebSocket"; import { PromiseSource } from "./Utils"; describe("auto reconnect", () => { @@ -785,4 +789,93 @@ describe("auto reconnect", () => { } }); }); + + it("can be stopped while restarting the underlying connection and negotiate throws", async () => { + await VerifyLogger.run(async (logger) => { + let onreconnectingCount = 0; + let onreconnectedCount = 0; + let closeCount = 0; + + const nextRetryDelayCalledPromise = new PromiseSource(); + + const defaultConnectionId = "abc123"; + const defaultConnectionToken = "123abc"; + const defaultNegotiateResponse: INegotiateResponse = { + availableTransports: [ + { transport: "WebSockets", transferFormats: ["Text", "Binary"] }, + { transport: "ServerSentEvents", transferFormats: ["Text"] }, + { transport: "LongPolling", transferFormats: ["Text", "Binary"] }, + ], + connectionId: defaultConnectionId, + connectionToken: defaultConnectionToken, + negotiateVersion: 1, + }; + + const startStarted = new PromiseSource(); + let negotiateCount = 0; + + const options: IHttpConnectionOptions = { + WebSocket: TestWebSocket, + httpClient: new TestHttpClient() + .on("POST", async () => { + ++negotiateCount; + if (negotiateCount === 1) { + return defaultNegotiateResponse; + } + startStarted.resolve(); + return Promise.reject("Error with negotiate"); + }) + .on("GET", () => ""), + logger, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + const hubConnection = HubConnection.create(connection, logger, new JsonHubProtocol(), { + nextRetryDelayInMilliseconds() { + nextRetryDelayCalledPromise.resolve(); + return 0; + }, + }); + + hubConnection.onreconnecting(() => { + onreconnectingCount++; + }); + + hubConnection.onreconnected(() => { + onreconnectedCount++; + }); + + hubConnection.onclose(() => { + closeCount++; + }); + + TestWebSocket.webSocketSet = new PromiseSource(); + const startPromise = hubConnection.start(); + await TestWebSocket.webSocketSet; + await TestWebSocket.webSocket.openSet; + TestWebSocket.webSocket.onopen(new TestEvent()); + TestWebSocket.webSocket.onmessage(new TestMessageEvent("{}\x1e")); + + await startPromise; + TestWebSocket.webSocket.close(); + TestWebSocket.webSocketSet = new PromiseSource(); + + await nextRetryDelayCalledPromise; + + expect(hubConnection.state).toBe(HubConnectionState.Reconnecting); + expect(onreconnectingCount).toBe(1); + expect(onreconnectedCount).toBe(0); + expect(closeCount).toBe(0); + + await startStarted; + await hubConnection.stop(); + + expect(hubConnection.state).toBe(HubConnectionState.Disconnected); + expect(onreconnectingCount).toBe(1); + expect(onreconnectedCount).toBe(0); + expect(closeCount).toBe(1); + }, + "Failed to complete negotiation with the server: Error with negotiate", + "Failed to start the connection: Error with negotiate"); + }); }); diff --git a/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts b/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts index 1a70413d2382..0269b8e658b8 100644 --- a/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts +++ b/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts @@ -219,3 +219,57 @@ export class TestCloseEvent { public CAPTURING_PHASE: number = 0; public NONE: number = 0; } + +export class TestMessageEvent implements MessageEvent { + constructor(data: any) { + this.data = data; + } + public data: any; + public lastEventId: string = ""; + public origin: string = ""; + public ports: MessagePort[] = []; + public source: Window | null = null; + public composed: boolean = false; + public composedPath(): EventTarget[]; + public composedPath(): any[] { + throw new Error("Method not implemented."); + } + public code: number = 0; + public reason: string = ""; + public wasClean: boolean = false; + public initMessageEvent(typeArg: string, canBubbleArg: boolean, cancelableArg: boolean, data: any, origin: string, lastEventId: string): void { + throw new Error("Method not implemented."); + } + public bubbles: boolean = false; + public cancelBubble: boolean = false; + public cancelable: boolean = false; + public currentTarget!: EventTarget; + public defaultPrevented: boolean = false; + public eventPhase: number = 0; + public isTrusted: boolean = false; + public returnValue: boolean = false; + public scoped: boolean = false; + public srcElement!: Element | null; + public target!: EventTarget; + public timeStamp: number = 0; + public type: string = ""; + public deepPath(): EventTarget[] { + throw new Error("Method not implemented."); + } + public initEvent(type: string, bubbles?: boolean | undefined, cancelable?: boolean | undefined): void { + throw new Error("Method not implemented."); + } + public preventDefault(): void { + throw new Error("Method not implemented."); + } + public stopImmediatePropagation(): void { + throw new Error("Method not implemented."); + } + public stopPropagation(): void { + throw new Error("Method not implemented."); + } + public AT_TARGET: number = 0; + public BUBBLING_PHASE: number = 0; + public CAPTURING_PHASE: number = 0; + public NONE: number = 0; +} From 13a34c6c063120468324d15237bfcb562c878171 Mon Sep 17 00:00:00 2001 From: John Luo Date: Fri, 9 Apr 2021 14:54:01 -0700 Subject: [PATCH 3/6] Set locale for ubuntu 18.04 (#31649) --- .azure/pipelines/ci.yml | 3 +++ .azure/pipelines/jobs/default-build.yml | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/.azure/pipelines/ci.yml b/.azure/pipelines/ci.yml index 13da1c1429c7..693e0f5417e3 100644 --- a/.azure/pipelines/ci.yml +++ b/.azure/pipelines/ci.yml @@ -701,6 +701,9 @@ stages: pool: vmImage: 'ubuntu-18.04' variables: + LC_ALL: 'en_US.UTF-8' + LANG: 'en_US.UTF-8' + LANGUAGE: 'en_US.UTF-8' DotNetCoreSdkDir: $(Agent.ToolsDirectory)/dotnet # This isn't needed in the path because build does not need to _use_ global tools. DOTNET_CLI_HOME: $(System.DefaultWorkingDirectory) diff --git a/.azure/pipelines/jobs/default-build.yml b/.azure/pipelines/jobs/default-build.yml index e939243a85f6..0218017a5102 100644 --- a/.azure/pipelines/jobs/default-build.yml +++ b/.azure/pipelines/jobs/default-build.yml @@ -116,6 +116,10 @@ jobs: - DOTNET_CLI_HOME: $(System.DefaultWorkingDirectory) - DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true - TeamName: AspNetCore + - ${{ if eq(parameters.agentOs, 'Linux') }}: + - LC_ALL: 'en_US.UTF-8' + - LANG: 'en_US.UTF-8' + - LANGUAGE: 'en_US.UTF-8' - ${{ if and(eq(parameters.installJdk, 'true'), eq(parameters.agentOs, 'Windows')) }}: - JAVA_HOME: $(Agent.BuildDirectory)\.tools\jdk\win-x64 - ${{ if or(ne(parameters.codeSign, true), ne(variables['System.TeamProject'], 'internal')) }}: From 41caa92bd80008786d27fe782dc8d560ac8d01c6 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 12 Apr 2021 08:59:01 -0700 Subject: [PATCH 4/6] [SignalR TS] Set keep alive timer in receive loop (#31300) (#31626) --- .../clients/ts/signalr/src/HubConnection.ts | 44 ++++++++++++++----- .../ts/signalr/tests/HubConnection.test.ts | 9 +++- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/HubConnection.ts b/src/SignalR/clients/ts/signalr/src/HubConnection.ts index 5d55bedf03af..49e9ae2eb5a7 100644 --- a/src/SignalR/clients/ts/signalr/src/HubConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HubConnection.ts @@ -54,6 +54,7 @@ export class HubConnection { private connectionStarted: boolean; private startPromise?: Promise; private stopPromise?: Promise; + private nextKeepAlive: number = 0; // The type of these a) doesn't matter and b) varies when building in browser and node contexts // Since we're building the WebPack bundle directly from the TypeScript, this matters (previously @@ -73,6 +74,8 @@ export class HubConnection { * * The default value is 15,000 milliseconds (15 seconds). * Allows the server to detect hard disconnects (like when a client unplugs their computer). + * The ping will happen at most as often as the server pings. + * If the server pings every 5 seconds, a value lower than 5 will ping every 5 seconds. */ public keepAliveIntervalInMilliseconds: number; @@ -599,24 +602,42 @@ export class HubConnection { } private resetKeepAliveInterval() { + if (this.connection.features.inherentKeepAlive) { + return; + } + + // Set the time we want the next keep alive to be sent + // Timer will be setup on next message receive + this.nextKeepAlive = new Date().getTime() + this.keepAliveIntervalInMilliseconds; + this.cleanupPingTimer(); - this.pingServerHandle = setTimeout(async () => { - if (this.connectionState === HubConnectionState.Connected) { - try { - await this.sendMessage(this.cachedPingMessage); - } catch { - // We don't care about the error. It should be seen elsewhere in the client. - // The connection is probably in a bad or closed state now, cleanup the timer so it stops triggering - this.cleanupPingTimer(); - } - } - }, this.keepAliveIntervalInMilliseconds); } private resetTimeoutPeriod() { if (!this.connection.features || !this.connection.features.inherentKeepAlive) { // Set the timeout timer this.timeoutHandle = setTimeout(() => this.serverTimeout(), this.serverTimeoutInMilliseconds); + + // Set keepAlive timer if there isn't one + if (this.pingServerHandle === undefined) { + let nextPing = this.nextKeepAlive - new Date().getTime(); + if (nextPing < 0) { + nextPing = 0; + } + + // The timer needs to be set from a networking callback to avoid Chrome timer throttling from causing timers to run once a minute + this.pingServerHandle = setTimeout(async () => { + if (this.connectionState === HubConnectionState.Connected) { + try { + await this.sendMessage(this.cachedPingMessage); + } catch { + // We don't care about the error. It should be seen elsewhere in the client. + // The connection is probably in a bad or closed state now, cleanup the timer so it stops triggering + this.cleanupPingTimer(); + } + } + }, nextPing); + } } } @@ -807,6 +828,7 @@ export class HubConnection { private cleanupPingTimer(): void { if (this.pingServerHandle) { clearTimeout(this.pingServerHandle); + this.pingServerHandle = undefined; } } diff --git a/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts index 1cb682cabb59..b62d3516c8b8 100644 --- a/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts @@ -109,7 +109,7 @@ describe("HubConnection", () => { }); describe("ping", () => { - it("automatically sends multiple pings", async () => { + it("sends pings when receiving pings", async () => { await VerifyLogger.run(async (logger) => { const connection = new TestConnection(); const hubConnection = createHubConnection(connection, logger); @@ -118,8 +118,15 @@ describe("HubConnection", () => { try { await hubConnection.start(); + + const pingInterval = setInterval(async () => { + await connection.receive({ type: MessageType.Ping }); + }, 5); + await delayUntil(500); + clearInterval(pingInterval); + const numPings = connection.sentData.filter((s) => JSON.parse(s).type === MessageType.Ping).length; expect(numPings).toBeGreaterThanOrEqual(2); } finally { From 22cdfd666d7b0b2d57f283b960efd7d5b4a54d2b Mon Sep 17 00:00:00 2001 From: John Luo Date: Mon, 12 Apr 2021 14:07:22 -0700 Subject: [PATCH 5/6] Update docker image to 18.04 (#31722) --- eng/docker/ubuntu-alpine37.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/docker/ubuntu-alpine37.Dockerfile b/eng/docker/ubuntu-alpine37.Dockerfile index 859098f04efe..7cc731257db5 100644 --- a/eng/docker/ubuntu-alpine37.Dockerfile +++ b/eng/docker/ubuntu-alpine37.Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-16.04-cross-arm64-alpine10fcdcf-20190208200917 +FROM mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-cross-arm64-alpine-85814b7-20200327151156 ARG USER ARG USER_ID ARG GROUP_ID From 9a0ddaa71529bfe2da4b793ec0ec01810cf5fca5 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 13 Apr 2021 14:08:59 -0700 Subject: [PATCH 6/6] Update TestWebSocket.ts --- .../clients/ts/signalr/tests/TestWebSocket.ts | 54 ------------------- 1 file changed, 54 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts b/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts index 529185f3d079..dc1e1a01aa33 100644 --- a/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts +++ b/src/SignalR/clients/ts/signalr/tests/TestWebSocket.ts @@ -285,57 +285,3 @@ export class TestMessageEvent implements MessageEvent { public CAPTURING_PHASE: number = 0; public NONE: number = 0; } - -export class TestMessageEvent implements MessageEvent { - constructor(data: any) { - this.data = data; - } - public data: any; - public lastEventId: string = ""; - public origin: string = ""; - public ports: MessagePort[] = []; - public source: Window | null = null; - public composed: boolean = false; - public composedPath(): EventTarget[]; - public composedPath(): any[] { - throw new Error("Method not implemented."); - } - public code: number = 0; - public reason: string = ""; - public wasClean: boolean = false; - public initMessageEvent(typeArg: string, canBubbleArg: boolean, cancelableArg: boolean, data: any, origin: string, lastEventId: string): void { - throw new Error("Method not implemented."); - } - public bubbles: boolean = false; - public cancelBubble: boolean = false; - public cancelable: boolean = false; - public currentTarget!: EventTarget; - public defaultPrevented: boolean = false; - public eventPhase: number = 0; - public isTrusted: boolean = false; - public returnValue: boolean = false; - public scoped: boolean = false; - public srcElement!: Element | null; - public target!: EventTarget; - public timeStamp: number = 0; - public type: string = ""; - public deepPath(): EventTarget[] { - throw new Error("Method not implemented."); - } - public initEvent(type: string, bubbles?: boolean | undefined, cancelable?: boolean | undefined): void { - throw new Error("Method not implemented."); - } - public preventDefault(): void { - throw new Error("Method not implemented."); - } - public stopImmediatePropagation(): void { - throw new Error("Method not implemented."); - } - public stopPropagation(): void { - throw new Error("Method not implemented."); - } - public AT_TARGET: number = 0; - public BUBBLING_PHASE: number = 0; - public CAPTURING_PHASE: number = 0; - public NONE: number = 0; -}