Repository navigation
feat(SignalR): add Dispatch/Connection services for wasm mode - #1252
Merged
Merged
Conversation
Reviewer's GuideAdds end-to-end SignalR support for BootstrapBlazor, including a reconnecting shared client connection, generic dispatch and connection-monitoring services, server Hub handlers with opt-in authorization/configuration, and service/endpoint registration; the existing IP middleware is extracted into a dedicated handler. Sequence diagram for SignalR dispatch and subscriptionsequenceDiagram
participant App as Blazor app
participant Client as DefaultDispatchService
participant Connection as SignalRConnection
participant Hub as BootstrapBlazorHub
participant Server as DefaultDispatchService
App->>Client: SubscribeAsync(callback)
Client->>Connection: SubscribeAsync(entryType, ReceiveAsync)
Connection->>Hub: Subscribe(entryType)
Hub-->>Connection: true
Connection-->>Client: subscription ready
App->>Client: DispatchAsync(entry)
Client->>Connection: DispatchAsync(entryType, entry)
Connection->>Hub: Dispatch(entryType, entry)
Hub->>Server: DispatchAsync(entry)
Hub-->>Connection: dispatch accepted
Connection-->>Client: dispatch completed
Client-->>App: invoke local callbacks
Sequence diagram for connection heartbeat and snapshot pollingsequenceDiagram
participant Client as DefaultConnectionService
participant Connection as SignalRConnection
participant Hub as BootstrapBlazorHub
participant Handler as ConnectionHandler
loop every 3 seconds
Client->>Connection: InvokeAsync(GetConnectionSnapshot)
Connection->>Hub: GetConnectionSnapshot()
Hub->>Handler: GetSnapshotAsync()
Handler-->>Hub: ConnectionSnapshot or null
Hub-->>Connection: snapshot result
Connection-->>Client: update local snapshot
end
Client->>Connection: InvokeAsync(Heartbeat, client)
Connection->>Hub: Heartbeat(client)
Hub->>Handler: ReportHeartbeatAsync(client)
Handler-->>Hub: true or false
Hub-->>Connection: heartbeat result
Flow diagram for SignalR reconnect subscription restorationflowchart TD
Disconnected[SignalR connection disconnected] --> Reconnect[Automatic reconnect]
Reconnect --> Restored[RestoreSubscriptionsAsync]
Restored --> Subscribe[SubscribeCoreAsync]
Subscribe --> Connected[Subscriptions restored]
Subscribe --> Retry[Transient failure]
Retry --> Delay[Wait 3 seconds]
Delay --> Subscribe
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![bb-auto[bot]](https://avatars.githubusercontent.com/in/287253?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/middleware/BootstrapBlazor.Middleware/ServiceCollectionExtensions.cs" line_range="47" />
<code_context>
+ /// </summary>
+ /// <param name="endpoints"></param>
+ /// <param name="pattern"></param>
+ public static HubEndpointConventionBuilder MapBootstrapBlazorHub(this IEndpointRouteBuilder endpoints, string pattern = "/bb.hub")
+ => endpoints.MapHub<BootstrapBlazorHub>(pattern);
+}
</code_context>
<issue_to_address>
**Cross-site pages can hijack Hub sessions**
When the host uses cookie authentication for this Hub and a victim visits an attacker-controlled site while the Hub exposes sensitive operations, `MapBootstrapBlazorHub` adds no WebSocket origin validation. ASP.NET Core endpoint authorization alone does not prevent a browser from opening a cross-site WebSocket with the victim’s cookie, allowing attacker-controlled JavaScript to invoke Hub methods and read their responses as that victim.
Validate the WebSocket handshake Origin against an explicit allowlist for this Hub; do not rely on CORS or endpoint authorization alone to prevent cross-site WebSocket requests.
</issue_to_address>
### Comment 2
<location path="src/middleware/BootstrapBlazor.Middleware/ConnectionHandler.cs" line_range="57-72" />
<code_context>
+ return null;
+ }
+
+ if (!string.IsNullOrEmpty(options.ConnectionSnapshotAuthorizationPolicy))
+ {
+ // 如果设置了授权策略则进行授权检查
+ var authorization = services.GetRequiredService<IAuthorizationService>();
+ var result = await authorization.AuthorizeAsync(context.User ?? new ClaimsPrincipal(), null, options.ConnectionSnapshotAuthorizationPolicy);
+ if (!result.Succeeded)
+ {
+ return null;
+ }
+ }
+
+ var service = services.GetRequiredService<IConnectionService>();
+ return new ConnectionSnapshot
+ {
+ Count = service.Count,
+ Connections = service.Connections
+ };
+ }
</code_context>
<issue_to_address>
**Online client details are exposed**
When the host enables connection snapshots, leaves the snapshot authorization policy unset, and maps the Hub without `RequireAuthorization`, `GetSnapshotAsync` skips authorization when no snapshot policy is configured and returns the full connection collection. Since `MapBootstrapBlazorHub` maps the Hub without requiring authorization, anonymous callers can retrieve online client details, including IP addresses and usernames.
Require an authorization policy for snapshot queries, or require Hub endpoint authorization whenever snapshots are enabled.
Also at `src/middleware/BootstrapBlazor.Middleware/ServiceCollectionExtensions.cs:47`.
</issue_to_address>
### Comment 3
<location path="src/middleware/BootstrapBlazor.Middleware/ConnectionHandler.cs" line_range="27-28" />
<code_context>
+ return false;
+ }
+
+ if (client == null || string.IsNullOrWhiteSpace(client.Id) || client.Id.Length > 128 ||
+ !Enum.IsDefined(typeof(WebClientDeviceType), client.Device))
+ {
+ throw new HubException("Invalid connection heartbeat.");
</code_context>
<issue_to_address>
**Client IDs overwrite other records**
When two clients submit the same `ClientInfo.Id`, `ReportHeartbeatAsync` accepts any nonblank client-supplied `Id` and passes that `ClientInfo` to `AddOrUpdateAsync`; clients that reuse or deliberately choose another ID overwrite that connection record, so stored connection data is attributed to the wrong client.
Derive the stored identity from a server-controlled identifier, or reject IDs already associated with another connection or owner.
</issue_to_address>
### Comment 4
<location path="src/extensions/BootstrapBlazor.SignalR.Client/DefaultDispatchService.cs" line_range="74-93" />
<code_context>
+ try
+ {
+ var initialize = true;
+ while (true)
+ {
+ await _gate.WaitAsync(token);
+ try
+ {
+ if (initialize)
+ {
+ _pendingSubscriptions.UnionWith(_subscriptions.Keys);
+ initialize = false;
+ }
+ if (_connection.State != HubConnectionState.Connected)
+ {
</code_context>
<issue_to_address>
**Subscriptions never return**
When the Hub is unavailable or returns a persistent Hub error during subscription, `SubscribeCoreAsync` wraps every `HubException` as `IOException`, `IsTransientFailure` retries it, and the subscription loop has no retry limit, so `SubscribeAsync` waits indefinitely.
Preserve permanent Hub errors as non-transient and bound retries for transient subscription failures.
Also at `src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs:49`, `src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs:340-342`.
</issue_to_address>
### Comment 5
<location path="src/middleware/BootstrapBlazor.Middleware/DefaultDispatchService.cs" line_range="36" />
<code_context>
+ var clients = excludedConnectionId == null
+ ? hub.Clients.Group(_entryType)
+ : hub.Clients.GroupExcept(_entryType, [excludedConnectionId]);
+ await clients.SendAsync("Receive", _entryType, JsonSerializer.SerializeToElement(entry, _serializerOptions), cancellationToken);
+ }
+
</code_context>
<issue_to_address>
**Local subscribers miss dispatches**
When the SignalR send or its backing transport fails, `DispatchAsync` awaits `clients.SendAsync` before invoking local callbacks, so a send exception skips the callback loop and local subscribers miss the entry.
Invoke local callbacks even when the SignalR send fails, then report the send failure.
</issue_to_address>
### Comment 6
<location path="src/middleware/BootstrapBlazor.Middleware/DefaultDispatchService.cs" line_range="36" />
<code_context>
+ var clients = excludedConnectionId == null
+ ? hub.Clients.Group(_entryType)
+ : hub.Clients.GroupExcept(_entryType, [excludedConnectionId]);
+ await clients.SendAsync("Receive", _entryType, JsonSerializer.SerializeToElement(entry, _serializerOptions), cancellationToken);
+ }
+
</code_context>
<issue_to_address>
**Retried dispatch is delivered twice**
When a client retries a dispatch after an ambiguous transport failure, `Dispatch` runs the publish callback again for each invocation, and this broadcast path has no idempotency check. When a client retries after the server handled the first invocation but its response was lost, remote subscribers and server-side callbacks process the entry twice.
Deduplicate dispatches by a stable entry ID before broadcasting or invoking callbacks, or document that callers must not retry ambiguous sends.
</issue_to_address>
### Comment 7
<location path="src/middleware/BootstrapBlazor.Middleware/ConnectionHandler.cs" line_range="71-72" />
<code_context>
+ var service = services.GetRequiredService<IConnectionService>();
+ return new ConnectionSnapshot
+ {
+ Count = service.Count,
+ Connections = service.Connections
+ };
+ }
</code_context>
<issue_to_address>
**Snapshot totals become inconsistent**
When a heartbeat changes the connection service between the separate property reads, `GetSnapshotAsync` reads `Count` and `Connections` separately, so the returned count can disagree with the returned collection and callers see inconsistent online-connection data.
Read the count and connections together from one atomic connection-service snapshot.
</issue_to_address>
### Comment 8
<location path="src/middleware/BootstrapBlazor.Middleware/BootstrapBlazorHubOptions.cs" line_range="66-67" />
<code_context>
+ throw new HubException("Invalid dispatch payload.");
+ }
+
+ var service = (DefaultDispatchService<TEntry>)services.GetRequiredService<IDispatchService<TEntry>>();
+ await service.DispatchAsync(entry, connectionId, cancellationToken);
+ }
+}
</code_context>
<issue_to_address>
**Custom dispatch services fail**
When an application replaces `IDispatchService<TEntry>` and enables client publishing for that entry type, `PublishAsync<TEntry>` casts the resolved `IDispatchService<TEntry>` to `DefaultDispatchService<TEntry>`, so a custom implementation causes `InvalidCastException` and the Hub publish fails.
Dispatch through the resolved `IDispatchService<TEntry>` without casting it to the default implementation.
</issue_to_address>
### Comment 9
<location path="src/extensions/BootstrapBlazor.SignalR.Client/SignalRConnection.cs" line_range="64" />
<code_context>
+ {
+ _logger = logger;
+ _connection = new HubConnectionBuilder()
+ .WithUrl(navigation.ToAbsoluteUri(hubPath))
+ .WithAutomaticReconnect(new ConnectionRetryPolicy())
+ .Build();
</code_context>
<issue_to_address>
**Bearer-authenticated hubs cannot connect**
When the mapped Hub requires a bearer token rather than ambient cookie authentication, `WithUrl` receives no `HttpConnectionOptions` configuration, so callers cannot provide an `AccessTokenProvider` through this extension; a Hub protected by bearer-token authorization rejects the connection and its dispatch, heartbeat, and snapshot features remain unavailable.
Add an overload or configuration callback that lets callers configure the SignalR `HttpConnectionOptions`, including `AccessTokenProvider`.
</issue_to_address>
Comment on lines
+27
to
+28
| if (client == null || string.IsNullOrWhiteSpace(client.Id) || client.Id.Length > 128 || | ||
| !Enum.IsDefined(typeof(WebClientDeviceType), client.Device)) |
There was a problem hiding this comment.
🟡 Medium · Client IDs overwrite other records
When two clients submit the same ClientInfo.Id, ReportHeartbeatAsync accepts any nonblank client-supplied Id and passes that ClientInfo to AddOrUpdateAsync; clients that reuse or deliberately choose another ID overwrite that connection record, so stored connection data is attributed to the wrong client.
Derive the stored identity from a server-controlled identifier, or reject IDs already associated with another connection or owner.
Prompt for AI agents
In `src/middleware/BootstrapBlazor.Middleware/ConnectionHandler.cs` at lines 27-28:
**Client IDs overwrite other records**
When two clients submit the same `ClientInfo.Id`, `ReportHeartbeatAsync` accepts any nonblank client-supplied `Id` and passes that `ClientInfo` to `AddOrUpdateAsync`; clients that reuse or deliberately choose another ID overwrite that connection record, so stored connection data is attributed to the wrong client.
Derive the stored identity from a server-controlled identifier, or reject IDs already associated with another connection or owner.
ArgoZhang
changed the title
ArgoZhang
force-pushed
the
dev-middleware
branch
from
October 27, 2026 01:22
35f8973 to
0e4e228
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #1254
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable WebAssembly applications to use shared SignalR-based dispatch and connection services while standardizing client asset loading and lifecycle management.
New Features:
Bug Fixes:
Enhancements:
Build: