Commit Graph
1 Commits
Author SHA1 Message Date
romanettandClaude Opus 5 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>
2026-09-14 18:51:49 +00:00