diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index 789431e..51725fa 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -14,6 +14,7 @@ jobs: build: runs-on: ubuntu-latest + timeout-minutes: 5 steps: - name: Checkout source diff --git a/CoreRemoting.Tests/RpcTests.cs b/CoreRemoting.Tests/RpcTests.cs index 12af61d..d20c412 100644 --- a/CoreRemoting.Tests/RpcTests.cs +++ b/CoreRemoting.Tests/RpcTests.cs @@ -531,6 +531,28 @@ public void NonSerializableError_method_throws_Exception() } } + [Fact] + public void Failing_component_constructor_throws_RemoteInvocationException() + { + using var client = new RemotingClient(new ClientConfig() + { + ConnectionTimeout = 3, + InvocationTimeout = 3, + SendTimeout = 3, + Channel = ClientChannel, + MessageEncryption = false, + ServerPort = _serverFixture.Server.Config.NetworkPort, + }); + + client.Connect(); + + var proxy = client.CreateProxy(); + var ex = Assert.Throws(() => proxy.Hello()); + + Assert.NotNull(ex); + Assert.Contains("FailingService", ex.Message); + } + [Fact] public async Task Disposed_client_subscription_doesnt_break_other_clients() { diff --git a/CoreRemoting.Tests/ServerFixture.cs b/CoreRemoting.Tests/ServerFixture.cs index 0ae80c3..abcb3d0 100644 --- a/CoreRemoting.Tests/ServerFixture.cs +++ b/CoreRemoting.Tests/ServerFixture.cs @@ -48,6 +48,10 @@ public ServerFixture() // Service for enum tests container.RegisterService( lifetime: ServiceLifetime.Singleton); + + // Service for session tests + container.RegisterService( + lifetime: ServiceLifetime.SingleCall); } }; } diff --git a/CoreRemoting.Tests/SessionTests.cs b/CoreRemoting.Tests/SessionTests.cs index aea7c86..c9aa9e0 100644 --- a/CoreRemoting.Tests/SessionTests.cs +++ b/CoreRemoting.Tests/SessionTests.cs @@ -21,7 +21,7 @@ public SessionTests(ServerFixture serverFixture) _serverFixture = serverFixture; _serverFixture.Start(); } - + [Fact] public void Client_Connect_should_create_new_session_AND_Disconnect_should_close_session() { @@ -54,7 +54,7 @@ public void Client_Connect_should_create_new_session_AND_Disconnect_should_close client.Dispose(); }); - + var clientThread1 = new Thread(() => clientAction(0)); var clientThread2 = new Thread(() => clientAction(1)); @@ -94,7 +94,7 @@ public void Client_Connect_should_throw_exception_on_invalid_auth_credentials() AuthenticateFake = credentials => credentials[1].Value == "secret" } }; - + var server = new RemotingServer(serverConfig); server.Start(); @@ -102,7 +102,7 @@ public void Client_Connect_should_throw_exception_on_invalid_auth_credentials() { var clientAction = new Action((password, shouldThrow) => { - using var client = + using var client = new RemotingClient(new ClientConfig() { ConnectionTimeout = 0, @@ -114,7 +114,7 @@ public void Client_Connect_should_throw_exception_on_invalid_auth_credentials() new Credential() {Name = "Password", Value = password } } }); - + if (shouldThrow) Assert.Throws(() => client.Connect()); else @@ -124,7 +124,7 @@ public void Client_Connect_should_throw_exception_on_invalid_auth_credentials() var clientThread1 = new Thread(() => clientAction("wrong", true)); clientThread1.Start(); clientThread1.Join(); - + var clientThread2 = new Thread(() => clientAction("secret", false)); clientThread2.Start(); clientThread2.Join(); @@ -161,7 +161,7 @@ public void RemotingSession_Dispose_should_disconnect_client() { waitForDisconnect.Set(); }; - + client.Connect(); var proxy = client.CreateProxy(); @@ -169,7 +169,7 @@ public void RemotingSession_Dispose_should_disconnect_client() waitForDisconnect.Wait(); Assert.False(client.IsConnected); - + client.Dispose(); } } diff --git a/CoreRemoting.Tests/Tools/FailingService.cs b/CoreRemoting.Tests/Tools/FailingService.cs new file mode 100644 index 0000000..08e68a2 --- /dev/null +++ b/CoreRemoting.Tests/Tools/FailingService.cs @@ -0,0 +1,15 @@ +using System; + +namespace CoreRemoting.Tests.Tools; + +public class FailingService : IFailingService +{ + public FailingService() + { + throw new NotImplementedException(); + } + + public void Hello() + { + } +} diff --git a/CoreRemoting.Tests/Tools/IFailingService.cs b/CoreRemoting.Tests/Tools/IFailingService.cs new file mode 100644 index 0000000..f9a6507 --- /dev/null +++ b/CoreRemoting.Tests/Tools/IFailingService.cs @@ -0,0 +1,11 @@ +using System; +using System.Data; +using System.Threading.Tasks; +using CoreRemoting.Tests.ExternalTypes; + +namespace CoreRemoting.Tests.Tools; + +public interface IFailingService +{ + void Hello(); +} diff --git a/CoreRemoting.sln.DotSettings.user b/CoreRemoting.sln.DotSettings.user deleted file mode 100644 index dd336b1..0000000 --- a/CoreRemoting.sln.DotSettings.user +++ /dev/null @@ -1,36 +0,0 @@ - - True - ForceIncluded - ForceIncluded - ForceIncluded - ForceIncluded - ShowAndRun - <AssemblyExplorer> - <Assembly Path="/home/rainbird/.nuget/packages/microsoft.extensions.dependencyinjection.abstractions/3.1.7/lib/netstandard2.0/Microsoft.Extensions.DependencyInjection.Abstractions.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/microsoft.extensions.dependencyinjection/3.1.7/lib/netstandard2.0/Microsoft.Extensions.DependencyInjection.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/newtonsoft.json.bson/1.0.2/lib/netstandard2.0/Newtonsoft.Json.Bson.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/grpc.core.api/2.31.0/lib/netstandard2.0/Grpc.Core.Api.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/grpc.core/2.31.0/lib/netstandard2.0/Grpc.Core.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/system.security.cryptography.cng/4.7.0/ref/netstandard2.0/System.Security.Cryptography.Cng.dll" /> - <Assembly Path="/home/rainbird/.nuget/packages/npam/1.0.1/lib/netstandard1.2/Npam.dll" /> -</AssemblyExplorer> - /usr/share/dotnet/sdk/5.0.201/MSBuild.dll - - NewVersion - - <SessionState ContinuousTestingMode="0" FrameworkVersion="v4_7" IsActive="True" Name="All tests from Solution" PlatformType="x64" xmlns="urn:schemas-jetbrains-com:jetbrains-ut-session"> - <Solution /> -</SessionState> - - - - - - - - - - - - - \ No newline at end of file diff --git a/CoreRemoting/Channels/Websocket/WebsocketServerChannel.cs b/CoreRemoting/Channels/Websocket/WebsocketServerChannel.cs index 2ceaa18..8ffafe7 100644 --- a/CoreRemoting/Channels/Websocket/WebsocketServerChannel.cs +++ b/CoreRemoting/Channels/Websocket/WebsocketServerChannel.cs @@ -14,8 +14,8 @@ public class WebsocketServerChannel : IServerChannel private IRemotingServer Server { get; set; } - private ConcurrentDictionary Connections { get; } = - new ConcurrentDictionary(); + private ConcurrentDictionary Connections { get; } = + new ConcurrentDictionary(); /// public bool IsListening => HttpListener.IsListening; @@ -57,7 +57,7 @@ private async Task ReceiveConnection() // accept websocket request and start a new session var websocketContext = await context.AcceptWebSocketAsync(null); var websocket = websocketContext.WebSocket; - var connection = new WebsocketConnection(websocketContext, websocket, Server); + var connection = new WebsocketServerConnection(websocketContext, websocket, Server); // handle incoming websocket messages var sessionId = connection.StartListening(); diff --git a/CoreRemoting/Channels/Websocket/WebsocketConnection.cs b/CoreRemoting/Channels/Websocket/WebsocketServerConnection.cs similarity index 94% rename from CoreRemoting/Channels/Websocket/WebsocketConnection.cs rename to CoreRemoting/Channels/Websocket/WebsocketServerConnection.cs index 4143ffc..4e97716 100644 --- a/CoreRemoting/Channels/Websocket/WebsocketConnection.cs +++ b/CoreRemoting/Channels/Websocket/WebsocketServerConnection.cs @@ -9,15 +9,15 @@ namespace CoreRemoting.Channels.Websocket; /// /// Websocket connection. /// -public class WebsocketConnection : IRawMessageTransport +public class WebsocketServerConnection : IRawMessageTransport { // note: LOH threshold is ~85 kilobytes private const int BufferSize = 16 * 1024; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - public WebsocketConnection(HttpListenerWebSocketContext websocketContext, WebSocket websocket, IRemotingServer remotingServer) + public WebsocketServerConnection(HttpListenerWebSocketContext websocketContext, WebSocket websocket, IRemotingServer remotingServer) { WebSocketContext = websocketContext ?? throw new ArgumentNullException(nameof(websocketContext)); WebSocket = websocket ?? throw new ArgumentNullException(nameof(websocket)); diff --git a/CoreRemoting/RemotingClient.cs b/CoreRemoting/RemotingClient.cs index 37c427c..808462f 100644 --- a/CoreRemoting/RemotingClient.cs +++ b/CoreRemoting/RemotingClient.cs @@ -434,7 +434,7 @@ private void Authenticate() /// Raw message data private void OnMessage(byte[] rawMessage) { - var message = Serializer.Deserialize(rawMessage); + var message = TryDeserialize(rawMessage); switch (message.MessageType.ToLower()) { @@ -456,6 +456,29 @@ private void OnMessage(byte[] rawMessage) case "session_closed": Disconnect(quiet: true); break; + default: + // TODO: how do we handle invalid wire messages received by the client? + // A wire message could have been tampered with and couldn't be deserialized + break; + } + } + + private WireMessage TryDeserialize(byte[] rawMessage) + { + try + { + return Serializer.Deserialize(rawMessage); + } + catch // TODO: dispatch message deserialization exception? + { + return new WireMessage + { + Data = rawMessage, + Error = true, + Iv = Array.Empty(), + MessageType = "invalid", + UniqueCallKey = Array.Empty(), + }; } } @@ -585,16 +608,27 @@ private void ProcessRpcResultMessage(WireMessage message) if (message.Error) { - var remoteException = - Serializer.Deserialize( - MessageEncryptionManager.GetDecryptedMessageData( - message: message, - serializer: Serializer, - sharedSecret: sharedSecret, - sendersPublicKeyBlob: _serverPublicKeyBlob, - sendersPublicKeySize: _keyPair?.KeySize ?? 0)); + try + { + var remoteException = + Serializer.Deserialize( + MessageEncryptionManager.GetDecryptedMessageData( + message: message, + serializer: Serializer, + sharedSecret: sharedSecret, + sendersPublicKeyBlob: _serverPublicKeyBlob, + sendersPublicKeySize: _keyPair?.KeySize ?? 0)); + + clientRpcContext.RemoteException = remoteException; + } + catch (Exception deserializationException) + { + var remoteException = new RemoteInvocationException( + "Remote exception couldn't be deserialized", + deserializationException); - clientRpcContext.RemoteException = remoteException; + clientRpcContext.RemoteException = remoteException; + } } else { diff --git a/CoreRemoting/RemotingSession.cs b/CoreRemoting/RemotingSession.cs index b3284e3..bd0d0ea 100644 --- a/CoreRemoting/RemotingSession.cs +++ b/CoreRemoting/RemotingSession.cs @@ -455,7 +455,7 @@ private void ProcessRpcMessage(WireMessage request) serverRpcContext.Exception = new RemoteInvocationException( message: ex.Message, - innerEx: ex.GetType().IsSerializable ? ex : null); + innerEx: ex.ToSerializable()); if (oneWay) return; diff --git a/CoreRemoting/Serialization/ExceptionExtensions.cs b/CoreRemoting/Serialization/ExceptionExtensions.cs index 8650318..c29cfbf 100644 --- a/CoreRemoting/Serialization/ExceptionExtensions.cs +++ b/CoreRemoting/Serialization/ExceptionExtensions.cs @@ -1,4 +1,5 @@ -using System; +using Castle.MicroKernel.ComponentActivator; +using System; using System.Linq; namespace CoreRemoting.Serialization; @@ -20,6 +21,9 @@ public static class ExceptionExtensions agg.InnerException.IsSerializable() && agg.GetType().IsSerializable, + // pesky exception that looks like serializable but really isn't + ComponentActivatorException cax => false, + _ => ex.GetType().IsSerializable && ex.InnerException.IsSerializable() };