Skip to content

Commit 1c23575

Browse files
authored
Enforce Chunk Size on Incoming Data Stream (dotnet#35110)
* Enforce Chunk Size on Incoming Data Stream Realized we were missing this explicit check for the JS->.NET Streaming. Regardless, the SignalR message size limit should serve as a good failsafe in this case. * Fix typo
1 parent 9b6afb4 commit 1c23575

File tree

5 files changed

+31
-9
lines changed

5 files changed

+31
-9
lines changed

src/Components/Server/src/Circuits/RemoteJSDataStream.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ internal sealed class RemoteJSDataStream : Stream
1515
private readonly RemoteJSRuntime _runtime;
1616
private readonly long _streamId;
1717
private readonly long _totalLength;
18+
private readonly int _chunkSize;
1819
private readonly TimeSpan _jsInteropDefaultCallTimeout;
1920
private readonly CancellationToken _streamCancellationToken;
2021
private readonly Stream _pipeReaderStream;
@@ -49,11 +50,11 @@ public static async ValueTask<RemoteJSDataStream> CreateRemoteJSDataStreamAsync(
4950
// transfer per chunk with a 1 kb message size.
5051
// Additionally, to maintain interactivity, we put an upper limit of 50 kb on the message size.
5152
var chunkSize = signalRMaximumIncomingBytes > 1024 ?
52-
Math.Min(signalRMaximumIncomingBytes, 50*1024) - 512 :
53+
(int)Math.Min(signalRMaximumIncomingBytes, 50*1024) - 512 :
5354
throw new ArgumentException($"SignalR MaximumIncomingBytes must be at least 1 kb.");
5455

5556
var streamId = runtime.RemoteJSDataStreamNextInstanceId++;
56-
var remoteJSDataStream = new RemoteJSDataStream(runtime, streamId, totalLength, jsInteropDefaultCallTimeout, cancellationToken);
57+
var remoteJSDataStream = new RemoteJSDataStream(runtime, streamId, totalLength, chunkSize, jsInteropDefaultCallTimeout, cancellationToken);
5758
await runtime.InvokeVoidAsync("Blazor._internal.sendJSDataStream", jsStreamReference, streamId, chunkSize);
5859
return remoteJSDataStream;
5960
}
@@ -62,12 +63,14 @@ private RemoteJSDataStream(
6263
RemoteJSRuntime runtime,
6364
long streamId,
6465
long totalLength,
66+
int chunkSize,
6567
TimeSpan jsInteropDefaultCallTimeout,
6668
CancellationToken cancellationToken)
6769
{
6870
_runtime = runtime;
6971
_streamId = streamId;
7072
_totalLength = totalLength;
73+
_chunkSize = chunkSize;
7174
_jsInteropDefaultCallTimeout = jsInteropDefaultCallTimeout;
7275
_streamCancellationToken = cancellationToken;
7376

@@ -107,6 +110,11 @@ private async Task<bool> ReceiveData(long chunkId, byte[] chunk, string error)
107110
throw new EndOfStreamException("The incoming data chunk cannot be empty.");
108111
}
109112

113+
if (chunk.Length > _chunkSize)
114+
{
115+
throw new EndOfStreamException("The incoming data chunk exceeded the permitted length.");
116+
}
117+
110118
_bytesRead += chunk.Length;
111119

112120
if (_bytesRead > _totalLength)

src/Components/Server/src/ComponentHub.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,6 @@ private static class Log
361361
private static readonly Action<ILogger, string, Exception> _invalidCircuitId =
362362
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(8, "InvalidCircuitId"), "ConnectAsync received an invalid circuit id '{CircuitIdSecret}'");
363363

364-
private static readonly Action<ILogger, Exception> _sendingDotNetStreamFailed =
365-
LoggerMessage.Define(LogLevel.Debug, new EventId(9, "SendingDotNetStreamFailed"), "Sending the .NET stream data to JS failed");
366-
367364
public static void ReceivedConfirmationForBatch(ILogger logger, long batchId) => _receivedConfirmationForBatch(logger, batchId, null);
368365

369366
public static void CircuitAlreadyInitialized(ILogger logger, CircuitId circuitId) => _circuitAlreadyInitialized(logger, circuitId, null);
@@ -376,8 +373,6 @@ private static class Log
376373

377374
public static void CircuitInitializationFailed(ILogger logger, Exception exception) => _circuitInitializationFailed(logger, exception);
378375

379-
public static void SendingDotNetStreamFailed(ILogger logger, Exception exception) => _sendingDotNetStreamFailed(logger, exception);
380-
381376
public static void CreatedCircuit(ILogger logger, CircuitId circuitId, string circuitSecret, string connectionId)
382377
{
383378
// Redact the secret unless tracing is on.

src/Components/Server/test/Circuits/RemoteJSDataStreamTest.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,25 @@ public async Task ReceiveData_WithZeroLengthChunk()
140140
Assert.Equal("The incoming data chunk cannot be empty.", ex.Message);
141141
}
142142

143+
[Fact]
144+
public async Task ReceiveData_WithLargerChunksThanPermitted()
145+
{
146+
// Arrange
147+
var jsRuntime = new TestRemoteJSRuntime(Options.Create(new CircuitOptions()), Options.Create(new HubOptions()), Mock.Of<ILogger<RemoteJSRuntime>>());
148+
var remoteJSDataStream = await CreateRemoteJSDataStreamAsync(jsRuntime);
149+
var streamId = GetStreamId(remoteJSDataStream, jsRuntime);
150+
var chunk = new byte[50_000]; // more than the 32k maximum chunk size
151+
152+
// Act & Assert 1
153+
var ex = await Assert.ThrowsAsync<EndOfStreamException>(async () => await RemoteJSDataStream.ReceiveData(jsRuntime, streamId, chunkId: 0, chunk, error: null).DefaultTimeout());
154+
Assert.Equal("The incoming data chunk exceeded the permitted length.", ex.Message);
155+
156+
// Act & Assert 2
157+
using var mem = new MemoryStream();
158+
ex = await Assert.ThrowsAsync<EndOfStreamException>(async () => await remoteJSDataStream.CopyToAsync(mem).DefaultTimeout());
159+
Assert.Equal("The incoming data chunk exceeded the permitted length.", ex.Message);
160+
}
161+
143162
[Fact]
144163
public async Task ReceiveData_ProvidedWithMoreBytesThanRemaining()
145164
{

src/Components/Web.JS/dist/Release/blazor.server.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Components/Web.JS/dist/Release/blazor.webview.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)