From 459186c5063c9a75b28309620fd8a6a85019f37f Mon Sep 17 00:00:00 2001 From: Marcin Hoppe Date: Thu, 9 Jun 2016 13:45:42 +0200 Subject: [PATCH 1/4] Quote table and schema name when assembling queue table name --- .../NServiceBus.SqlServer.UnitTests.csproj | 1 + .../TableBasedQueueTests.cs | 15 +++++++++++++++ src/NServiceBus.SqlServer/TableBasedQueue.cs | 18 ++++++++++-------- 3 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs diff --git a/src/NServiceBus.SqlServer.UnitTests/NServiceBus.SqlServer.UnitTests.csproj b/src/NServiceBus.SqlServer.UnitTests/NServiceBus.SqlServer.UnitTests.csproj index fb932a30b..93e22a46e 100644 --- a/src/NServiceBus.SqlServer.UnitTests/NServiceBus.SqlServer.UnitTests.csproj +++ b/src/NServiceBus.SqlServer.UnitTests/NServiceBus.SqlServer.UnitTests.csproj @@ -84,6 +84,7 @@ + diff --git a/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs b/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs new file mode 100644 index 000000000..9e588f024 --- /dev/null +++ b/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs @@ -0,0 +1,15 @@ +namespace NServiceBus.SqlServer.UnitTests +{ + using NServiceBus.Transports.SQLServer; + using NUnit.Framework; + + class TableBasedQueueTests + { + [Test] + public void Table_name_and_schema_should_be_quoted() + { + Assert.AreEqual("[nsb].[MyEndpoint]", new TableBasedQueue("MyEndpoint", "nsb").ToString()); + Assert.AreNotEqual("[nsb].[MyEndoint]SomeOtherData]", new TableBasedQueue("MyEndoint]SomeOtherData", "nsb").ToString()); + } + } +} \ No newline at end of file diff --git a/src/NServiceBus.SqlServer/TableBasedQueue.cs b/src/NServiceBus.SqlServer/TableBasedQueue.cs index 927bd535e..490e61167 100644 --- a/src/NServiceBus.SqlServer/TableBasedQueue.cs +++ b/src/NServiceBus.SqlServer/TableBasedQueue.cs @@ -17,8 +17,10 @@ public TableBasedQueue(Address address, string schema) public TableBasedQueue(string tableName, string schema) { - this.tableName = tableName; - this.schema = schema; + var sanitizer = new SqlCommandBuilder(); + + this.tableName = sanitizer.QuoteIdentifier(tableName); + this.schema = sanitizer.QuoteIdentifier(schema); } public void Send(TransportMessage message, SendOptions sendOptions, SqlConnection connection, SqlTransaction transaction = null) @@ -209,7 +211,7 @@ public void LogWarningWhenIndexIsMissing(SqlConnection connection) if (rowsCount == 0) { - Logger.Warn($@"Table [{schema}].[{tableName}] does not contain index '{ExpiresIndexName}'. + Logger.Warn($@"Table {schema}.{tableName} does not contain index '{ExpiresIndexName}'. Adding this index will speed up the process of purging expired messages from the queue. Please consult the documentation for further information."); } } @@ -217,7 +219,7 @@ public void LogWarningWhenIndexIsMissing(SqlConnection connection) public override string ToString() { - return tableName; + return $"{schema}.{tableName}"; } static readonly ILog Logger = LogManager.GetLogger(typeof(TableBasedQueue)); @@ -240,20 +242,20 @@ public override string ToString() }; const string SqlSend = - @"INSERT INTO [{0}].[{1}] ([Id],[CorrelationId],[ReplyToAddress],[Recoverable],[Expires],[Headers],[Body]) + @"INSERT INTO {0}.{1} ([Id],[CorrelationId],[ReplyToAddress],[Recoverable],[Expires],[Headers],[Body]) VALUES (@Id,@CorrelationId,@ReplyToAddress,@Recoverable,CASE WHEN @TimeToBeReceivedMs IS NOT NULL THEN DATEADD(ms, @TimeToBeReceivedMs, GETUTCDATE()) END,@Headers,@Body)"; const string SqlReceive = - @"WITH message AS (SELECT TOP(1) * FROM [{0}].[{1}] WITH (UPDLOCK, READPAST, ROWLOCK) ORDER BY [RowVersion] ASC) + @"WITH message AS (SELECT TOP(1) * FROM {0}.{1} WITH (UPDLOCK, READPAST, ROWLOCK) ORDER BY [RowVersion] ASC) DELETE FROM message OUTPUT deleted.Id, deleted.CorrelationId, deleted.ReplyToAddress, deleted.Recoverable, CASE WHEN deleted.Expires IS NOT NULL THEN DATEDIFF(ms, GETUTCDATE(), deleted.Expires) END, deleted.Headers, deleted.Body;"; const string SqlPurgeBatchOfExpiredMessages = - @"DELETE FROM [{1}].[{2}] WHERE [Id] IN (SELECT TOP ({0}) [Id] FROM [{1}].[{2}] WITH (UPDLOCK, READPAST, ROWLOCK) WHERE [Expires] < GETUTCDATE() ORDER BY [RowVersion])"; + @"DELETE FROM {1}.{2} WHERE [Id] IN (SELECT TOP ({0}) [Id] FROM {1}.{2} WITH (UPDLOCK, READPAST, ROWLOCK) WHERE [Expires] < GETUTCDATE() ORDER BY [RowVersion])"; const string SqlCheckIfExpiresIndexIsPresent = - @"SELECT COUNT(*) FROM [sys].[indexes] WHERE [name] = '{0}' AND [object_id] = OBJECT_ID('[{1}].[{2}]')"; + @"SELECT COUNT(*) FROM [sys].[indexes] WHERE [name] = '{0}' AND [object_id] = OBJECT_ID('{1}.{2}')"; const string ExpiresIndexName = "Index_Expires"; From 7e1d3672c9428ead37cd53710f6b03bf1890f44c Mon Sep 17 00:00:00 2001 From: Marcin Hoppe Date: Thu, 9 Jun 2016 21:23:01 +0200 Subject: [PATCH 2/4] Refactor unit test for better readability --- src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs b/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs index 9e588f024..1cb000ec1 100644 --- a/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs +++ b/src/NServiceBus.SqlServer.UnitTests/TableBasedQueueTests.cs @@ -9,7 +9,7 @@ class TableBasedQueueTests public void Table_name_and_schema_should_be_quoted() { Assert.AreEqual("[nsb].[MyEndpoint]", new TableBasedQueue("MyEndpoint", "nsb").ToString()); - Assert.AreNotEqual("[nsb].[MyEndoint]SomeOtherData]", new TableBasedQueue("MyEndoint]SomeOtherData", "nsb").ToString()); + Assert.AreEqual("[nsb].[MyEndoint]]; SOME OTHER SQL;--]", new TableBasedQueue("MyEndoint]; SOME OTHER SQL;--", "nsb").ToString()); } } } \ No newline at end of file From c9c13421d9aed1ca0cc394adc56f77bbae517cd2 Mon Sep 17 00:00:00 2001 From: Marcin Hoppe Date: Wed, 15 Jun 2016 14:03:53 +0200 Subject: [PATCH 3/4] Acceptance test to validate proper queue table name quoting --- ...erviceBus.SqlServer.AcceptanceTests.csproj | 1 + .../When_ReplyTo_address_does_not_exist.cs | 118 ++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 src/NServiceBus.SqlServer.AcceptanceTests/When_ReplyTo_address_does_not_exist.cs diff --git a/src/NServiceBus.SqlServer.AcceptanceTests/NServiceBus.SqlServer.AcceptanceTests.csproj b/src/NServiceBus.SqlServer.AcceptanceTests/NServiceBus.SqlServer.AcceptanceTests.csproj index 57d5346c7..4bf6a07c2 100644 --- a/src/NServiceBus.SqlServer.AcceptanceTests/NServiceBus.SqlServer.AcceptanceTests.csproj +++ b/src/NServiceBus.SqlServer.AcceptanceTests/NServiceBus.SqlServer.AcceptanceTests.csproj @@ -207,6 +207,7 @@ + diff --git a/src/NServiceBus.SqlServer.AcceptanceTests/When_ReplyTo_address_does_not_exist.cs b/src/NServiceBus.SqlServer.AcceptanceTests/When_ReplyTo_address_does_not_exist.cs new file mode 100644 index 000000000..2f4e4f22a --- /dev/null +++ b/src/NServiceBus.SqlServer.AcceptanceTests/When_ReplyTo_address_does_not_exist.cs @@ -0,0 +1,118 @@ +namespace NServiceBus.SqlServer.AcceptanceTests +{ + using System; + using NServiceBus.AcceptanceTesting; + using NServiceBus.AcceptanceTests.EndpointTemplates; + using NServiceBus.Config; + using NServiceBus.Faults; + using NServiceBus.Features; + using NUnit.Framework; + + public class When_ReplyTo_address_does_not_exist + { + [Test] + public void Should_throw() + { + var context = Scenario.Define() + .WithEndpoint(b => b.When(bus => bus.SendLocal(new StartCommand()))) + .WithEndpoint() + .AllowExceptions() + .Done(c => c.ExceptionReceived) + .Run(); + + Assert.That(context.ExceptionMessage, Contains.Substring("The destination queue") & Contains.Substring("could not be found")); + Assert.That(context.ExceptionMessage, Contains.Substring("error] VALUES(NEWID(), NULL, NULL, 1, NULL, '', NULL); DROP TABLE [Victim]; INSERT INTO [error")); + } + + class Context : ScenarioContext + { + public bool ExceptionReceived { get; set; } + public string ExceptionMessage { get; set; } + } + + class Attacker : EndpointConfigurationBuilder + { + public Attacker() + { + EndpointSetup(c => + { + c.UseTransport() + .DisableCallbackReceiver(); + c.OverridePublicReturnAddress(Address.Parse("error] VALUES(NEWID(), NULL, NULL, 1, NULL, '', NULL); DROP TABLE [Victim]; INSERT INTO [error")); + }) + .AddMapping(typeof(Victim)); + } + + class StartHandler : IHandleMessages + { + public IBus Bus { get; set; } + + public void Handle(StartCommand message) + { + Bus.Send(new AttackCommand()); + } + } + + class AttackResponseHandler : IHandleMessages + { + public void Handle(AttackResponse message) + { + } + } + } + + class Victim : EndpointConfigurationBuilder + { + public Victim() + { + EndpointSetup(b => + { + b.RegisterComponents(c => c.ConfigureComponent(DependencyLifecycle.SingleInstance)); + b.DisableFeature(); + }) + .WithConfig(c => c.MaxRetries = 0); + } + + class AttackHandler : IHandleMessages + { + public IBus Bus { get; set; } + + public void Handle(AttackCommand message) + { + Bus.Reply(new AttackResponse()); + } + } + + class CustomFaultManager : IManageMessageFailures + { + public Context Context { get; set; } + + public void SerializationFailedForMessage(TransportMessage message, Exception e) + { + } + + public void ProcessingAlwaysFailsForMessage(TransportMessage message, Exception e) + { + Context.ExceptionMessage = e.Message; + Context.ExceptionReceived = true; + } + + public void Init(Address address) + { + } + } + } + + class StartCommand : ICommand + { + } + + class AttackCommand : ICommand + { + } + + class AttackResponse : IMessage + { + } + } +} \ No newline at end of file From ff5bed13c2f94b2d77cc8765a53940eb379d98fc Mon Sep 17 00:00:00 2001 From: Marcin Hoppe Date: Mon, 27 Jun 2016 11:31:03 +0200 Subject: [PATCH 4/4] Dispose SqlCommandBuilder --- src/NServiceBus.SqlServer/TableBasedQueue.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/NServiceBus.SqlServer/TableBasedQueue.cs b/src/NServiceBus.SqlServer/TableBasedQueue.cs index 490e61167..fa92c3e6d 100644 --- a/src/NServiceBus.SqlServer/TableBasedQueue.cs +++ b/src/NServiceBus.SqlServer/TableBasedQueue.cs @@ -17,10 +17,11 @@ public TableBasedQueue(Address address, string schema) public TableBasedQueue(string tableName, string schema) { - var sanitizer = new SqlCommandBuilder(); - - this.tableName = sanitizer.QuoteIdentifier(tableName); - this.schema = sanitizer.QuoteIdentifier(schema); + using (var sanitizer = new SqlCommandBuilder()) + { + this.tableName = sanitizer.QuoteIdentifier(tableName); + this.schema = sanitizer.QuoteIdentifier(schema); + } } public void Send(TransportMessage message, SendOptions sendOptions, SqlConnection connection, SqlTransaction transaction = null)