warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

docs: tighten comments to the non-obvious why

Browse source 
Cuts explanation that restated the code or repeated itself, and keeps the parts
a future change would otherwise undo.

The same fact was asserted in three places: BanChannel.IsExempt, BanExemptions
and LoginAllowlist each explained that an exemption withholds escalation only.
It now lives on the seam that defines it, and the other two point at it.

The seed short-read comment was eight lines arguing a decision; the load-bearing
part is that waiting costs a connection slot per one- or two-byte client, so
that is what it says now. The new test-collection markers are one line each
rather than a summary of how STArrayPool works internally.

Also drops a fully-qualified System.Func in BanExemptions in favour of a using.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-07-30 18:40:02 -07:00
parent 65bdfe6cd7
commit b6c0a0bde8
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
6 changed files with 17 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

5
Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListReentrancyTests.cs
View file

@ -8,8 +8,7 @@ namespace Server.Tests;
/// hole is evaluated while it is live. A Reset()/Dispose() landing in that window used to leave the
/// next Append* spanning a null array: ArgumentNullException, parameter "array".
/// </summary>
// Sequential: building a list rents and returns through STArrayPool, whose bucket cache is a plain static
// with a check-then-act initialize. Two test classes doing this on different threads race it.
// Sequential: building a list rents from STArrayPool, which is not thread-safe.
[Collection("Sequential Server Tests")]
public class ObjectPropertyListReentrancyTests
{
@ -55,7 +54,7 @@ public class ObjectPropertyListReentrancyTests
/// The guard is per-list, so nested builds (a GetProperties override that reads another entity's
/// PropertyList) cannot unguard the outer one the way a single shared slot would.
/// </summary>
[Collection("Sequential Server Tests")] // same STArrayPool exposure as above
[Collection("Sequential Server Tests")]
public class ObjectPropertyListNestedBuildTests
{
[Fact]

8
Projects/Server/Network/Bans/BanChannel.cs
View file

@ -104,13 +104,11 @@ public static class BanChannel
/// <summary>Fans a locally-decided ban out to every reporter. Non-blocking; never throws.</summary>
/// <summary>
/// Optional content-supplied exemption. When it returns true the contribution is dropped before reaching
/// any reporter.
/// Optional content-supplied exemption; true drops the contribution before any reporter sees it. This
/// withholds escalation only — the gate that reached the verdict has already acted.
/// </summary>
/// <remarks>
/// Suppresses escalation only — the gate that reached the verdict has already acted, so the shard stays
/// protected. The reason is passed so policy can refuse to exempt an operator's own decision; an
/// implementation that ignores it would silently swallow manual bans. See
/// An implementation that ignores <c>reason</c> would silently swallow manual bans. See
/// <see cref="BanReasons.IsBehavioral"/>.
/// </remarks>
public static Func<IPAddress, string, bool> IsExempt { get; set; }

11
Projects/Server/Network/NetState/NetState.cs
View file

@ -696,14 +696,9 @@ public partial class NetState : IComparable<NetState>, IValueLinkListNode<NetSta
}
else
{
// Fewer than four bytes for a raw seed: disconnect rather than wait. This
// only affects pre-0xEF clients (0xEF goes through HandlePacket, which
// already waits for its 21 bytes), and waiting here would be paid for on
// the path that has to survive a flood -- a garbage client sending one or
// two bytes, or a slow loris dribbling a byte every few seconds, would
// hold a connection slot for the full ConnectingSocketIdleLimit instead of
// being dropped immediately. With a fixed MaxConnections table that trades
// capacity for a rare fragmentation case a reconnect already fixes.
// Disconnect rather than wait. Waiting would hold a connection slot for
// the full ConnectingSocketIdleLimit per one- or two-byte client, which
// is what a flood sends. Only pre-0xEF clients reach here.
Disconnect(string.Empty);
}
break;

8
Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs
View file

@ -20,12 +20,8 @@ using Xunit;
namespace Server.Tests.Network.AutoDenylists;
// Static store, so every test resets it first. Addresses come from TEST-NET-2 (198.51.100.0/24) to stay clear
// of the other ban tests if xUnit runs these classes concurrently.
//
// Sequential because the cap tests reach AutoDenylist.Sweep, which rents through STArrayPool. That pool is
// single-threaded by design and its bucket cache is a plain static, so touching it from two test threads
// races. The blocklist tests need no such marking because BlocklistSnapshot.Build asks for the mt pool.
// Static store, so every test resets it first. Addresses come from TEST-NET-2 (198.51.100.0/24).
// Sequential: the cap tests reach Sweep, which rents from STArrayPool, which is not thread-safe.
[Collection("Sequential UOContent Tests")]
public class AutoDenylistTests
{

12
Projects/UOContent/Network/BanExemptions.cs
View file

@ -13,20 +13,16 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System;
using System.Net;
using Server.Network.Bans;
namespace Server.Network;
/// <summary>
/// The single answer to "should this ban contribution be dropped?", combining the operator's file allowlist
/// with the one addresses earn by logging in. Owns <see cref="BanChannel.IsExempt"/> so neither source has to
/// know about the other.
/// Combines <see cref="FileAllowlist"/> and <see cref="LoginAllowlist"/> into the one answer
/// <see cref="BanChannel.IsExempt"/> asks for, so neither source has to know about the other.
/// </summary>
/// <remarks>
/// Suppresses escalation only. Whatever gate reached the verdict has already acted, so the shard is
/// protected either way; what is withheld is pushing the address to an external bouncer.
/// </remarks>
public static class BanExemptions
{
public static void Configure()
@ -41,7 +37,7 @@ public static class BanExemptions
/// Split for testing. <paramref name="loginAllowlist"/> is stateful — calling it spends a strike — so it
/// must not be invoked once the answer is already decided.
/// </summary>
internal static bool IsExempt(IPAddress address, string reason, System.Func<IPAddress, string, bool> loginAllowlist)
internal static bool IsExempt(IPAddress address, string reason, Func<IPAddress, string, bool> loginAllowlist)
{
if (address == null)
{

6
Projects/UOContent/Network/LoginAllowlist/LoginAllowlist.cs
View file

@ -33,9 +33,9 @@ namespace Server.Network;
/// <remarks>
/// <para>
/// Consulted only after the blocklist has already matched, and again before a ban is contributed, so a
/// normal accept pays nothing for it. Suppresses escalation only. An entry is evidence rather than a
/// licence: enough strikes inside the window revokes it. It cannot bootstrap, so it hedges stable addresses
/// and does not replace <see cref="FileAllowlist"/>. See <c>dev-docs/ip-bans-and-allowlists.md</c>.
/// normal accept pays nothing for it. An entry is evidence rather than a licence: enough strikes inside the
/// window revokes it. It cannot bootstrap, so it hedges stable addresses and does not replace
/// <see cref="FileAllowlist"/>. See <c>dev-docs/ip-bans-and-allowlists.md</c>.
/// </para>
/// <para>
/// Both dictionaries are game-loop state. Only the file write runs off-loop, over a snapshot taken on the