Skip to content

Add option to restrict the maximum hub message size #8135

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 3 commits into from
Mar 3, 2019

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented Mar 3, 2019

  • This change moves the limit checking from the transport layer to the protocol parsing layer. One nice side effect is that it gives us better control over error handling.

Fixes #8121 and #5307

Notes:

  • The transport limits just slow down the client, but it doesn't prevent extra buffering. See the issue for more details.

- This change moves the limit checking from the transport layer to the protocol parsing layer. One nice side effect is that it gives us better control over error handling.
@@ -72,7 +72,7 @@ public static IServiceProvider CreateServiceProvider(Action<ServiceCollection> a
return services.BuildServiceProvider();
}

public static Connections.ConnectionHandler GetHubConnectionHandler(Type hubType, Action<ServiceCollection> addServices = null, ILoggerFactory loggerFactory = null)
public static Connections.ConnectionHandler GetHubConnectionHandler(Type hubType, ILoggerFactory loggerFactory = null, Action<ServiceCollection> addServices = null)
Copy link
Member Author

Choose a reason for hiding this comment

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

Lambdas should be last.

@Eilon Eilon added the area-signalr Includes: SignalR clients and servers label Mar 3, 2019
}

[Fact]
public async Task ManyHubMessagesUnderTheMessageSizeButConfiguredWithMax()
Copy link
Member

Choose a reason for hiding this comment

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

good test 👍

- Rename the property to MaximumReceiveMessageSize
client.Connection.Application.Output.Write(payload3);
await client.Connection.Application.Output.FlushAsync();

// 2 invocations should be processed
Copy link
Member

Choose a reason for hiding this comment

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

nit: 3

@@ -36,6 +36,11 @@ public class HubOptions
/// </summary>
public IList<string> SupportedProtocols { get; set; } = null;

/// <summary>
/// Gets or sets the maximum message size of a single incoming hub message. The default is 32KB.
Copy link
Member

Choose a reason for hiding this comment

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

"Null means no maximum message size" or is that implicit with nullable types

@davidfowl davidfowl merged commit 9cb1185 into master Mar 3, 2019
@davidfowl davidfowl deleted the davidfowl/message-limits branch March 4, 2019 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants