fix(advanced-search): A/F - worker filter is exception-safe, skips deleted entities, guards empty expressions
This commit is contained in:
parent
eb5e15e228
commit
8aa797a586
2 changed files with 102 additions and 0 deletions
|
|
@ -0,0 +1,75 @@
|
|||
using System;
|
||||
using System.Collections.Concurrent;
|
||||
using Server;
|
||||
using Server.Engines.AdvancedSearch;
|
||||
using Server.Items;
|
||||
using Server.Tests;
|
||||
using Xunit;
|
||||
|
||||
namespace UOContent.Tests;
|
||||
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class AdvancedSearchWorkerTests
|
||||
{
|
||||
// An item whose property test path will throw when evaluated.
|
||||
private sealed class ThrowingItem : Item
|
||||
{
|
||||
public ThrowingItem() : base(0x1) { }
|
||||
public ThrowingItem(Serial s) : base(s) { }
|
||||
public string Boom => throw new InvalidOperationException("boom");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Worker_FilterThrows_DoesNotEscape_ReturnsNoMatch()
|
||||
{
|
||||
var worker = new AdvancedSearchThreadWorker();
|
||||
var results = new ConcurrentQueue<AdvancedSearchResult>();
|
||||
var ignore = new ConcurrentQueue<IEntity>();
|
||||
var filter = new AdvancedSearchFilter
|
||||
{
|
||||
FilterPropertyTest = true,
|
||||
PropertyTest = "Boom=1", // reflection GetValue -> throws
|
||||
};
|
||||
|
||||
var item = new ThrowingItem();
|
||||
|
||||
try
|
||||
{
|
||||
worker.Wake(new WorldLocation(Point3D.Zero, Map.Felucca), filter, results, ignore);
|
||||
worker.Push(item);
|
||||
worker.Sleep(); // drains; must not crash the test process
|
||||
|
||||
Assert.Empty(results);
|
||||
}
|
||||
finally
|
||||
{
|
||||
item.Delete();
|
||||
worker.Exit();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Worker_DeletedEntity_IsSkipped()
|
||||
{
|
||||
var worker = new AdvancedSearchThreadWorker();
|
||||
var results = new ConcurrentQueue<AdvancedSearchResult>();
|
||||
var ignore = new ConcurrentQueue<IEntity>();
|
||||
var filter = new AdvancedSearchFilter(); // no filters -> everything matches
|
||||
|
||||
var item = new Item(0x1);
|
||||
item.Delete();
|
||||
|
||||
try
|
||||
{
|
||||
worker.Wake(new WorldLocation(Point3D.Zero, Map.Felucca), filter, results, ignore);
|
||||
worker.Push(item);
|
||||
worker.Sleep();
|
||||
|
||||
Assert.Empty(results);
|
||||
}
|
||||
finally
|
||||
{
|
||||
worker.Exit();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -3,6 +3,7 @@ using System.Collections.Concurrent;
|
|||
using System.Reflection;
|
||||
using System.Threading;
|
||||
using Server.Items;
|
||||
using Server.Logging;
|
||||
using Server.Mobiles;
|
||||
using Server.Multis;
|
||||
|
||||
|
|
@ -10,6 +11,8 @@ namespace Server.Engines.AdvancedSearch;
|
|||
|
||||
public class AdvancedSearchThreadWorker
|
||||
{
|
||||
private static readonly ILogger _logger = LogFactory.GetLogger(typeof(AdvancedSearchThreadWorker));
|
||||
|
||||
private readonly Thread _thread;
|
||||
private readonly AutoResetEvent _startEvent; // Main thread tells the thread to start working
|
||||
private readonly AutoResetEvent _stopEvent; // Main thread waits for the worker finish draining
|
||||
|
|
@ -101,6 +104,24 @@ public class AdvancedSearchThreadWorker
|
|||
}
|
||||
|
||||
private AdvancedSearchResult DoEntitySearch(IEntity entity)
|
||||
{
|
||||
if (entity == null || entity.Deleted)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
return DoEntitySearchCore(entity);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.Warning(ex, "AdvancedSearch: filter threw for {Entity}; skipping", entity);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private AdvancedSearchResult DoEntitySearchCore(IEntity entity)
|
||||
{
|
||||
if (_filter.HideValidInternalMap)
|
||||
{
|
||||
|
|
@ -317,6 +338,12 @@ public class AdvancedSearchThreadWorker
|
|||
|
||||
private static bool EvaluateSingleExpression(IEntity entity, ReadOnlySpan<char> expression)
|
||||
{
|
||||
expression = expression.Trim();
|
||||
if (expression.Length == 0)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
var negate = false;
|
||||
if (expression[0] == '~')
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue