mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-29 07:07:24 +08:00
copilot/fix-arrayof-toarrayof-issue
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
aee30e9bf6 |
Echo the RequestHandle in ServiceFaults for requests that cannot be decoded (#4488)
# Description OPC 10000-4 §7.33 (ServiceFault): *"The requestHandle in the ResponseHeader should be set to what was provided in the RequestHeader even if these values were not valid."* When the decoder rejects a request, for example a string argument above `MaxStringLength` (`BadEncodingLimitsExceeded`), the server answered with a ServiceFault carrying RequestHandle 0 and no Timestamp. The OPC Foundation CTT reports this: *"CallResponseHeader.RequestHandle does not match expected RequestHandle. Expected: 18 Received: 0"*. It was seen during the CTT GDS triage of #4481, with a FindApplications ApplicationUri of MaxStringLength + 10 % characters. ### Changes - **`RequestHandleReader`** (new, internal, `Opc.Ua.Core`) reads the RequestHandle best effort from a message that could not be decoded: - binary: the encoding NodeId, then the leading RequestHeader fields AuthenticationToken (NodeId), Timestamp (UtcTime) and RequestHandle (UInt32); - JSON: `UaBody.RequestHeader.RequestHandle`, ignoring same-named members anywhere else. It returns 0 when the handle cannot be read. - **`TcpServerChannel` / `TcpListenerChannel`:** new `SendServiceFault` overloads take the RequestHandle, and faults now carry a response Timestamp. `ProcessRequestMessage` (decode failure, null request, discovery-only rejection) and the OpenSecureChannel error path pass the handle. This covers `opc.tcp`, the Kestrel TCP listener and UA-SC over WebSocket, which all use `TcpServerChannel`. - **`EndpointBase.CreateFault`:** - new internal overload with a fallback RequestHandle for a request that was not decoded; - the Timestamp is always set; - no longer dereferences a null RequestHeader. - **`HttpsTransportListener`:** - The JSON path and both JSON-over-WebSocket loops (`opcua+uajson`, OpenAPI) now echo the handle read from the payload. - Discovery-only faults use the handle of the decoded request. ### ⚠️ Behaviour change for reviewers: HTTPS binary A binary HTTPS request the decoder rejects used to get **HTTP 500 with a text body** and no ServiceFault. It now gets **HTTP 200 with a ServiceFault** carrying the decoder status and the echoed RequestHandle, the same as the JSON path in the same listener. Other exceptions in that path still return HTTP 500. OPC 10000-6 §7.4 does not specify how an HTTPS server reports an undecodable request. The existing test `SendAsyncReturnsInternalServerErrorForInvalidBodyAsync` was replaced by `SendAsyncRespondsWithServiceFaultForInvalidBodyAsync`. If HTTP 500 should be kept, that hunk can be reverted independently. WebApi REST (`WebApiServer`) already echoed the handle and is unchanged. ### Tests - `RequestHandleReaderTests`: - binary messages above MaxStringLength, with numeric and string authentication tokens, from a byte array and from a stream; - truncated and null input; - JSON field order, decoy `RequestHandle` members, wrong types and malformed JSON; - `CreateFault` fallback and precedence. - `TcpServerChannelBufferTests.UndecodableRequestServiceFaultEchoesRequestHandleAsync`: feeds a ReadRequest with a 640-character NodeId (MaxStringLength 64) through `TcpServerChannel`. It asserts that the sent ServiceFault has `BadEncodingLimitsExceeded`, RequestHandle 4711, the request id and a Timestamp. - `HttpsTransportListenerTests`: the binary ServiceFault for an invalid body, the echoed handle for an oversized binary request, and the echoed handle for an undecodable JSON request. - No unit test covers the two JSON-over-WebSocket loops; the test infrastructure has no WebSocket driver, and the change there is mechanical. Local results (net10.0): - `Opc.Ua.Core.Tests`: 4,669 passed, 0 failed, 88 skipped. - `Opc.Ua.Bindings.Https.WebApi.Tests`: 346 passed. - `Opc.Ua.Client.Tests`: 2,266 passed. `Opc.Ua.Core.Tests` also builds for net48. `dotnet format whitespace --verify-no-changes` passes on the changed files. ## Related Issues - Found while triaging the CTT GDS group in #4481 (`plans/ctt-issues.md` there, "Open server findings"). ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |