Skip to content

Commit

Permalink
Changes per PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
pranavkm committed Sep 22, 2020
1 parent cfbfbd7 commit 998cc42
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ internal static HttpConnectionOptions ShallowCopyHttpConnectionOptions(HttpConne
AccessTokenProvider = options.AccessTokenProvider,
CloseTimeout = options.CloseTimeout,
DefaultTransferFormat = options.DefaultTransferFormat,
WebSocketConfiguration = options.WebSocketConfiguration,
};

if (!OperatingSystem.IsBrowser())
Expand All @@ -103,6 +102,7 @@ internal static HttpConnectionOptions ShallowCopyHttpConnectionOptions(HttpConne
newOptions.Credentials = options.Credentials;
newOptions.Proxy = options.Proxy;
newOptions.UseDefaultCredentials = options.UseDefaultCredentials;
newOptions.WebSocketConfiguration = options.WebSocketConfiguration;
}

return newOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class HttpConnectionOptions
private ICredentials _credentials;
private IWebProxy _proxy;
private bool? _useDefaultCredentials;
private Action<ClientWebSocketOptions> _webSocketConfiguration;

/// <summary>
/// Initializes a new instance of the <see cref="HttpConnectionOptions"/> class.
Expand Down Expand Up @@ -192,7 +193,20 @@ public bool? UseDefaultCredentials
/// This delegate is invoked after headers from <see cref="Headers"/> and the access token from <see cref="AccessTokenProvider"/>
/// has been applied.
/// </remarks>
public Action<ClientWebSocketOptions> WebSocketConfiguration { get; set; }
[UnsupportedOSPlatform("browser")]
public Action<ClientWebSocketOptions> WebSocketConfiguration
{
get
{
ThrowIfUnsupportedPlatform();
return _webSocketConfiguration;
}
set
{
ThrowIfUnsupportedPlatform();
_webSocketConfiguration = value;
}
}

private static void ThrowIfUnsupportedPlatform()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ private static class Log
private static readonly Action<ILogger, Exception> _startedTransport =
LoggerMessage.Define(LogLevel.Debug, new EventId(19, "StartedTransport"), "Started transport.");

private static readonly Action<ILogger, Exception> _headersNotSupported =
LoggerMessage.Define(LogLevel.Warning, new EventId(20, "HeadersNotSupported"),
$"Configuring request headers using {nameof(HttpConnectionOptions)}.{nameof(HttpConnectionOptions.Headers)} is not supported when using websockets transport " +
"on the browser platform.");


public static void StartTransport(ILogger logger, TransferFormat transferFormat, Uri webSocketUrl)
{
_startTransport(logger, transferFormat, webSocketUrl, null);
Expand Down Expand Up @@ -163,6 +169,11 @@ public static void StartedTransport(ILogger logger)
{
_startedTransport(logger, null);
}

public static void HeadersNotSupported(ILogger logger)
{
_headersNotSupported(logger, null);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,18 @@ public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerF

if (httpConnectionOptions != null)
{
if (httpConnectionOptions.Headers != null)
if (httpConnectionOptions.Headers.Count > 0)
{
if (isBrowser)
{
throw new PlatformNotSupportedException("Configuring request headers is not supported when using WebSocketsTransport in the browser platform.");
Log.HeadersNotSupported(_logger);
}

foreach (var header in httpConnectionOptions.Headers)
else
{
_webSocket.Options.SetRequestHeader(header.Key, header.Value);
foreach (var header in httpConnectionOptions.Headers)
{
_webSocket.Options.SetRequestHeader(header.Key, header.Value);
}
}
}

Expand Down Expand Up @@ -90,9 +92,9 @@ public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerF
{
_webSocket.Options.UseDefaultCredentials = httpConnectionOptions.UseDefaultCredentials.Value;
}
}

httpConnectionOptions.WebSocketConfiguration?.Invoke(_webSocket.Options);
httpConnectionOptions.WebSocketConfiguration?.Invoke(_webSocket.Options);
}
}

if (!isBrowser)
Expand All @@ -101,7 +103,7 @@ public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerF
// See: https://github.com/aspnet/Security/blob/ff9f145a8e89c9756ea12ff10c6d47f2f7eb345f/src/Microsoft.AspNetCore.Authentication.Cookies/Events/CookieAuthenticationEvents.cs#L42
#pragma warning disable CA1416 // Analyzer bug
_webSocket.Options.SetRequestHeader("X-Requested-With", "XMLHttpRequest");
#pragma warning restore CA1416 // Validate platform compatibility
#pragma warning restore CA1416 // Analyzer bug
}

_closeTimeout = httpConnectionOptions?.CloseTimeout ?? default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<Description>Implements the SignalR Hub Protocol over MsgPack.</Description>
<TargetFrameworks>$(DefaultNetFxTargetFramework);netstandard2.0</TargetFrameworks>
<TargetFrameworks>$(DefaultNetCoreTargetFramework);$(DefaultNetFxTargetFramework);netstandard2.0</TargetFrameworks>
<RootNamespace>Microsoft.AspNetCore.SignalR</RootNamespace>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal abstract class MessagePackHubProtocolWorker
private const int VoidResult = 2;
private const int NonVoidResult = 3;

public bool TryParseMessage(ref ReadOnlySequence<byte> input, IInvocationBinder binder, out HubMessage message)
public bool TryParseMessage(ref ReadOnlySequence<byte> input, IInvocationBinder binder, out HubMessage? message)
{
if (!BinaryMessageParser.TryParseMessage(ref input, out var payload))
{
Expand All @@ -34,7 +34,7 @@ public bool TryParseMessage(ref ReadOnlySequence<byte> input, IInvocationBinder
return true;
}

private HubMessage ParseMessage(ref MessagePackReader reader, IInvocationBinder binder)
private HubMessage? ParseMessage(ref MessagePackReader reader, IInvocationBinder binder)
{
var itemCount = reader.ReadArrayHeader();

Expand Down Expand Up @@ -76,7 +76,7 @@ private HubMessage CreateInvocationMessage(ref MessagePackReader reader, IInvoca

var target = ReadString(ref reader, "target");

object[] arguments = null;
object[]? arguments;
try
{
var parameterTypes = binder.GetParameterTypes(target);
Expand All @@ -87,7 +87,7 @@ private HubMessage CreateInvocationMessage(ref MessagePackReader reader, IInvoca
return new InvocationBindingFailureMessage(invocationId, target, ExceptionDispatchInfo.Capture(ex));
}

string[] streams = null;
string[]? streams = null;
// Previous clients will send 5 items, so we check if they sent a stream array or not
if (itemCount > 5)
{
Expand All @@ -103,7 +103,7 @@ private HubMessage CreateStreamInvocationMessage(ref MessagePackReader reader, I
var invocationId = ReadInvocationId(ref reader);
var target = ReadString(ref reader, "target");

object[] arguments = null;
object[] arguments;
try
{
var parameterTypes = binder.GetParameterTypes(target);
Expand All @@ -114,7 +114,7 @@ private HubMessage CreateStreamInvocationMessage(ref MessagePackReader reader, I
return new InvocationBindingFailureMessage(invocationId, target, ExceptionDispatchInfo.Capture(ex));
}

string[] streams = null;
string[]? streams = null;
// Previous clients will send 5 items, so we check if they sent a stream array or not
if (itemCount > 5)
{
Expand Down Expand Up @@ -148,8 +148,8 @@ private CompletionMessage CreateCompletionMessage(ref MessagePackReader reader,
var invocationId = ReadInvocationId(ref reader);
var resultKind = ReadInt32(ref reader, "resultKind");

string error = null;
object result = null;
string? error = null;
object? result = null;
var hasResult = false;

switch (resultKind)
Expand Down Expand Up @@ -198,7 +198,7 @@ private CloseMessage CreateCloseMessage(ref MessagePackReader reader, int itemCo
return new CloseMessage(error, allowReconnect);
}

private Dictionary<string, string> ReadHeaders(ref MessagePackReader reader)
private Dictionary<string, string>? ReadHeaders(ref MessagePackReader reader)
{
var headerCount = ReadMapLength(ref reader, "headers");
if (headerCount > 0)
Expand All @@ -219,10 +219,10 @@ private Dictionary<string, string> ReadHeaders(ref MessagePackReader reader)
}
}

private string[] ReadStreamIds(ref MessagePackReader reader)
private string[]? ReadStreamIds(ref MessagePackReader reader)
{
var streamIdCount = ReadArrayLength(ref reader, "streamIds");
List<string> streams = null;
List<string>? streams = null;

if (streamIdCount > 0)
{
Expand Down Expand Up @@ -264,7 +264,7 @@ private object[] BindArguments(ref MessagePackReader reader, IReadOnlyList<Type>

protected abstract object DeserializeObject(ref MessagePackReader reader, Type type, string field);

private T ApplyHeaders<T>(IDictionary<string, string> source, T destination) where T : HubInvocationMessage
private T ApplyHeaders<T>(IDictionary<string, string>? source, T destination) where T : HubInvocationMessage
{
if (source != null && source.Count > 0)
{
Expand Down Expand Up @@ -374,10 +374,18 @@ private void WriteInvocationMessage(InvocationMessage message, ref MessagePackWr
writer.Write(message.InvocationId);
}
writer.Write(message.Target);
writer.WriteArrayHeader(message.Arguments.Length);
foreach (var arg in message.Arguments)

if (message.Arguments is null)
{
WriteArgument(arg, ref writer);
writer.WriteArrayHeader(0);
}
else
{
writer.WriteArrayHeader(message.Arguments.Length);
foreach (var arg in message.Arguments)
{
WriteArgument(arg, ref writer);
}
}

WriteStreamIds(message.StreamIds, ref writer);
Expand All @@ -392,10 +400,17 @@ private void WriteStreamInvocationMessage(StreamInvocationMessage message, ref M
writer.Write(message.InvocationId);
writer.Write(message.Target);

writer.WriteArrayHeader(message.Arguments.Length);
foreach (var arg in message.Arguments)
if (message.Arguments is null)
{
WriteArgument(arg, ref writer);
writer.WriteArrayHeader(0);
}
else
{
writer.WriteArrayHeader(message.Arguments.Length);
foreach (var arg in message.Arguments)
{
WriteArgument(arg, ref writer);
}
}

WriteStreamIds(message.StreamIds, ref writer);
Expand All @@ -410,7 +425,7 @@ private void WriteStreamingItemMessage(StreamItemMessage message, ref MessagePac
WriteArgument(message.Item, ref writer);
}

private void WriteArgument(object argument, ref MessagePackWriter writer)
private void WriteArgument(object? argument, ref MessagePackWriter writer)
{
if (argument == null)
{
Expand All @@ -424,7 +439,7 @@ private void WriteArgument(object argument, ref MessagePackWriter writer)

protected abstract void Serialize(ref MessagePackWriter writer, Type type, object value);

private void WriteStreamIds(string[] streamIds, ref MessagePackWriter writer)
private void WriteStreamIds(string[]? streamIds, ref MessagePackWriter writer)
{
if (streamIds != null)
{
Expand Down Expand Up @@ -493,7 +508,7 @@ private void WritePingMessage(PingMessage pingMessage, ref MessagePackWriter wri
writer.Write(HubProtocolConstants.PingMessageType);
}

private void PackHeaders(IDictionary<string, string> headers, ref MessagePackWriter writer)
private void PackHeaders(IDictionary<string, string>? headers, ref MessagePackWriter writer)
{
if (headers != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<Description>Implements the SignalR Hub Protocol using Newtonsoft.Json.</Description>
<TargetFrameworks>$(DefaultNetFxTargetFramework);netstandard2.0</TargetFrameworks>
<TargetFrameworks>$(DefaultNetCoreTargetFramework);$(DefaultNetFxTargetFramework);netstandard2.0</TargetFrameworks>
<RootNamespace>Microsoft.AspNetCore.SignalR</RootNamespace>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/common/Shared/JsonUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public static bool ReadAsBoolean(JsonTextReader reader, string propertyName)
return Convert.ToInt32(reader.Value, CultureInfo.InvariantCulture);
}

public static string ReadAsString(JsonTextReader reader, string propertyName)
public static string? ReadAsString(JsonTextReader reader, string propertyName)
{
reader.Read();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public StreamInvocationMessage(string invocationId, string target, object[] argu
/// <param name="target">The target method name.</param>
/// <param name="arguments">The target method arguments.</param>
/// <param name="streamIds">The target methods stream IDs.</param>
public StreamInvocationMessage(string invocationId, string target, object[] arguments, string[] streamIds)
public StreamInvocationMessage(string invocationId, string target, object[] arguments, string[]? streamIds)
: base(invocationId, target, arguments, streamIds)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class InvocationBindingFailureMessage : HubInvocationMessage
/// <param name="invocationId">The invocation ID.</param>
/// <param name="target">The target method name.</param>
/// <param name="bindingFailure">The exception thrown during binding.</param>
public InvocationBindingFailureMessage(string invocationId, string target, ExceptionDispatchInfo bindingFailure) : base(invocationId)
public InvocationBindingFailureMessage(string? invocationId, string target, ExceptionDispatchInfo bindingFailure) : base(invocationId)
{
Target = target;
BindingFailure = bindingFailure;
Expand Down

0 comments on commit 998cc42

Please sign in to comment.