diff --git a/docs/FileSystemClient.md b/docs/FileSystemClient.md index f8d4d92e05..e1468f36cf 100644 --- a/docs/FileSystemClient.md +++ b/docs/FileSystemClient.md @@ -174,6 +174,11 @@ share a single instance: concurrent calls are serialised by an internal corrupt" contract. `DisposeAsync` issues `FileType.Close` exactly once even when called concurrently. +Server-issued file handles are opaque and bound to the Session that +opened them. They cannot be used by another Session, and the server +automatically closes all remaining handles when their owning Session +closes. + ## Error mapping OPC UA Bad status codes returned by the server are translated into the diff --git a/src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs b/src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs index 7d9135d4ef..ee9507fcda 100644 --- a/src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs +++ b/src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs @@ -222,6 +222,24 @@ private async ValueTask OnCreateFileAsync( "File name required.") }; } + + NodeId sessionId = NodeId.Null; + if (requestFileOpen) + { + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId validSessionId, + out ServiceResult sessionResult)) + { + return new CreateFileMethodStateResult + { + ServiceResult = sessionResult + }; + } + + sessionId = validSessionId; + } + string newPath = manager.CombineProviderPath(ProviderPath, fileName); try { @@ -248,7 +266,7 @@ private async ValueTask OnCreateFileAsync( } // Open with write + erase to mirror the spec's // "create + open for write" semantics. - ServiceResult openResult = handle.Open(0x6, out uint fileHandle); + ServiceResult openResult = handle.Open(sessionId, 0x6, out uint fileHandle); return new CreateFileMethodStateResult { ServiceResult = openResult, diff --git a/src/Opc.Ua.Server/FileSystem/FileHandle.cs b/src/Opc.Ua.Server/FileSystem/FileHandle.cs index 47489b60c1..dbd559402d 100644 --- a/src/Opc.Ua.Server/FileSystem/FileHandle.cs +++ b/src/Opc.Ua.Server/FileSystem/FileHandle.cs @@ -114,17 +114,20 @@ public string MimeType } } - public Stream? GetStream(uint fileHandle) + public Stream? GetStream(NodeId sessionId, uint fileHandle) { lock (m_lock) { - if (m_write != null && fileHandle == 1) + if (m_write != null && + fileHandle == m_write.Handle && + m_write.SessionId.Equals(sessionId)) { - return m_write; + return m_write.Stream; } - if (m_reads.TryGetValue(fileHandle, out Stream? stream)) + if (m_reads.TryGetValue(fileHandle, out OpenFile? openFile) && + openFile.SessionId.Equals(sessionId)) { - return stream; + return openFile.Stream; } return null; } @@ -135,9 +138,16 @@ public string MimeType /// Part 5 §C: 0x1 = Read, 0x2 = Write, 0x4 = EraseExisting, /// 0x8 = Append. /// - public ServiceResult Open(byte mode, out uint fileHandle) + public ServiceResult Open(NodeId sessionId, byte mode, out uint fileHandle) { fileHandle = 0u; + if (sessionId.IsNull) + { + return ServiceResult.Create( + StatusCodes.BadSessionIdInvalid, + "A valid Session is required to open a file."); + } + bool wantsRead = (mode & 0x1) != 0; bool wantsWrite = (mode & 0x2) != 0; @@ -167,17 +177,22 @@ public ServiceResult Open(byte mode, out uint fileHandle) Stream stream = m_provider .OpenReadAsync(ProviderPath, CancellationToken.None) .AsTask().GetAwaiter().GetResult(); + bool fileAlreadyOpen; lock (m_lock) { - if (m_write != null) + fileAlreadyOpen = m_write != null; + if (!fileAlreadyOpen) { - stream.Dispose(); - return ServiceResult.Create( - StatusCodes.BadInvalidState, - "File already open for write."); + fileHandle = CreateFileHandle(); + m_reads.Add(fileHandle, new OpenFile(fileHandle, sessionId, stream)); } - fileHandle = ++m_nextHandle; - m_reads.Add(fileHandle, stream); + } + if (fileAlreadyOpen) + { + stream.Dispose(); + return ServiceResult.Create( + StatusCodes.BadInvalidState, + "File already open for write."); } return ServiceResult.Good; } @@ -199,17 +214,22 @@ public ServiceResult Open(byte mode, out uint fileHandle) Stream writeStream = m_provider .OpenWriteAsync(ProviderPath, writeMode, CancellationToken.None) .AsTask().GetAwaiter().GetResult(); + bool fileAlreadyOpenForReadOrWrite; lock (m_lock) { - if (m_reads.Count != 0 || m_write != null) + fileAlreadyOpenForReadOrWrite = m_reads.Count != 0 || m_write != null; + if (!fileAlreadyOpenForReadOrWrite) { - writeStream.Dispose(); - return ServiceResult.Create( - StatusCodes.BadInvalidState, - "File already open for read or write."); + fileHandle = CreateFileHandle(); + m_write = new OpenFile(fileHandle, sessionId, writeStream); } - m_write = writeStream; - fileHandle = 1u; + } + if (fileAlreadyOpenForReadOrWrite) + { + writeStream.Dispose(); + return ServiceResult.Create( + StatusCodes.BadInvalidState, + "File already open for read or write."); } return ServiceResult.Good; } @@ -230,44 +250,123 @@ public ServiceResult Open(byte mode, out uint fileHandle) } } - public bool Close(uint fileHandle) + public bool Close(NodeId sessionId, uint fileHandle) { + Stream? stream = null; lock (m_lock) { - if (m_write != null && fileHandle == 1) + if (m_write != null && + fileHandle == m_write.Handle && + m_write.SessionId.Equals(sessionId)) { - m_write.Dispose(); + stream = m_write.Stream; m_write = null; - return true; } - if (m_reads.TryGetValue(fileHandle, out Stream? stream)) + else if (m_reads.TryGetValue(fileHandle, out OpenFile? openFile) && + openFile.SessionId.Equals(sessionId)) { - stream.Dispose(); + stream = openFile.Stream; m_reads.Remove(fileHandle); - return true; } } - return false; + stream?.Dispose(); + return stream != null; + } + + public void CloseSession(NodeId sessionId) + { + List streamsToClose = []; + lock (m_lock) + { + if (m_write != null && m_write.SessionId.Equals(sessionId)) + { + streamsToClose.Add(m_write.Stream); + m_write = null; + } + + var handlesToClose = new List(); + foreach (KeyValuePair entry in m_reads) + { + if (entry.Value.SessionId.Equals(sessionId)) + { + streamsToClose.Add(entry.Value.Stream); + handlesToClose.Add(entry.Key); + } + } + + foreach (uint fileHandle in handlesToClose) + { + m_reads.Remove(fileHandle); + } + } + + DisposeStreams(streamsToClose); } public void Dispose() { + List streamsToClose; lock (m_lock) { - m_write?.Dispose(); + streamsToClose = new List(m_reads.Count + (m_write != null ? 1 : 0)); + if (m_write != null) + { + streamsToClose.Add(m_write.Stream); + } m_write = null; - foreach (Stream stream in m_reads.Values) + foreach (OpenFile openFile in m_reads.Values) { - stream.Dispose(); + streamsToClose.Add(openFile.Stream); } m_reads.Clear(); } + + DisposeStreams(streamsToClose); + } + + private uint CreateFileHandle() + { + uint fileHandle; + do + { + fileHandle = BitConverter.ToUInt32( + Nonce.CreateRandomNonceData(sizeof(uint)), + 0); + } + while (fileHandle == 0 || + m_write?.Handle == fileHandle || + m_reads.ContainsKey(fileHandle)); + + return fileHandle; + } + + private static void DisposeStreams(List streams) + { + foreach (Stream stream in streams) + { + stream.Dispose(); + } } private readonly Lock m_lock = new(); - private readonly Dictionary m_reads = []; + private readonly Dictionary m_reads = []; private readonly IFileSystemProvider m_provider; - private uint m_nextHandle = 1; - private Stream? m_write; + private OpenFile? m_write; + + private sealed class OpenFile + { + public OpenFile(uint handle, NodeId sessionId, Stream stream) + { + Handle = handle; + SessionId = sessionId; + Stream = stream; + } + + public uint Handle { get; } + + public NodeId SessionId { get; } + + public Stream Stream { get; } + } } } diff --git a/src/Opc.Ua.Server/FileSystem/FileObjectState.cs b/src/Opc.Ua.Server/FileSystem/FileObjectState.cs index 86f9f1270a..84feb69bdc 100644 --- a/src/Opc.Ua.Server/FileSystem/FileObjectState.cs +++ b/src/Opc.Ua.Server/FileSystem/FileObjectState.cs @@ -250,7 +250,14 @@ private ServiceResult OnOpen(ISystemContext context, MethodState method, { return result; } - return handle.Open(mode, out fileHandle); + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + return handle.Open(sessionId, mode, out fileHandle); } private ServiceResult OnClose(ISystemContext context, MethodState method, @@ -260,10 +267,17 @@ private ServiceResult OnClose(ISystemContext context, MethodState method, { return result; } - return handle.Close(fileHandle) + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + return handle.Close(sessionId, fileHandle) ? ServiceResult.Good : ServiceResult.Create(StatusCodes.BadInvalidState, - "File handle could not be closed."); + "File handle is invalid, belongs to another Session, or is already closed."); } private ServiceResult OnSetPosition(ISystemContext context, MethodState method, @@ -273,7 +287,14 @@ private ServiceResult OnSetPosition(ISystemContext context, MethodState method, { return result; } - Stream? stream = handle.GetStream(fileHandle); + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + Stream? stream = handle.GetStream(sessionId, fileHandle); if (stream == null) { return ServiceResult.Create(StatusCodes.BadInvalidState, @@ -290,7 +311,14 @@ private ServiceResult OnGetPosition(ISystemContext context, MethodState method, { return result; } - Stream? stream = handle.GetStream(fileHandle); + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + Stream? stream = handle.GetStream(sessionId, fileHandle); if (stream == null) { return ServiceResult.Create(StatusCodes.BadInvalidState, @@ -307,7 +335,14 @@ private ServiceResult OnRead(ISystemContext context, MethodState method, { return result; } - Stream? stream = handle.GetStream(fileHandle); + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + Stream? stream = handle.GetStream(sessionId, fileHandle); if (stream == null) { return ServiceResult.Create(StatusCodes.BadInvalidState, @@ -341,7 +376,14 @@ private ServiceResult OnWrite(ISystemContext context, MethodState method, { return result; } - Stream? stream = handle.GetStream(fileHandle); + if (!FileSystemNodeManager.TryGetSessionId( + context, + out NodeId sessionId, + out result)) + { + return result; + } + Stream? stream = handle.GetStream(sessionId, fileHandle); if (stream == null) { return ServiceResult.Create(StatusCodes.BadInvalidState, diff --git a/src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs b/src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs index bf26f6a331..305314cbc2 100644 --- a/src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs +++ b/src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs @@ -160,31 +160,62 @@ protected override ValueTask GetManagerHandleAsync( /// public override ValueTask DeleteAddressSpaceAsync(CancellationToken cancellationToken = default) { + FileHandle[] handles; lock (m_lock) { - foreach (FileHandle handle in m_handles.Values) - { - handle.Dispose(); - } + handles = [.. m_handles.Values]; m_handles.Clear(); } + foreach (FileHandle handle in handles) + { + handle.Dispose(); + } + return base.DeleteAddressSpaceAsync(cancellationToken); } + /// + public override ValueTask SessionClosingAsync( + OperationContext context, + NodeId sessionId, + bool deleteSubscriptions, + CancellationToken cancellationToken = default) + { + FileHandle[] handles; + lock (m_lock) + { + handles = [.. m_handles.Values]; + } + + foreach (FileHandle handle in handles) + { + handle.CloseSession(sessionId); + } + + return base.SessionClosingAsync( + context, + sessionId, + deleteSubscriptions, + cancellationToken); + } + /// protected override void Dispose(bool disposing) { if (disposing) { + FileHandle[] handles; lock (m_lock) { - foreach (FileHandle handle in m_handles.Values) - { - handle.Dispose(); - } + handles = [.. m_handles.Values]; m_handles.Clear(); } + + foreach (FileHandle handle in handles) + { + handle.Dispose(); + } } base.Dispose(disposing); } @@ -299,6 +330,26 @@ internal NodeId GetParentNodeId(string providerPath) : FileSystemNodeId.BuildDirectory(parent, NamespaceIndex); } + internal static bool TryGetSessionId( + ISystemContext context, + out NodeId sessionId, + out ServiceResult result) + { + if (context is ISessionSystemContext sessionContext && + sessionContext.SessionId is { IsNull: false } validSessionId) + { + sessionId = validSessionId; + result = ServiceResult.Good; + return true; + } + + sessionId = NodeId.Null; + result = ServiceResult.Create( + StatusCodes.BadSessionIdInvalid, + "A valid Session is required for file-handle operations."); + return false; + } + /// /// Combines a parent provider path with a child name, taking /// care of empty parents and trailing slashes. diff --git a/tests/Opc.Ua.Server.Tests/FileSystem/DirectoryObjectStateTests.cs b/tests/Opc.Ua.Server.Tests/FileSystem/DirectoryObjectStateTests.cs index 55cd30c542..660f097d6f 100644 --- a/tests/Opc.Ua.Server.Tests/FileSystem/DirectoryObjectStateTests.cs +++ b/tests/Opc.Ua.Server.Tests/FileSystem/DirectoryObjectStateTests.cs @@ -57,6 +57,7 @@ public class DirectoryObjectStateTests private ITelemetryContext m_telemetry = null!; private FileSystemNodeManager m_manager = null!; private ISystemContext m_context = null!; + private NodeId m_sessionId; [SetUp] public void SetUp() @@ -93,7 +94,12 @@ private void UseProvider(IFileSystemProvider provider) Mock mockServer = DeterministicServerMock.Create(out _); mockServer.Setup(s => s.Telemetry).Returns(m_telemetry); m_manager = new FileSystemNodeManager(mockServer.Object, new ApplicationConfiguration(), provider); - m_context = m_manager.SystemContext; + m_sessionId = new NodeId("directory-session", 0); + var session = new Mock(); + session.Setup(s => s.Id).Returns(m_sessionId); + session.Setup(s => s.Identity).Returns(new Mock().Object); + session.Setup(s => s.PreferredLocales).Returns([]); + m_context = m_manager.SystemContext.Copy(session.Object); } private DirectoryObjectState CreateRootDirectory() @@ -168,6 +174,47 @@ public async Task CreateFileWithOpenReturnsFileHandleAsync() Assert.That(result.FileHandle, Is.GreaterThan(0u)); } + [Test] + public async Task CreateFileWithOpenWithoutSessionReturnsBadSessionIdInvalidAsync() + { + DirectoryObjectState state = CreateRootDirectory(); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + CreateFileMethodStateResult result = await state.CreateFile!.OnCallAsync!( + contextWithoutSession, + state.CreateFile, + state.NodeId, + "opened.txt", + true, + CancellationToken.None).ConfigureAwait(false); + + Assert.That( + result.ServiceResult.StatusCode.Code, + Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + Assert.That(result.FileHandle, Is.Zero); + Assert.That(File.Exists(Path.Combine(m_root, "opened.txt")), Is.False); + } + + [Test] + public async Task CreateFileWithOpenScopesFileHandleToCreatingSessionAsync() + { + DirectoryObjectState state = CreateRootDirectory(); + + CreateFileMethodStateResult result = await state.CreateFile!.OnCallAsync!( + m_context, state.CreateFile, state.NodeId, "scoped.txt", true, CancellationToken.None) + .ConfigureAwait(false); + Assert.That(ServiceResult.IsGood(result.ServiceResult), Is.True); + + FileHandle handle = m_manager.GetOrCreateHandle(result.FileNodeId, "scoped.txt")!; + var otherSessionId = new NodeId("other-directory-session", 0); + + bool closedByOtherSession = handle.Close(otherSessionId, result.FileHandle); + bool closedByCreatingSession = handle.Close(m_sessionId, result.FileHandle); + + Assert.That(closedByOtherSession, Is.False); + Assert.That(closedByCreatingSession, Is.True); + } + [Test] public async Task CreateFileWithEmptyNameReturnsBadInvalidArgumentAsync() { diff --git a/tests/Opc.Ua.Server.Tests/FileSystem/FileHandleTests.cs b/tests/Opc.Ua.Server.Tests/FileSystem/FileHandleTests.cs index 9a05ae411a..e6292e9616 100644 --- a/tests/Opc.Ua.Server.Tests/FileSystem/FileHandleTests.cs +++ b/tests/Opc.Ua.Server.Tests/FileSystem/FileHandleTests.cs @@ -30,8 +30,12 @@ #nullable enable using System; +using System.Collections.Generic; using System.IO; +using System.Runtime.CompilerServices; using System.Text; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using Opc.Ua.Server.FileSystem; @@ -50,6 +54,8 @@ public class FileHandleTests private const byte ModeEraseExisting = 0x4; private const byte ModeAppend = 0x8; + private static readonly NodeId s_sessionId = new("FileHandleSession", 0); + private static readonly NodeId s_otherSessionId = new("OtherFileHandleSession", 0); private string m_root = null!; [SetUp] @@ -90,18 +96,29 @@ public void OpenWithNoModeReturnsBadInvalidArgument() { using var handle = new FileHandle(CreateProvider(), "f.txt"); - ServiceResult result = handle.Open(0x0, out uint fileHandle); + ServiceResult result = handle.Open(s_sessionId, 0x0, out uint fileHandle); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidArgument)); Assert.That(fileHandle, Is.Zero); } + [Test] + public void OpenWithNullSessionReturnsBadSessionIdInvalid() + { + using var handle = new FileHandle(CreateProvider(), "f.txt"); + + ServiceResult result = handle.Open(NodeId.Null, ModeRead, out uint fileHandle); + + Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + Assert.That(fileHandle, Is.Zero); + } + [Test] public void OpenWithReadAndWriteReturnsBadInvalidArgument() { using var handle = new FileHandle(CreateProvider(), "f.txt"); - ServiceResult result = handle.Open(ModeRead | ModeWrite, out _); + ServiceResult result = handle.Open(s_sessionId, ModeRead | ModeWrite, out _); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidArgument)); } @@ -112,7 +129,7 @@ public void OpenWriteOnReadOnlyProviderReturnsBadUserAccessDenied() WriteFile("f.txt", "abc"); using var handle = new FileHandle(CreateProvider(isWritable: false), "f.txt"); - ServiceResult result = handle.Open(ModeWrite, out _); + ServiceResult result = handle.Open(s_sessionId, ModeWrite, out _); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadUserAccessDenied)); } @@ -122,7 +139,7 @@ public void OpenReadForMissingFileReturnsBadNotFound() { using var handle = new FileHandle(CreateProvider(), "missing.txt"); - ServiceResult result = handle.Open(ModeRead, out _); + ServiceResult result = handle.Open(s_sessionId, ModeRead, out _); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadNotFound)); } @@ -133,12 +150,12 @@ public void OpenReadSucceedsAndDispensesHandle() WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - ServiceResult result = handle.Open(ModeRead, out uint fileHandle); + ServiceResult result = handle.Open(s_sessionId, ModeRead, out uint fileHandle); Assert.That(ServiceResult.IsGood(result), Is.True); Assert.That(fileHandle, Is.GreaterThan(0u)); Assert.That(handle.OpenCount, Is.EqualTo(1)); - Assert.That(handle.GetStream(fileHandle), Is.Not.Null); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); } [Test] @@ -147,8 +164,8 @@ public void MultipleReadersGetDistinctHandles() WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeRead, out uint first); - handle.Open(ModeRead, out uint second); + handle.Open(s_sessionId, ModeRead, out uint first); + handle.Open(s_sessionId, ModeRead, out uint second); Assert.That(first, Is.Not.EqualTo(second)); Assert.That(handle.OpenCount, Is.EqualTo(2)); @@ -159,9 +176,9 @@ public void OpenWriteWhileReadingReturnsBadInvalidState() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeRead, out _); + handle.Open(s_sessionId, ModeRead, out _); - ServiceResult result = handle.Open(ModeWrite, out _); + ServiceResult result = handle.Open(s_sessionId, ModeWrite, out _); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidState)); } @@ -171,24 +188,59 @@ public void OpenReadWhileWritingReturnsBadInvalidState() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeWrite, out _); + handle.Open(s_sessionId, ModeWrite, out _); - ServiceResult result = handle.Open(ModeRead, out _); + ServiceResult result = handle.Open(s_sessionId, ModeRead, out _); Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidState)); } [Test] - public void OpenWriteReturnsHandleOne() + public void OpenReadWhileWriteHandleAlreadyOpenInternallyReturnsBadInvalidState() + { + // Uses a provider whose streams do not take exclusive OS-level + // file locks, so this exercises FileHandle's own in-memory + // "already open for write" guard rather than an IOException + // bubbling up from the OS. + using var handle = new FileHandle(new NonExclusiveStreamProvider(), "f.txt"); + handle.Open(s_sessionId, ModeWrite, out uint writeHandle); + + ServiceResult result = handle.Open(s_sessionId, ModeRead, out uint readHandle); + + Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(readHandle, Is.Zero); + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, writeHandle), Is.Not.Null); + } + + [Test] + public void OpenWriteWhileAnyHandleAlreadyOpenInternallyReturnsBadInvalidState() + { + // Same rationale as above, but for the write-open guard that + // rejects opening for write while any reader or writer holds + // the file. + using var handle = new FileHandle(new NonExclusiveStreamProvider(), "f.txt"); + handle.Open(s_sessionId, ModeRead, out uint readHandle); + + ServiceResult result = handle.Open(s_sessionId, ModeWrite, out uint writeHandle); + + Assert.That(result.StatusCode, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(writeHandle, Is.Zero); + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, readHandle), Is.Not.Null); + } + + [Test] + public void OpenWriteReturnsOpaqueHandle() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - ServiceResult result = handle.Open(ModeWrite, out uint fileHandle); + ServiceResult result = handle.Open(s_sessionId, ModeWrite, out uint fileHandle); Assert.That(ServiceResult.IsGood(result), Is.True); - Assert.That(fileHandle, Is.EqualTo(1u)); - Assert.That(handle.GetStream(1u), Is.Not.Null); + Assert.That(fileHandle, Is.Not.Zero); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); } [Test] @@ -197,11 +249,11 @@ public void OpenWriteWithEraseExistingTruncates() WriteFile("f.txt", "existing-content"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeWrite | ModeEraseExisting, out uint fileHandle); - Stream stream = handle.GetStream(fileHandle)!; + handle.Open(s_sessionId, ModeWrite | ModeEraseExisting, out uint fileHandle); + Stream stream = handle.GetStream(s_sessionId, fileHandle)!; byte[] payload = Encoding.UTF8.GetBytes("new"); stream.Write(payload, 0, payload.Length); - handle.Close(fileHandle); + handle.Close(s_sessionId, fileHandle); Assert.That(File.ReadAllText(Path.Combine(m_root, "f.txt")), Is.EqualTo("new")); } @@ -212,11 +264,11 @@ public void OpenWriteWithAppendKeepsExistingContent() WriteFile("f.txt", "a"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeWrite | ModeAppend, out uint fileHandle); - Stream stream = handle.GetStream(fileHandle)!; + handle.Open(s_sessionId, ModeWrite | ModeAppend, out uint fileHandle); + Stream stream = handle.GetStream(s_sessionId, fileHandle)!; byte[] payload = Encoding.UTF8.GetBytes("b"); stream.Write(payload, 0, payload.Length); - handle.Close(fileHandle); + handle.Close(s_sessionId, fileHandle); Assert.That(File.ReadAllText(Path.Combine(m_root, "f.txt")), Is.EqualTo("ab")); } @@ -226,7 +278,29 @@ public void GetStreamReturnsNullForUnknownHandle() { using var handle = new FileHandle(CreateProvider(), "f.txt"); - Assert.That(handle.GetStream(999u), Is.Null); + Assert.That(handle.GetStream(s_sessionId, 999u), Is.Null); + } + + [Test] + public void GetStreamReturnsNullForDifferentSession() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeRead, out uint fileHandle); + + Assert.That(handle.GetStream(s_otherSessionId, fileHandle), Is.Null); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); + } + + [Test] + public void GetStreamReturnsNullForDifferentSessionOnWriteHandle() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeWrite, out uint fileHandle); + + Assert.That(handle.GetStream(s_otherSessionId, fileHandle), Is.Null); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); } [Test] @@ -234,7 +308,76 @@ public void CloseUnknownHandleReturnsFalse() { using var handle = new FileHandle(CreateProvider(), "f.txt"); - Assert.That(handle.Close(42u), Is.False); + Assert.That(handle.Close(s_sessionId, 42u), Is.False); + } + + [Test] + public void CloseReturnsFalseForDifferentSessionOnWriteHandle() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeWrite, out uint fileHandle); + + bool closed = handle.Close(s_otherSessionId, fileHandle); + + Assert.That(closed, Is.False); + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); + } + + [Test] + public void CloseReturnsFalseForDifferentSessionOnReadHandle() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeRead, out uint fileHandle); + + bool closed = handle.Close(s_otherSessionId, fileHandle); + + Assert.That(closed, Is.False); + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); + } + + [Test] + public void CloseSessionRemovesOwnedWriteHandle() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeWrite, out uint fileHandle); + + handle.CloseSession(s_sessionId); + + Assert.That(handle.OpenCount, Is.Zero); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Null); + } + + [Test] + public void CloseSessionRemovesOwnedReadHandlesButKeepsOtherSessions() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeRead, out uint ownHandle); + handle.Open(s_otherSessionId, ModeRead, out uint otherHandle); + + handle.CloseSession(s_sessionId); + + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, ownHandle), Is.Null); + Assert.That(handle.GetStream(s_otherSessionId, otherHandle), Is.Not.Null); + } + + [Test] + public void CloseSessionForSessionWithNoOpenHandlesIsNoOp() + { + WriteFile("f.txt", "hello"); + using var handle = new FileHandle(CreateProvider(), "f.txt"); + handle.Open(s_sessionId, ModeRead, out uint fileHandle); + + handle.CloseSession(s_otherSessionId); + + Assert.That(handle.OpenCount, Is.EqualTo(1)); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Not.Null); } [Test] @@ -242,13 +385,13 @@ public void CloseReaderReleasesHandle() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeRead, out uint fileHandle); + handle.Open(s_sessionId, ModeRead, out uint fileHandle); - bool closed = handle.Close(fileHandle); + bool closed = handle.Close(s_sessionId, fileHandle); Assert.That(closed, Is.True); Assert.That(handle.OpenCount, Is.Zero); - Assert.That(handle.GetStream(fileHandle), Is.Null); + Assert.That(handle.GetStream(s_sessionId, fileHandle), Is.Null); } [Test] @@ -256,9 +399,9 @@ public void CloseWriterReleasesHandle() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeWrite, out uint fileHandle); + handle.Open(s_sessionId, ModeWrite, out uint fileHandle); - bool closed = handle.Close(fileHandle); + bool closed = handle.Close(s_sessionId, fileHandle); Assert.That(closed, Is.True); Assert.That(handle.OpenCount, Is.Zero); @@ -292,7 +435,7 @@ public void IsWriteableIsFalseWhileOpen() { WriteFile("f.txt", "hello"); using var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeRead, out _); + handle.Open(s_sessionId, ModeRead, out _); Assert.That(handle.IsWriteable, Is.False); } @@ -311,12 +454,75 @@ public void DisposeClosesAllStreams() { WriteFile("f.txt", "hello"); var handle = new FileHandle(CreateProvider(), "f.txt"); - handle.Open(ModeRead, out _); - handle.Open(ModeRead, out _); + handle.Open(s_sessionId, ModeRead, out _); + handle.Open(s_sessionId, ModeRead, out _); handle.Dispose(); Assert.That(handle.OpenCount, Is.Zero); } + + /// + /// Minimal fake whose read/write + /// streams are independent in-memory buffers rather than OS file + /// handles. Unlike , opening + /// the same path twice never fails at the OS level, which lets + /// tests drive 's own in-memory + /// already-open guards instead of the IOException fallback path. + /// + private sealed class NonExclusiveStreamProvider : IFileSystemProvider + { + public string MountName => "mount"; + + public bool IsWritable => true; + + public ValueTask GetEntryAsync(string path, CancellationToken ct) + { + return new ValueTask((FileSystemEntry?)null); + } + + public async IAsyncEnumerable EnumerateAsync( + string path, + [EnumeratorCancellation] CancellationToken ct) + { + await Task.CompletedTask.ConfigureAwait(false); + yield break; + } + + public ValueTask OpenReadAsync(string path, CancellationToken ct) + { + return new ValueTask((Stream)new MemoryStream()); + } + + public ValueTask OpenWriteAsync(string path, FileWriteMode mode, CancellationToken ct) + { + return new ValueTask((Stream)new MemoryStream()); + } + + public ValueTask CreateDirectoryAsync(string path, CancellationToken ct) + { + throw new NotSupportedException(); + } + + public ValueTask CreateFileAsync(string path, CancellationToken ct) + { + throw new NotSupportedException(); + } + + public ValueTask DeleteAsync(string path, CancellationToken ct) + { + throw new NotSupportedException(); + } + + public ValueTask MoveAsync(string source, string target, CancellationToken ct) + { + throw new NotSupportedException(); + } + + public ValueTask CopyAsync(string source, string target, CancellationToken ct) + { + throw new NotSupportedException(); + } + } } } diff --git a/tests/Opc.Ua.Server.Tests/FileSystem/FileObjectStateTests.cs b/tests/Opc.Ua.Server.Tests/FileSystem/FileObjectStateTests.cs index c320471ba4..e7e3470a31 100644 --- a/tests/Opc.Ua.Server.Tests/FileSystem/FileObjectStateTests.cs +++ b/tests/Opc.Ua.Server.Tests/FileSystem/FileObjectStateTests.cs @@ -32,6 +32,7 @@ using System; using System.IO; using System.Text; +using System.Threading.Tasks; using Moq; using NUnit.Framework; using Opc.Ua.Server.FileSystem; @@ -54,6 +55,8 @@ public class FileObjectStateTests private ITelemetryContext m_telemetry = null!; private FileSystemNodeManager m_manager = null!; private ISystemContext m_context = null!; + private Mock m_session = null!; + private NodeId m_sessionId; [SetUp] public void SetUp() @@ -66,7 +69,9 @@ public void SetUp() mockServer.Setup(s => s.Telemetry).Returns(m_telemetry); var provider = new PhysicalFileSystemProvider(m_root, "TestMount"); m_manager = new FileSystemNodeManager(mockServer.Object, new ApplicationConfiguration(), provider); - m_context = m_manager.SystemContext; + m_sessionId = new NodeId("file-session", 0); + m_session = CreateSession(m_sessionId); + m_context = m_manager.SystemContext.Copy(m_session.Object); } [TearDown] @@ -95,6 +100,15 @@ private FileObjectState CreateFileState(string providerPath, string? content = n return new FileObjectState(m_context, nodeId, providerPath, Path.GetFileName(providerPath)); } + private static Mock CreateSession(NodeId sessionId) + { + var session = new Mock(); + session.Setup(s => s.Id).Returns(sessionId); + session.Setup(s => s.Identity).Returns(new Mock().Object); + session.Setup(s => s.PreferredLocales).Returns([]); + return session; + } + [Test] public void OpenForReadThenReadReturnsFileContent() { @@ -115,6 +129,183 @@ public void OpenForReadThenReadReturnsFileContent() Assert.That(Encoding.UTF8.GetString(data.ToArray()), Is.EqualTo("hello")); } + [Test] + public void OpenReturnsDistinctOpaqueHandles() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + + uint firstHandle = 0; + uint secondHandle = 0; + ServiceResult firstResult = state.Open!.OnCall!( + m_context, state.Open, state.NodeId, 0x1, ref firstHandle); + ServiceResult secondResult = state.Open.OnCall!( + m_context, state.Open, state.NodeId, 0x1, ref secondHandle); + + Assert.That(ServiceResult.IsGood(firstResult), Is.True); + Assert.That(ServiceResult.IsGood(secondResult), Is.True); + Assert.That(firstHandle, Is.Not.Zero); + Assert.That(secondHandle, Is.Not.Zero); + Assert.That(secondHandle, Is.Not.EqualTo(firstHandle)); + } + + [Test] + public void OpenWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + uint fileHandle = 0; + ServiceResult result = state.Open!.OnCall!( + contextWithoutSession, state.Open, state.NodeId, 0x1, ref fileHandle); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + Assert.That(fileHandle, Is.Zero); + } + + [Test] + public void CloseWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + state.Open!.OnCall!(m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + ServiceResult result = state.Close!.OnCall!( + contextWithoutSession, state.Close, state.NodeId, fileHandle); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + } + + [Test] + public void SetPositionWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + state.Open!.OnCall!(m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + ServiceResult result = state.SetPosition!.OnCall!( + contextWithoutSession, state.SetPosition, state.NodeId, fileHandle, 1u); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + } + + [Test] + public void GetPositionWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + state.Open!.OnCall!(m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + ulong position = 0; + ServiceResult result = state.GetPosition!.OnCall!( + contextWithoutSession, state.GetPosition, state.NodeId, fileHandle, ref position); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + } + + [Test] + public void ReadWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + state.Open!.OnCall!(m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + ByteString data = default; + ServiceResult result = state.Read!.OnCall!( + contextWithoutSession, state.Read, state.NodeId, fileHandle, 1, ref data); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + } + + [Test] + public void WriteWithoutSessionReturnsBadSessionIdInvalid() + { + FileObjectState state = CreateFileState("out.txt"); + uint fileHandle = 0; + state.Open!.OnCall!(m_context, state.Open, state.NodeId, 0x2, ref fileHandle); + ISystemContext contextWithoutSession = m_manager.SystemContext.Copy(); + + var payload = ByteString.From([1, 2, 3]); + ServiceResult result = state.Write!.OnCall!( + contextWithoutSession, state.Write, state.NodeId, fileHandle, payload); + + Assert.That(result.StatusCode.Code, Is.EqualTo(StatusCodes.BadSessionIdInvalid)); + } + + [Test] + public void FileHandleMethodsRejectDifferentSession() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + ServiceResult openResult = state.Open!.OnCall!( + m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + Assert.That(ServiceResult.IsGood(openResult), Is.True); + + Mock otherSession = CreateSession(new NodeId("other-file-session", 0)); + ISystemContext otherContext = m_manager.SystemContext.Copy(otherSession.Object); + + ByteString data = default; + ServiceResult readResult = state.Read!.OnCall!( + otherContext, state.Read, state.NodeId, fileHandle, 1, ref data); + var payload = ByteString.From([1]); + ServiceResult writeResult = state.Write!.OnCall!( + otherContext, state.Write, state.NodeId, fileHandle, payload); + ulong position = 0; + ServiceResult getPositionResult = state.GetPosition!.OnCall!( + otherContext, state.GetPosition, state.NodeId, fileHandle, ref position); + ServiceResult setPositionResult = state.SetPosition!.OnCall!( + otherContext, state.SetPosition, state.NodeId, fileHandle, 1); + ServiceResult closeResult = state.Close!.OnCall!( + otherContext, state.Close, state.NodeId, fileHandle); + + Assert.Multiple(() => + { + Assert.That(readResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(writeResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(getPositionResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(setPositionResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(closeResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + }); + } + + [Test] + public async Task SessionClosingClosesOwnedFileHandlesAsync() + { + FileObjectState state = CreateFileState("data.txt", "hello"); + uint fileHandle = 0; + ServiceResult openResult = state.Open!.OnCall!( + m_context, state.Open, state.NodeId, 0x1, ref fileHandle); + Assert.That(ServiceResult.IsGood(openResult), Is.True); + + Mock otherSession = CreateSession(new NodeId("other-file-session", 0)); + ISystemContext otherContext = m_manager.SystemContext.Copy(otherSession.Object); + uint otherFileHandle = 0; + ServiceResult otherOpenResult = state.Open.OnCall!( + otherContext, state.Open, state.NodeId, 0x1, ref otherFileHandle); + Assert.That(ServiceResult.IsGood(otherOpenResult), Is.True); + + var operationContext = new OperationContext(m_session.Object, DiagnosticsMasks.None); + await m_manager.SessionClosingAsync( + operationContext, + m_sessionId, + deleteSubscriptions: false).ConfigureAwait(false); + + ByteString data = default; + ServiceResult readResult = state.Read!.OnCall!( + m_context, state.Read, state.NodeId, fileHandle, 1, ref data); + ServiceResult otherReadResult = state.Read.OnCall!( + otherContext, state.Read, state.NodeId, otherFileHandle, 1, ref data); + Variant openCount = ReadProperty(state.OpenCount!); + + Assert.That(readResult.StatusCode.Code, Is.EqualTo(StatusCodes.BadInvalidState)); + Assert.That(ServiceResult.IsGood(otherReadResult), Is.True); + Assert.That(openCount.TryGetValue(out ushort count), Is.True); + Assert.That(count, Is.EqualTo(1)); + } + [Test] public void ReadWithShortRemainderTrimsBuffer() { diff --git a/tests/Opc.Ua.Server.Tests/FileSystem/FileSystemNodeManagerTests.cs b/tests/Opc.Ua.Server.Tests/FileSystem/FileSystemNodeManagerTests.cs index dcccc01344..a5cca65c07 100644 --- a/tests/Opc.Ua.Server.Tests/FileSystem/FileSystemNodeManagerTests.cs +++ b/tests/Opc.Ua.Server.Tests/FileSystem/FileSystemNodeManagerTests.cs @@ -385,6 +385,53 @@ await manager.DeleteAddressSpaceAsync(CancellationToken.None) Assert.That(afterDelete, Is.Not.SameAs(first)); } + [Test] + public void DisposeClosesOpenFileHandles() + { + FileSystemNodeManager manager = CreateManager(out _); + NodeId nodeId = FileSystemNodeId.BuildFile("f.txt", manager.NamespaceIndex); + FileHandle handle = manager.GetOrCreateHandle(nodeId, "f.txt")!; + var sessionId = new NodeId("dispose-session", 0); + ServiceResult openResult = handle.Open(sessionId, 0x2, out uint fileHandle); + Assert.That(ServiceResult.IsGood(openResult), Is.True); + + manager.Dispose(); + + Assert.That(handle.OpenCount, Is.Zero); + Assert.That(handle.GetStream(sessionId, fileHandle), Is.Null); + } + + [Test] + public async Task SessionClosingAsyncClosesHandlesOwnedBySessionAcrossMultipleFilesAsync() + { + using FileSystemNodeManager manager = CreateManager(out _); + NodeId nodeIdA = FileSystemNodeId.BuildFile("a.txt", manager.NamespaceIndex); + NodeId nodeIdB = FileSystemNodeId.BuildFile("b.txt", manager.NamespaceIndex); + FileHandle handleA = manager.GetOrCreateHandle(nodeIdA, "a.txt")!; + FileHandle handleB = manager.GetOrCreateHandle(nodeIdB, "b.txt")!; + + var sessionId = new NodeId("closing-session", 0); + var otherSessionId = new NodeId("other-closing-session", 0); + handleA.Open(sessionId, 0x2, out uint fileHandleA); + handleB.Open(otherSessionId, 0x2, out uint fileHandleB); + + var session = new Mock(); + session.Setup(s => s.Id).Returns(sessionId); + session.Setup(s => s.Identity).Returns(new Mock().Object); + session.Setup(s => s.PreferredLocales).Returns([]); + var operationContext = new OperationContext(session.Object, DiagnosticsMasks.None); + + await manager.SessionClosingAsync( + operationContext, + sessionId, + deleteSubscriptions: false).ConfigureAwait(false); + + Assert.That(handleA.OpenCount, Is.Zero); + Assert.That(handleA.GetStream(sessionId, fileHandleA), Is.Null); + Assert.That(handleB.OpenCount, Is.EqualTo(1)); + Assert.That(handleB.GetStream(otherSessionId, fileHandleB), Is.Not.Null); + } + [Test] public void FactoryWithNullProviderThrows() {