Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
6 changes: 0 additions & 6 deletions src/Components/Samples/BlazorServerApp/Startup.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.HttpsPolicy;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Buffers;
using System.Collections.Generic;
using System.IO;
using MessagePack;
using Microsoft.AspNetCore.SignalR.Protocol;
Expand Down Expand Up @@ -34,6 +36,21 @@ protected override object DeserializeObject(ref MessagePackReader reader, Type t
{
return reader.ReadSingle();
}
else if (type == typeof(byte[][]))
{
if (reader.IsNil)
{
return null;
}

var length = reader.ReadArrayHeader();
var result = new List<byte[]>();
for (var i = 0; i < length; i++)
{
result.Add(reader.ReadBytes().GetValueOrDefault().ToArray());
}
return result.ToArray();
Copy link
Member

Choose a reason for hiding this comment

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

I notice this logic is doing all the allocations for the byte arrays up front. That might be the only possible way to do it, but I'm asking to be sure.

Reason

There's no maximum number of entries in this byte[][], other than the constraint imposed by the SignalR max message size. So as it stands, if the SignalR max message size was (say) 32KB, and if MessagePack could represent an empty array in (say) 4 bytes, then this logic could have the server perform 8000-ish heap allocations on each incoming message, whether or not there even is any JS invokable endpoint that accepts binary data.

I suppose that's not much different from a JSON payload that contains 8000 very short distinct strings, so maybe it's nothing to worry about.

Possible alternative

If this logic returned a List<ReadOnlySequence<byte>> (or similar) instead, would that avoid the per-entry allocations? I guess it depends on how MessagePack's ReadBytes is implemented. But it we were able to do this and pass it through to the JSON deserializer, then the JSON deserializer would only need to convert the ReadOnlySequence<byte> into a byte[] in the case where it matches an actual byte[] property declared on a .NET type. So a mischief-maker wouldn't be able to force any more allocations than the target data structure declares (except if the target data structure contains something like a List).

I know it really might not make any difference, given that people will sometimes declare List-type structures on their target types. But if it's no more difficult to delay the allocations instead of doing them up front, it's worth considering.

Copy link
Member

Choose a reason for hiding this comment

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

Update: the suggestion below on a different transport mechanism would make this concern obsolete.

}
}
catch (Exception ex)
{
Expand Down Expand Up @@ -75,6 +92,14 @@ protected override void Serialize(ref MessagePackWriter writer, Type type, objec
writer.Write(bytes);
break;

case byte[][] byteArrays:
writer.WriteArrayHeader(byteArrays.Length);
foreach (var bytes in byteArrays)
{
writer.Write(bytes);
}
break;

default:
throw new FormatException($"Unsupported argument type {type}");
}
Expand Down
13 changes: 7 additions & 6 deletions src/Components/Server/src/Circuits/CircuitHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.JSInterop;
using Microsoft.JSInterop.Infrastructure;

namespace Microsoft.AspNetCore.Components.Server.Circuits
Expand Down Expand Up @@ -336,7 +337,7 @@ public async Task OnRenderCompletedAsync(long renderId, string errorMessageOrNul

// BeginInvokeDotNetFromJS is used in a fire-and-forget context, so it's responsible for its own
// error handling.
public async Task BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson)
public async Task BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, SerializedArgs serializedArgs)
{
AssertInitialized();
AssertNotDisposed();
Expand All @@ -347,7 +348,7 @@ await Renderer.Dispatcher.InvokeAsync(() =>
{
Log.BeginInvokeDotNet(_logger, callId, assemblyName, methodIdentifier, dotNetObjectId);
var invocationInfo = new DotNetInvocationInfo(assemblyName, methodIdentifier, dotNetObjectId, callId);
DotNetDispatcher.BeginInvokeDotNet(JSRuntime, invocationInfo, argsJson);
DotNetDispatcher.BeginInvokeDotNet(JSRuntime, invocationInfo, serializedArgs);
});
}
catch (Exception ex)
Expand All @@ -362,7 +363,7 @@ await Renderer.Dispatcher.InvokeAsync(() =>

// EndInvokeJSFromDotNet is used in a fire-and-forget context, so it's responsible for its own
// error handling.
public async Task EndInvokeJSFromDotNet(long asyncCall, bool succeeded, string arguments)
public async Task EndInvokeJSFromDotNet(long callId, bool succeeded, SerializedArgs serializedArgs)
{
AssertInitialized();
AssertNotDisposed();
Expand All @@ -374,14 +375,14 @@ await Renderer.Dispatcher.InvokeAsync(() =>
if (!succeeded)
{
// We can log the arguments here because it is simply the JS error with the call stack.
Log.EndInvokeJSFailed(_logger, asyncCall, arguments);
Log.EndInvokeJSFailed(_logger, callId, serializedArgs.ArgsJson);
}
else
{
Log.EndInvokeJSSucceeded(_logger, asyncCall);
Log.EndInvokeJSSucceeded(_logger, callId);
}

DotNetDispatcher.EndInvokeJS(JSRuntime, arguments);
DotNetDispatcher.EndInvokeJS(JSRuntime, serializedArgs);
});
}
catch (Exception ex)
Expand Down
7 changes: 4 additions & 3 deletions src/Components/Server/src/Circuits/RemoteJSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,12 @@ protected override void EndInvokeDotNet(DotNetInvocationInfo invocationInfo, in
_clientProxy.SendAsync("JS.EndInvokeDotNet",
invocationInfo.CallId,
/* success */ true,
invocationResult.ResultJson);
invocationResult.ResultJson,
invocationResult.ByteArrays);
}
}

protected override void BeginInvokeJS(long asyncHandle, string identifier, string argsJson, JSCallResultType resultType, long targetInstanceId)
protected override void BeginInvokeJS(long asyncHandle, string identifier, SerializedArgs serializedArgs, JSCallResultType resultType, long targetInstanceId)
{
if (_clientProxy is null)
{
Expand All @@ -86,7 +87,7 @@ protected override void BeginInvokeJS(long asyncHandle, string identifier, strin

Log.BeginInvokeJS(_logger, asyncHandle, identifier);

_clientProxy.SendAsync("JS.BeginInvokeJS", asyncHandle, identifier, argsJson, (int)resultType, targetInstanceId);
_clientProxy.SendAsync("JS.BeginInvokeJS", asyncHandle, identifier, serializedArgs.ArgsJson, serializedArgs.ByteArrays, (int)resultType, targetInstanceId);
}

public static class Log
Expand Down
11 changes: 7 additions & 4 deletions src/Components/Server/src/ComponentHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.Logging;
using Microsoft.JSInterop;

namespace Microsoft.AspNetCore.Components.Server
{
Expand Down Expand Up @@ -187,26 +188,28 @@ public async ValueTask<bool> ConnectCircuit(string circuitIdSecret)
return false;
}

public async ValueTask BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson)
public async ValueTask BeginInvokeDotNetFromJS(string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson, byte[][] byteArrays)
{
var circuitHost = await GetActiveCircuitAsync();
if (circuitHost == null)
{
return;
}

_ = circuitHost.BeginInvokeDotNetFromJS(callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson);
var serializedArgs = new SerializedArgs(argsJson, byteArrays);
_ = circuitHost.BeginInvokeDotNetFromJS(callId, assemblyName, methodIdentifier, dotNetObjectId, serializedArgs);
}

public async ValueTask EndInvokeJSFromDotNet(long asyncHandle, bool succeeded, string arguments)
public async ValueTask EndInvokeJSFromDotNet(long callId, bool succeeded, string resultOrError, byte[][] byteArrays)
{
var circuitHost = await GetActiveCircuitAsync();
if (circuitHost == null)
{
return;
}

_ = circuitHost.EndInvokeJSFromDotNet(asyncHandle, succeeded, arguments);
var serializedArgs = new SerializedArgs(resultOrError, byteArrays);
_ = circuitHost.EndInvokeJSFromDotNet(callId, succeeded, serializedArgs);
}

public async ValueTask DispatchBrowserEvent(string eventDescriptor, string eventArgs)
Expand Down
2 changes: 2 additions & 0 deletions src/Components/Shared/src/ArrayBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace Ignitor
namespace Microsoft.AspNetCore.Components.WebView
#elif COMPONENTS_SERVER
namespace Microsoft.AspNetCore.Components.Server.Circuits
#elif JS_INTEROP
namespace Microsoft.JSInterop.Infrastructure
#else
namespace Microsoft.AspNetCore.Components.RenderTree
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.webview.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/Components/Web.JS/src/Boot.Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,11 @@ async function initializeConnection(options: CircuitStartOptions, logger: Logger
}

DotNet.attachDispatcher({
beginInvokeDotNetFromJS: (callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson): void => {
connection.send('BeginInvokeDotNetFromJS', callId ? callId.toString() : null, assemblyName, methodIdentifier, dotNetObjectId || 0, argsJson);
beginInvokeDotNetFromJS: (callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson, byteArrays): void => {
connection.send('BeginInvokeDotNetFromJS', callId ? callId.toString() : null, assemblyName, methodIdentifier, dotNetObjectId || 0, argsJson, byteArrays);
},
endInvokeJSFromDotNet: (asyncHandle, succeeded, argsJson): void => {
connection.send('EndInvokeJSFromDotNet', asyncHandle, succeeded, argsJson);
endInvokeJSFromDotNet: (asyncHandle, succeeded, resultOrError, byteArrays): void => {
connection.send('EndInvokeJSFromDotNet', asyncHandle, succeeded, resultOrError, byteArrays);
},
});

Expand Down
13 changes: 8 additions & 5 deletions src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ function attachInteropInvoker(): void {
const dotNetDispatcherEndInvokeJSMethodHandle = bindStaticMethod('Microsoft.AspNetCore.Components.WebAssembly', 'Microsoft.AspNetCore.Components.WebAssembly.Services.DefaultWebAssemblyJSRuntime', 'EndInvokeJS');

DotNet.attachDispatcher({
beginInvokeDotNetFromJS: (callId: number, assemblyName: string | null, methodIdentifier: string, dotNetObjectId: any | null, argsJson: string): void => {
beginInvokeDotNetFromJS: (callId: number, assemblyName: string | null, methodIdentifier: string, dotNetObjectId: any | null, argsJson: string, byteArrays: Uint8Array[] | null): void => {
assertHeapIsNotLocked();
if (!dotNetObjectId && !assemblyName) {
throw new Error('Either assemblyName or dotNetObjectId must have a non null value.');
Expand All @@ -503,21 +503,24 @@ function attachInteropInvoker(): void {
assemblyNameOrDotNetObjectId,
methodIdentifier,
argsJson,
byteArrays,
);
},
endInvokeJSFromDotNet: (asyncHandle, succeeded, serializedArgs): void => {
endInvokeJSFromDotNet: (asyncHandle, succeeded, argsJson, byteArrays: Uint8Array[] | null): void => {
dotNetDispatcherEndInvokeJSMethodHandle(
serializedArgs
argsJson,
byteArrays,
);
},
invokeDotNetFromJS: (assemblyName, methodIdentifier, dotNetObjectId, argsJson) => {
invokeDotNetFromJS: (assemblyName, methodIdentifier, dotNetObjectId, argsJson, byteArrays: Uint8Array[] | null) => {
assertHeapIsNotLocked();
return dotNetDispatcherInvokeMethodHandle(
assemblyName ? assemblyName : null,
methodIdentifier,
dotNetObjectId ? dotNetObjectId.toString() : null,
argsJson,
) as string;
byteArrays,
) as DotNet.SerializedArgs | null;
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ export function sendBrowserEvent(descriptor: EventDescriptor, eventArgs: any) {
send('DispatchBrowserEvent', descriptor, eventArgs);
}

export function sendBeginInvokeDotNetFromJS(callId: number, assemblyName: string | null, methodIdentifier: string, dotNetObjectId: number | null, argsJson: string): void {
send('BeginInvokeDotNet', callId ? callId.toString() : null, assemblyName, methodIdentifier, dotNetObjectId || 0, argsJson);
export function sendBeginInvokeDotNetFromJS(callId: number, assemblyName: string | null, methodIdentifier: string, dotNetObjectId: number | null, argsJson: string, byteArrays: Uint8Array[] | null): void {
send('BeginInvokeDotNet', callId ? callId.toString() : null, assemblyName, methodIdentifier, dotNetObjectId || 0, argsJson, byteArrays);
}

export function sendEndInvokeJSFromDotNet(asyncHandle: number, succeeded: boolean, argsJson: any) {
send('EndInvokeJS', asyncHandle, succeeded, argsJson);
export function sendEndInvokeJSFromDotNet(asyncHandle: number, succeeded: boolean, argsJson: any, byteArrays: Uint8Array[] | null) {
send('EndInvokeJS', asyncHandle, succeeded, argsJson, byteArrays);
}

export function sendLocationChanged(uri: string, intercepted: boolean) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ function getCaptureIdAttributeName(referenceCaptureId: string) {

// Support receiving ElementRef instances as args in interop calls
const elementRefKey = '__internalId'; // Keep in sync with ElementRef.cs
DotNet.attachReviver((key, value) => {
DotNet.attachReviver(function reviveElementReference(key: any, value: any, byteArrays: Uint8Array[] | null) {
if (value && typeof value === 'object' && value.hasOwnProperty(elementRefKey) && typeof value[elementRefKey] === 'string') {
return getElementByCaptureId(value[elementRefKey]);
} else {
return value;
}

// Unrecognized - let another reviver handle it
return value;
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#nullable enable
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.BeginInvokeJS(long asyncHandle, string! identifier, string? argsJson, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> void
Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeUnmarshalled<T0, T1, T2, TResult>(string! identifier, T0 arg0, T1 arg1, T2 arg2) -> TResult
Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeUnmarshalled<T0, T1, TResult>(string! identifier, T0 arg0, T1 arg1) -> TResult
Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeUnmarshalled<T0, TResult>(string! identifier, T0 arg0) -> TResult
Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeUnmarshalled<TResult>(string! identifier) -> TResult
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeJS(string! identifier, string? argsJson, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> string!
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.BeginInvokeJS(long asyncHandle, string! identifier, string? argsJson, byte[]![]? byteArrays, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> void
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeJS(string! identifier, string? argsJson, byte[]![]? byteArrays, Microsoft.JSInterop.JSCallResultType resultType, long targetInstanceId) -> string!
Loading