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

[automated] Merge branch 'release/3.1' => 'main' #31656

Merged
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions .azure/pipelines/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions .azure/pipelines/jobs/default-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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')) }}:
Expand Down
2 changes: 1 addition & 1 deletion eng/docker/ubuntu-alpine37.Dockerfile
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,15 @@ protected void AddAndCheckConsumedBytes(long consumedBytes)

protected ValueTask<ReadResult> StartTimingReadAsync(ValueTask<ReadResult> readAwaitable, CancellationToken cancellationToken)
{

if (!readAwaitable.IsCompleted && _timingEnabled)
if (!readAwaitable.IsCompleted)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@halter73 do these changes already exist in main?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. #31543

Interesting that it still shows up in the diff when the PR is targeting main.

{
TryProduceContinue();

_backpressure = true;
_context.TimeoutControl.StartTimingRead();
if (_timingEnabled)
{
_backpressure = true;
_context.TimeoutControl.StartTimingRead();
}
}

return readAwaitable;
Expand Down
36 changes: 36 additions & 0 deletions src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
6 changes: 4 additions & 2 deletions src/SignalR/clients/ts/signalr/src/HttpConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class HttpConnection implements IConnection {
private transport?: ITransport;
private startInternalPromise?: Promise<void>;
private stopPromise?: Promise<void>;
private stopPromiseResolver!: (value?: PromiseLike<void>) => void;
private stopPromiseResolver: (value?: PromiseLike<void>) => void = () => {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrennanConroy do we need to bring these changes over from 3.1 -> main?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should already be in main

private stopError?: Error;
private accessTokenFactory?: () => string | Promise<string>;
private sendQueue?: TransportSendQueue;
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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);
}
}
Expand Down
50 changes: 38 additions & 12 deletions src/SignalR/clients/ts/signalr/src/HubConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class HubConnection {
private connectionStarted: boolean;
private startPromise?: Promise<void>;
private stopPromise?: Promise<void>;
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
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -762,7 +783,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;
}

Expand Down Expand Up @@ -803,6 +828,7 @@ export class HubConnection {
private cleanupPingTimer(): void {
if (this.pingServerHandle) {
clearTimeout(this.pingServerHandle);
this.pingServerHandle = undefined;
}
}

Expand Down
19 changes: 13 additions & 6 deletions src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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");
});
});
9 changes: 8 additions & 1 deletion src/SignalR/clients/ts/signalr/tests/HubConnection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand Down
Loading