fix(worker): narrow AlarmAckWriter ArgumentException interop filter
Resolve the operator user, password and comment before the interop try blocks, and match ArgumentException as an MXAccess failure only when it carries E_INVALIDARG and is not ArgumentNullException or ArgumentOutOfRangeException. A programming error inside the try blocks now propagates instead of being reported as an authentication or write failure. Fixes #164 Claude-Session: https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT
This commit is contained in:
+1
-1
@@ -115,7 +115,7 @@ The worker calls `AuthenticateUser` as the operator, then writes `operator_comme
|
|||||||
| `AuthenticateUser` failed with a server-gone HRESULT (`RPC_E_DISCONNECTED`, `RPC_S_SERVER_UNAVAILABLE`, `RPC_E_SERVERFAULT`); the writer tears down its MXAccess registration and re-registers on the next ack | `MXACCESS_FAILURE` | that HRESULT |
|
| `AuthenticateUser` failed with a server-gone HRESULT (`RPC_E_DISCONNECTED`, `RPC_S_SERVER_UNAVAILABLE`, `RPC_E_SERVERFAULT`); the writer tears down its MXAccess registration and re-registers on the next ack | `MXACCESS_FAILURE` | that HRESULT |
|
||||||
| `AddItem`, `Advise` or `Write` on the `AckMsg` item raised an MXAccess error (`COMException`, or `ArgumentException` for `E_INVALIDARG`); the item is dropped and re-added on the next ack, and a server-gone HRESULT also tears down the registration | `MXACCESS_FAILURE` | the exception's HRESULT |
|
| `AddItem`, `Advise` or `Write` on the `AckMsg` item raised an MXAccess error (`COMException`, or `ArgumentException` for `E_INVALIDARG`); the item is dropped and re-added on the next ack, and a server-gone HRESULT also tears down the registration | `MXACCESS_FAILURE` | the exception's HRESULT |
|
||||||
| Write rejected (`OnWriteComplete` non-OK) | `MXACCESS_FAILURE` | the `detail` of the `OnWriteComplete` `MXSTATUS_PROXY`, or `-1` when `detail` is `0` |
|
| Write rejected (`OnWriteComplete` non-OK) | `MXACCESS_FAILURE` | the `detail` of the `OnWriteComplete` `MXSTATUS_PROXY`, or `-1` when `detail` is `0` |
|
||||||
| Any other exception escaping the ack (e.g. the ack writer's MXAccess `Register` failing, or the monitor's worker session having no active alarm subscription — `InvalidOperationException`) | `MXACCESS_FAILURE` | the exception's `HResult`; the diagnostic names only the exception type and HRESULT |
|
| Any other exception escaping the ack (e.g. the ack writer's MXAccess `Register` failing, the monitor's worker session having no active alarm subscription — `InvalidOperationException` — or an argument error that is not the interop's `E_INVALIDARG`: `ArgumentNullException`, `ArgumentOutOfRangeException`, or an `ArgumentException` with another HRESULT, which signal a bug rather than an MXAccess failure) | `MXACCESS_FAILURE` | the exception's `HResult`; the diagnostic names only the exception type and HRESULT |
|
||||||
|
|
||||||
`reply.Statuses` carries the `OnWriteComplete` `MxStatusProxy` rows when they were observed. Because an unconfirmed write also returns `OK`, a client that must be certain should watch for the resulting transition on `StreamAlarms`. `AcknowledgeAlarm` stays under the `invoke:write` scope; see [Authorization](./Authorization.md).
|
`reply.Statuses` carries the `OnWriteComplete` `MxStatusProxy` rows when they were observed. Because an unconfirmed write also returns `OK`, a client that must be certain should watch for the resulting transition on `StreamAlarms`. `AcknowledgeAlarm` stays under the `invoke:write` scope; see [Authorization](./Authorization.md).
|
||||||
|
|
||||||
|
|||||||
@@ -187,6 +187,70 @@ public sealed class AlarmAckWriterTests
|
|||||||
Assert.DoesNotContain("Unregister", server.Operations);
|
Assert.DoesNotContain("Unregister", server.Operations);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Acknowledge_WhenAuthenticateUserThrowsArgumentExceptionWithEInvalidArg_ReturnsAuthenticationFailed()
|
||||||
|
{
|
||||||
|
// The HRESULT is what the narrowed filter keys on; set it explicitly so the
|
||||||
|
// test does not depend on ArgumentException's default HResult.
|
||||||
|
RecordingAckServer server = new RecordingAckServer
|
||||||
|
{
|
||||||
|
AuthenticateResult = 5,
|
||||||
|
AuthenticateException = new HResultArgumentException("bad credentials", unchecked((int)0x80070057)),
|
||||||
|
};
|
||||||
|
using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
|
||||||
|
|
||||||
|
AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "one", Alice());
|
||||||
|
|
||||||
|
Assert.Equal(AlarmAckOutcome.AuthenticationFailed, result.Outcome);
|
||||||
|
Assert.Equal(unchecked((int)0x80070057), result.NativeStatus);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Acknowledge_WhenAuthenticateUserThrowsArgumentNullException_Propagates()
|
||||||
|
{
|
||||||
|
// A programming error is a bug, not an operator authentication failure.
|
||||||
|
RecordingAckServer server = new RecordingAckServer
|
||||||
|
{
|
||||||
|
AuthenticateResult = 5,
|
||||||
|
AuthenticateException = new ArgumentNullException("verifyUser"),
|
||||||
|
};
|
||||||
|
using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
|
||||||
|
|
||||||
|
Assert.Throws<ArgumentNullException>(() => writer.Acknowledge("T.A.AckMsg", "one", Alice()));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Acknowledge_WhenAuthenticateUserThrowsArgumentExceptionWithOtherHResult_Propagates()
|
||||||
|
{
|
||||||
|
RecordingAckServer server = new RecordingAckServer
|
||||||
|
{
|
||||||
|
AuthenticateResult = 5,
|
||||||
|
AuthenticateException = new HResultArgumentException("not interop", unchecked((int)0x80131500)),
|
||||||
|
};
|
||||||
|
using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
|
||||||
|
|
||||||
|
Assert.Throws<HResultArgumentException>(() => writer.Acknowledge("T.A.AckMsg", "one", Alice()));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Acknowledge_WhenWriteThrowsArgumentOutOfRangeException_Propagates()
|
||||||
|
{
|
||||||
|
RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5, WriteException = new ArgumentOutOfRangeException("value") };
|
||||||
|
using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
|
||||||
|
|
||||||
|
Assert.Throws<ArgumentOutOfRangeException>(() => writer.Acknowledge("T.A.AckMsg", "one", Alice()));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Acknowledge_WhenOperatorUserNameIsNull_ThrowsBeforeAuthenticating()
|
||||||
|
{
|
||||||
|
RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
|
||||||
|
using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
|
||||||
|
|
||||||
|
Assert.Throws<ArgumentException>(() => writer.Acknowledge("T.A.AckMsg", "one", new AlarmAckOperator { UserName = null! }));
|
||||||
|
Assert.DoesNotContain(server.Operations, op => op.StartsWith("AuthenticateUser", StringComparison.Ordinal));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Acknowledge_WhenWriteThrowsArgumentException_RejectsAndDropsItem()
|
public void Acknowledge_WhenWriteThrowsArgumentException_RejectsAndDropsItem()
|
||||||
{
|
{
|
||||||
@@ -290,6 +354,16 @@ public sealed class AlarmAckWriterTests
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>ArgumentException with an explicit HRESULT (the setter is protected on net48).</summary>
|
||||||
|
internal sealed class HResultArgumentException : ArgumentException
|
||||||
|
{
|
||||||
|
public HResultArgumentException(string message, int hresult)
|
||||||
|
: base(message)
|
||||||
|
{
|
||||||
|
HResult = hresult;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
internal sealed class RecordingAckServer : IMxAccessServer
|
internal sealed class RecordingAckServer : IMxAccessServer
|
||||||
{
|
{
|
||||||
private int nextItemHandle = 100;
|
private int nextItemHandle = 100;
|
||||||
|
|||||||
@@ -34,6 +34,9 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public const int DefaultMaxCachedItems = 256;
|
public const int DefaultMaxCachedItems = 256;
|
||||||
|
|
||||||
|
/// <summary>E_INVALIDARG, which the interop raises as ArgumentException.</summary>
|
||||||
|
private const int EInvalidArg = unchecked((int)0x80070057);
|
||||||
|
|
||||||
private readonly IMxAccessComObjectFactory? factory;
|
private readonly IMxAccessComObjectFactory? factory;
|
||||||
private readonly MxAccessWriteCompletionCache completionCache;
|
private readonly MxAccessWriteCompletionCache completionCache;
|
||||||
private readonly Action pumpStep;
|
private readonly Action pumpStep;
|
||||||
@@ -104,12 +107,20 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
if (string.IsNullOrWhiteSpace(itemAddress)) throw new ArgumentException("Ack item address is required.", nameof(itemAddress));
|
if (string.IsNullOrWhiteSpace(itemAddress)) throw new ArgumentException("Ack item address is required.", nameof(itemAddress));
|
||||||
if (ackOperator is null) throw new ArgumentNullException(nameof(ackOperator));
|
if (ackOperator is null) throw new ArgumentNullException(nameof(ackOperator));
|
||||||
|
|
||||||
|
// Resolve every argument before the interop try blocks: their catch
|
||||||
|
// filters accept E_INVALIDARG ArgumentExceptions as MXAccess failures, so
|
||||||
|
// an argument error raised by this code must fail before reaching them.
|
||||||
|
if (ackOperator.UserName is null) throw new ArgumentException("Operator user name is required.", nameof(ackOperator));
|
||||||
|
string verifyUser = ackOperator.VerifyUser;
|
||||||
|
string password = ackOperator.Password ?? string.Empty;
|
||||||
|
string commentValue = comment ?? string.Empty;
|
||||||
|
|
||||||
IMxAccessServer mx = EnsureRegistered();
|
IMxAccessServer mx = EnsureRegistered();
|
||||||
|
|
||||||
int userId;
|
int userId;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
userId = mx.AuthenticateUser(serverHandle, ackOperator.VerifyUser, ackOperator.Password ?? string.Empty);
|
userId = mx.AuthenticateUser(serverHandle, verifyUser, password);
|
||||||
}
|
}
|
||||||
catch (Exception ex) when (IsInteropFailure(ex))
|
catch (Exception ex) when (IsInteropFailure(ex))
|
||||||
{
|
{
|
||||||
@@ -130,7 +141,7 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
return AlarmAckResult.Create(
|
return AlarmAckResult.Create(
|
||||||
AlarmAckOutcome.AuthenticationFailed,
|
AlarmAckOutcome.AuthenticationFailed,
|
||||||
ex.HResult,
|
ex.HResult,
|
||||||
$"Operator authentication failed for '{ackOperator.VerifyUser}' (HRESULT 0x{ex.HResult:X8}).");
|
$"Operator authentication failed for '{verifyUser}' (HRESULT 0x{ex.HResult:X8}).");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (userId == 0)
|
if (userId == 0)
|
||||||
@@ -138,7 +149,7 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
return AlarmAckResult.Create(
|
return AlarmAckResult.Create(
|
||||||
AlarmAckOutcome.AuthenticationFailed,
|
AlarmAckOutcome.AuthenticationFailed,
|
||||||
-1,
|
-1,
|
||||||
$"Operator authentication failed for '{ackOperator.VerifyUser}' (no user id returned).");
|
$"Operator authentication failed for '{verifyUser}' (no user id returned).");
|
||||||
}
|
}
|
||||||
|
|
||||||
ulong baselineVersion = 0;
|
ulong baselineVersion = 0;
|
||||||
@@ -177,7 +188,7 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
pumpStep();
|
pumpStep();
|
||||||
step = "Write";
|
step = "Write";
|
||||||
baselineVersion = completionCache.CurrentVersion(serverHandle, itemHandle);
|
baselineVersion = completionCache.CurrentVersion(serverHandle, itemHandle);
|
||||||
mx.Write(serverHandle, itemHandle, comment ?? string.Empty, userId);
|
mx.Write(serverHandle, itemHandle, commentValue, userId);
|
||||||
}
|
}
|
||||||
catch (Exception ex) when (IsInteropFailure(ex))
|
catch (Exception ex) when (IsInteropFailure(ex))
|
||||||
{
|
{
|
||||||
@@ -249,10 +260,23 @@ public sealed class AlarmAckWriter : IAlarmAckWriter
|
|||||||
/// ArgumentException, E_ACCESSDENIED to UnauthorizedAccessException) and
|
/// ArgumentException, E_ACCESSDENIED to UnauthorizedAccessException) and
|
||||||
/// the rest to COMException. Each carries the HRESULT in Exception.HResult.
|
/// the rest to COMException. Each carries the HRESULT in Exception.HResult.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// The ArgumentException arm exists because a wrong password or unknown
|
||||||
|
/// user makes <c>AuthenticateUser</c> throw ArgumentException with
|
||||||
|
/// E_INVALIDARG (0x80070057), observed live on windev on 2026-10-19. It
|
||||||
|
/// is narrowed to that HRESULT, and excludes ArgumentNullException and
|
||||||
|
/// ArgumentOutOfRangeException, so a programming error inside the try
|
||||||
|
/// blocks propagates as a bug instead of being reported as an MXAccess
|
||||||
|
/// authentication or write failure. The interop never raises those two
|
||||||
|
/// subtypes for a COM HRESULT.
|
||||||
|
/// </remarks>
|
||||||
private static bool IsInteropFailure(Exception ex) =>
|
private static bool IsInteropFailure(Exception ex) =>
|
||||||
ex is ExternalException
|
ex is ExternalException
|
||||||
|| ex is ArgumentException
|
|| ex is UnauthorizedAccessException
|
||||||
|| ex is UnauthorizedAccessException;
|
|| (ex is ArgumentException
|
||||||
|
&& ex is not ArgumentNullException
|
||||||
|
&& ex is not ArgumentOutOfRangeException
|
||||||
|
&& ex.HResult == EInvalidArg);
|
||||||
|
|
||||||
private static bool IsServerGone(int hresult) =>
|
private static bool IsServerGone(int hresult) =>
|
||||||
hresult == unchecked((int)0x80010108) // RPC_E_DISCONNECTED
|
hresult == unchecked((int)0x80010108) // RPC_E_DISCONNECTED
|
||||||
|
|||||||
Reference in New Issue
Block a user