Fix the Opc.Ua.Types defects found by the read-only audit

src/Opc.Ua.Types/BuiltIn/ArrayOf.cs

@@ -580,7 +580,7 @@ public ArrayOf<T> AddItem(T value, int index)
         [Pure]
         public ArrayOf<T> ReplaceItem(T value, int index)
         {
-            if (index < 0 || index > Count)
+            if (index < 0 || index >= Count)
             {
                 throw new ArgumentOutOfRangeException(nameof(index));
             }

src/Opc.Ua.Types/BuiltIn/DateTimeUtc.cs

@@ -330,15 +330,17 @@ public static explicit operator DateTime(DateTimeUtc value)
         /// </summary>
         public DateTimeUtc AddMilliseconds(double milliseconds)
         {
-            return new DateTimeUtc(Value + (long)(milliseconds * TimeSpan.TicksPerMillisecond));
+            return new DateTimeUtc(
+                SaturatingAdd(Value, MillisecondsToTicks(milliseconds)));
         }
 
         /// <summary>
         /// Subtract milliseconds
         /// </summary>
         public DateTimeUtc SubtractMilliseconds(double milliseconds)
         {
-            return new DateTimeUtc(Value - (long)(milliseconds * TimeSpan.TicksPerMillisecond));
+            return new DateTimeUtc(
+                SaturatingSubtract(Value, MillisecondsToTicks(milliseconds)));
         }
 
         /// <summary>
@@ -354,15 +356,15 @@ public TimeSpan Subtract(DateTimeUtc value)
         /// </summary>
         public DateTimeUtc Add(TimeSpan value)
         {
-            return new DateTimeUtc(Value + value.Ticks);
+            return new DateTimeUtc(SaturatingAdd(Value, value.Ticks));
         }
 
         /// <summary>
         /// Subtract time
         /// </summary>
         public DateTimeUtc Subtract(TimeSpan value)
         {
-            return new DateTimeUtc(Value - value.Ticks);
+            return new DateTimeUtc(SaturatingSubtract(Value, value.Ticks));
         }
 
         /// <inheritdoc/>
@@ -444,16 +446,16 @@ public bool TryFormat(
         public static DateTimeUtc Parse(ReadOnlySpan<char> s, IFormatProvider? provider)
         {
 #if NET8_0_OR_GREATER
-            return DateTime.Parse(s, provider);
+            return DateTime.Parse(s, provider, kParseStyles);
 #else
-            return DateTime.Parse(new string(s.ToArray()), provider);
+            return DateTime.Parse(new string(s.ToArray()), provider, kParseStyles);
 #endif
         }
 
         /// <inheritdoc/>
         public static DateTimeUtc Parse(string s, IFormatProvider? provider)
         {
-            return DateTime.Parse(s, provider);
+            return DateTime.Parse(s, provider, kParseStyles);
         }
 
         /// <inheritdoc/>
@@ -465,7 +467,7 @@ public static bool TryParse(
             if (DateTime.TryParse(
                 s,
                 provider,
-                DateTimeStyles.AssumeUniversal,
+                kParseStyles,
                 out DateTime dt))
             {
                 result = dt;
@@ -488,7 +490,7 @@ public static bool TryParse(
                 new string(s.ToArray()),
 #endif
                 provider,
-                DateTimeStyles.AssumeUniversal,
+                kParseStyles,
                 out DateTime dt))
             {
                 result = dt;
@@ -557,6 +559,65 @@ internal static long ToFileTimeUtc(long ticks)
             return EnsureBounded(ticks - kTickOffset);
         }
 
+        /// <summary>
+        /// Adds a tick offset without wrapping around. <see cref="Value"/>
+        /// reports <see cref="long.MaxValue"/> for the "no expiry" sentinel, so
+        /// plain addition overflows into a negative file time and the bounds
+        /// check would then clamp it to <see cref="MinValue"/>.
+        /// </summary>
+        private static long SaturatingAdd(long value, long ticks)
+        {
+            long result = unchecked(value + ticks);
+            if (ticks > 0 && result < value)
+            {
+                return long.MaxValue;
+            }
+            if (ticks < 0 && result > value)
+            {
+                return long.MinValue;
+            }
+            return result;
+        }
+
+        /// <summary>
+        /// Subtracts a tick offset without wrapping around. Negating
+        /// <see cref="long.MinValue"/> is not representable - unchecked it
+        /// yields <see cref="long.MinValue"/> again - so subtracting the most
+        /// negative offset, which is mathematically a very large addition,
+        /// would saturate to <see cref="MinValue"/> instead of
+        /// <see cref="MaxValue"/>.
+        /// </summary>
+        private static long SaturatingSubtract(long value, long ticks)
+        {
+            return ticks == long.MinValue
+                ? long.MaxValue
+                : SaturatingAdd(value, -ticks);
+        }
+
+        /// <summary>
+        /// Converts a millisecond offset to ticks. A double to long conversion
+        /// saturates on .NET 5 and later but yields <see cref="long.MinValue"/>
+        /// for NaN and for out of range values on .NET Framework, so the
+        /// conversion is done explicitly to keep every target identical.
+        /// </summary>
+        private static long MillisecondsToTicks(double milliseconds)
+        {
+            double ticks = milliseconds * TimeSpan.TicksPerMillisecond;
+            if (double.IsNaN(ticks))
+            {
+                return 0;
+            }
+            if (ticks >= long.MaxValue)
+            {
+                return long.MaxValue;
+            }
+            if (ticks <= long.MinValue)
+            {
+                return long.MinValue;
+            }
+            return (long)ticks;
+        }
+
         /// <summary>
         /// Ensure the file time is in the correct boundaries defined
         /// by the specification
@@ -579,7 +640,11 @@ private static long EnsureBounded(long fileTime)
         /// <inheritdoc/>
         public static DateTimeUtc From(string s)
         {
-            return DateTime.ParseExact(s, "yyyy-MM-dd HH:mm:ss", CultureInfo.InvariantCulture);
+            return DateTime.ParseExact(
+                s,
+                "yyyy-MM-dd HH:mm:ss",
+                CultureInfo.InvariantCulture,
+                kParseStyles);
         }
 
         /// <inheritdoc/>
@@ -606,6 +671,14 @@ public string ToString(string? format, IFormatProvider? provider)
             return ToDateTime().ToString(format, provider);
         }
 
+        /// <summary>
+        /// Text without a time zone designator denotes UTC, not machine local
+        /// time. Without this a zone-less timestamp is shifted by the local
+        /// offset when it is converted into the UTC file time.
+        /// </summary>
+        private const DateTimeStyles kParseStyles =
+            DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal;
+
         private const long kMaxValue = 2650467743990000000;
         private const long kTickOffset = 584388 * TimeSpan.TicksPerDay;
         private readonly long m_value;

src/Opc.Ua.Types/BuiltIn/DiagnosticInfo.cs

@@ -199,7 +199,12 @@ private DiagnosticInfo(
 
             if (!serviceLevel)
             {
-                mask >>= 5;
+                // The operation level bits sit five positions above their
+                // service level counterparts, but UserPermissionAdditionalInfo
+                // is an internal flag outside those pairs and must survive the
+                // shift - otherwise operation level AdditionalInfo is dropped.
+                const uint userPermission = (uint)DiagnosticsMasks.UserPermissionAdditionalInfo;
+                mask = ((mask & ~userPermission) >> 5) | (mask & userPermission);
             }
 
             diagnosticsMask = (DiagnosticsMasks)mask;
@@ -226,7 +231,12 @@ public DiagnosticInfo(
 
             if (!serviceLevel)
             {
-                mask >>= 5;
+                // The operation level bits sit five positions above their
+                // service level counterparts, but UserPermissionAdditionalInfo
+                // is an internal flag outside those pairs and must survive the
+                // shift - otherwise operation level AdditionalInfo is dropped.
+                const uint userPermission = (uint)DiagnosticsMasks.UserPermissionAdditionalInfo;
+                mask = ((mask & ~userPermission) >> 5) | (mask & userPermission);
             }
 
             diagnosticsMask = (DiagnosticsMasks)mask;

src/Opc.Ua.Types/BuiltIn/ExpandedNodeId.cs

@@ -452,7 +452,7 @@ public int CompareTo(object? obj)
             // check for null.
             if (obj is null)
             {
-                return IsNull ? 0 : -1;
+                return IsNull ? 0 : 1;
             }
 
             // just compare node ids.
@@ -494,7 +494,8 @@ public int CompareTo(object? obj)
             }
             else
             {
-                nodeId = NodeId.Null;
+                // Not comparable to a foreign type - never report equality.
+                return -1;
             }
 
             // check for null.
@@ -538,6 +539,7 @@ public override bool Equals(object? obj)
             {
                 null => IsNull,
                 ExpandedNodeId e => Equals(e),
+                NodeId n => Equals(n),
                 _ => false
             };
         }
@@ -900,10 +902,12 @@ public void Format(IFormatProvider? formatProvider, StringBuilder buffer)
             }
             else
             {
+                // The identifier itself must still be written, otherwise the
+                // result ("nsu=<uri>;i=") cannot be parsed back.
                 Format(
                     formatProvider,
                     buffer,
-                    null!,
+                    "0",
                     IdType.Numeric,
                     0,
                     NamespaceUri,
@@ -1411,14 +1415,25 @@ private static bool InternalTryParseWithContext(
                     return false;
                 }
 
-                if (ushort.TryParse(text[4..index], out ushort ns))
+                if (!uint.TryParse(
+                    text[4..index],
+                    NumberStyles.None,
+                    CultureInfo.InvariantCulture,
+                    out uint svr))
                 {
-                    serverIndex = ns;
+                    // An unparsable or out of range index must not be silently
+                    // dropped - that would turn a remote node id into a local
+                    // one, and TryParse would report success while doing it.
+                    error = NodeIdParseError.InvalidServerIndex;
+                    return false;
+                }
 
-                    if (options?.ServerMappings != null && options.NamespaceMappings != null && options.NamespaceMappings.Length < ns)
-                    {
-                        serverIndex = options.NamespaceMappings[ns];
-                    }
+                serverIndex = (int)svr;
+
+                if (options?.ServerMappings != null &&
+                    svr < (uint)options.ServerMappings.Length)
+                {
+                    serverIndex = options.ServerMappings[svr];
                 }
 
                 text = text[(index + 1)..];
@@ -1461,8 +1476,11 @@ private static bool InternalTryParseWithContext(
                 return false;
             }
 
-            if (namespaceIndex > 0)
+            if (namespaceUri != null && namespaceIndex >= 0)
             {
+                // A resolved nsu= becomes a relative node id, also when it
+                // resolves to namespace zero, so that the result matches what
+                // NodeId.Parse produces for the equivalent ns= form.
                 value = new ExpandedNodeId(
                     WithResolvedNamespace(nodeId, (ushort)namespaceIndex),
                     null,

src/Opc.Ua.Types/BuiltIn/MatrixOf.cs

@@ -140,11 +140,23 @@ private MatrixOf(Array array)
                     indexes[array.Rank - row - 1] =
                         element / multiplier % dimensions[row];
                 }
-                values[element] = (T)
-                    (array.GetValue(indexes)
-                        ?? throw new ArgumentException(
+                object? item = array.GetValue(indexes);
+                if (item is null)
+                {
+                    // Reference types (and nullable value types) legitimately
+                    // carry unset entries, only value types cannot be null.
+                    if (default(T) is not null)
+                    {
+                        throw new ArgumentException(
                             "array contains null",
-                            nameof(array)));
+                            nameof(array));
+                    }
+                    values[element] = default!;
+                }
+                else
+                {
+                    values[element] = (T)item;
+                }
             }
             m_memory = values;
             m_dimensions =