diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index 3c642275a..8a86cf29a 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -489,3 +489,93 @@ jobs: TestResults/**/coverage.cobertura.xml if-no-files-found: warn retention-days: 14 + + # ------------------------------------------------------------------ + # Job 4 — Release-pack matrix gate. The build-and-test job above only + # exercises Debug, which compiles each csproj against a single TFM + # (net8.0 — see the per-project `` + # blocks). Release / Package configurations multi-target net4x → + # net9.0; some diagnostics only fire on the legacy TFMs (CS0162 from + # `#if NET5_0_OR_GREATER` branches that go unreachable on net4x), and + # some only on net9.0 (SYSLIB0057 X509Certificate2 ctor obsoletion). + # The May-22 regression slipped through CI precisely because Debug + # never built net4x; this job runs `dotnet pack -c Release` across + # the full TFM matrix to catch the next regression class at PR time. + # + # Single-process (no shard), no docs dependency, same draft-skip + # gate as build-and-test. The pack output is written to a workflow- + # local `dist/` directory and discarded — the produced .nupkg files + # are not uploaded as artefacts; the gate's only output is the exit + # code. + # ------------------------------------------------------------------ + release-pack: + name: release-pack + if: github.event_name == 'push' || github.event.pull_request.draft == false + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - name: Setup .NET 8.0 + 9.0 + uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4 + with: + dotnet-version: | + 8.0.x + 9.0.x + + - name: Restore solution + run: dotnet restore MTConnect.NET.sln + + - name: Pack -c Release across multi-TFM + run: | + rm -rf dist + mkdir -p dist + dotnet pack MTConnect.NET.sln -c Release \ + -p:VersionSuffix=ci-check \ + -p:ContinuousIntegrationBuild=true \ + -o dist 2>&1 | tee pack.log + shell: bash + + # Surface a compact summary of every error / warning that + # tripped the pack. The `dotnet pack` step above pipes into + # `tee pack.log`; the unmasked exit propagates via the + # pipeline's PIPESTATUS through `set -o pipefail`, which is + # the GitHub-default for bash steps. This step only runs on + # failure, by design — when the pack is green, the produced + # nupkgs are not inspected further. + # + # The regex matches any MSBuild-shaped diagnostic — + # `error XX0000` / `warning XX0000` where XX is one or more + # letters (CS, CA, NU, SYSLIB, MSB, NETSDK, IL, XA, StyleCop, + # Roslyn analyzers, and mixed-case analyzer families like + # `xUnit1004` / `nunit1001` that ship lowercase prefixes). + # The prior `(CS|CA|NU|SYSLIB|MSB)` enumeration silently + # dropped every family outside that set and produced empty + # summaries when the pack failed on any of them. The + # broadened pattern is a strict superset so no failure + # family is masked. `[A-Za-z]+[0-9]+` keeps the "letters + # then digits" shape so bare `warning:` prose lines and + # counter lines like `2 Warning(s)` are not matched. + # + # The `head -100` cap is a paste-into-summary safety valve; + # when it triggers, a `showing 100 of N` breadcrumb makes + # the truncation explicit so a maintainer triaging a + # summary-visible slice knows the full failure set is + # larger than what is rendered. + - name: Surface pack errors (if any) + if: failure() + run: | + diag_pattern='\b(error|warning)[[:space:]]+[A-Za-z]+[0-9]+' + diag_total=$(grep -cE "$diag_pattern" pack.log 2>/dev/null || echo 0) + diag_unique=$(grep -E "$diag_pattern" pack.log 2>/dev/null | sort -u | wc -l) + echo "### Release-pack diagnostics" >> "$GITHUB_STEP_SUMMARY" + if [ "$diag_unique" -gt 100 ]; then + echo "_Showing first 100 of ${diag_unique} unique diagnostics (${diag_total} total matching lines)._" >> "$GITHUB_STEP_SUMMARY" + elif [ "$diag_unique" -gt 0 ]; then + echo "_${diag_unique} unique diagnostics (${diag_total} total matching lines)._" >> "$GITHUB_STEP_SUMMARY" + fi + echo '```' >> "$GITHUB_STEP_SUMMARY" + grep -E "$diag_pattern" pack.log \ + | sort -u | head -100 >> "$GITHUB_STEP_SUMMARY" || true + echo '```' >> "$GITHUB_STEP_SUMMARY" + shell: bash diff --git a/adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs b/adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs index 69f6ee21e..5f94b2b26 100644 --- a/adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs +++ b/adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs @@ -45,7 +45,7 @@ public class MTConnectAdapterApplication : IMTConnectAdapterApplication #if NET5_0_OR_GREATER /// NLog log-level applied to every internal logger. - /// Defaults to ; the debug + /// Defaults to ; the debug /// CLI command overrides it. protected LogLevel _logLevel = LogLevel.Debug; #else diff --git a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs index 4b604371c..117d12393 100644 --- a/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs +++ b/agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs @@ -55,7 +55,7 @@ public class MTConnectAgentApplication : IMTConnectAgentApplication #if NET5_0_OR_GREATER /// NLog log-level applied to every internal logger. - /// Defaults to ; the debug + /// Defaults to ; the debug /// and trace CLI commands override it. protected LogLevel _logLevel = LogLevel.Debug; #else diff --git a/docs/testing/workflows.md b/docs/testing/workflows.md index 543fc313b..0e2f9f988 100644 --- a/docs/testing/workflows.md +++ b/docs/testing/workflows.md @@ -96,6 +96,22 @@ package-write privileges. The sweep is opt-in via `dotnet test tests/Compliance/MTConnect-Compliance-Tests/MTConnect-Compliance-Tests.csproj --filter "Category=XsdLoadStrict"`. +## CI workflow — `release-pack` (multi-TFM Release-pack gate) + +Sibling job in `.github/workflows/dotnet.yml`. Runs on every push to `master` and every non-draft PR. Executes `dotnet pack MTConnect.NET.sln -c Release` across the full net461 → net9.0 TFM matrix — a stricter surface than `build-test-coverage`, because Release configuration enables NuGet package generation (`.nupkg` + `.snupkg`), rich API doc surfaces on every TFM, and the multi-TFM `SupportedOSPlatform` / `LangVersion` gates the Debug matrix does not exercise. + +**Purpose:** guard against the class of regressions the 2026-05-22 landing bypassed — a Debug-only CI floor let CS-family Release-only diagnostics ship as PR-level warnings that only surfaced during release packaging. The gate turns every such regression into a red PR check. + +**Exit contract:** must be RC=0 (zero errors, zero MSBuild-shaped diagnostics — every `error XX0000` / `warning XX0000` family, e.g. CS / CA / NU / SYSLIB / MSB / NETSDK / IL / StyleCop / xUnit / NUnit / third-party analyzers) to merge. The remaining MSB3277 assembly-conflict + NETSDK1138 EOL-TFM + transitive-package net461-compat build-summary warnings are pre-existing and are not tracked by this gate; the FLOOR is only against new code-level diagnostics. When the pack fails, the `Surface pack errors` step renders a step-summary table of the unique matching lines (capped at 100 with an explicit truncation breadcrumb naming the full total). + +**Local repro** (matches CI): + +```bash +dotnet pack MTConnect.NET.sln -c Release +``` + +The command runs across every TFM configured in each project's `TargetFrameworks`. Failing quickly on net461 (the strictest TFM for `SupportedOSPlatform` / `LangVersion` guards) is the fastest sanity check when triaging a Release-only diagnostic locally. + ## Why the integration project is a separate CI step The integration project drives the in-process Agent + embedded HTTP diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Common/Interfaces.cs b/libraries/MTConnect.NET-HTTP/Ceen/Common/Interfaces.cs index 2fdfb7051..d3379bda5 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Common/Interfaces.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Common/Interfaces.cs @@ -267,6 +267,16 @@ internal interface IHttpResponse /// An optional content type to set. Throws an exception if the headers are already sent. Task WriteAllAsync(Stream data, string contenttype = null); + /// + /// Copies the stream to the output while honouring the supplied cancellation token. Note that the stream is copied from the current position to the end, and the stream must report the length. + /// Same bug class as MTConnectPostResponseHandler.ReadRequestBytes (dime F-IMP-001): a client abort mid-response must short-circuit the copy rather than fully drain the response body into a disconnected socket. + /// + /// The awaitable task + /// The stream to copy. + /// Cancellation token forwarded to the underlying CopyToAsync. + /// An optional content type to set. Throws an exception if the headers are already sent. + Task WriteAllAsync(Stream data, System.Threading.CancellationToken cancellationToken, string contenttype = null); + /// /// Writes the byte array to the output. /// diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/Handler/SimpleProxyHandler.cs b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/Handler/SimpleProxyHandler.cs index 397c9d857..ef1f1196c 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/Handler/SimpleProxyHandler.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/Handler/SimpleProxyHandler.cs @@ -66,7 +66,17 @@ public async Task HandleAsync(IHttpContext context, CancellationToken canc wr.Method = context.Request.Method; if (context.Request.ContentLength > 0) using (var rs = await wr.GetRequestStreamAsync()) - await context.Request.Body.CopyToAsync(rs); + // Same bug class as MTConnectPostResponseHandler.ReadRequestBytes + // (dime F-IMP-001): a request-body drain must honour the outer + // cancellation token so a client abort short-circuits the copy + // rather than reading the full body into the proxied upstream. + // Uses the (Stream, int bufferSize, CancellationToken) overload + // (universal since .NET 4.5); the 2-arg (Stream, CancellationToken) + // shape is netstandard2.1 / net5+ only and cannot ship on the + // library's net4x / netstandard2.0 targets. The 81920 buffer + // matches the .NET runtime default for the tokenless + // CopyToAsync overload so throughput is unchanged. + await context.Request.Body.CopyToAsync(rs, 81920, cancellationToken); using (var res = await GetResponseWithoutExceptionAsync(wr)) { @@ -83,12 +93,19 @@ public async Task HandleAsync(IHttpContext context, CancellationToken canc await context.Response.FlushHeadersAsync(); using (var r = context.Response.GetResponseStream()) using (var rr = res.GetResponseStream()) - -#if NET5_0_OR_GREATER - await rr.CopyToAsync(r, context.Request.TimeoutCancellationToken); -#else - await rr.CopyToAsync(r); -#endif + // Same bug class as the request-body drain 21 lines above + // (dime F-IMP-005) and the sibling + // MTConnectPostResponseHandler.ReadRequestBytes (dime F-IMP-001): + // a slow upstream response must not block a client abort. + // Uses the universal 3-arg (Stream, int bufferSize, + // CancellationToken) overload so net461-net48 / + // netstandard2.0 / net6.0 also honour the token — the + // pre-existing #if NET5_0_OR_GREATER guard left those six + // TFMs on a tokenless CopyToAsync, letting a slow-response + // upstream fully drain after cancel. The 81920 buffer + // matches the runtime default for the tokenless overload + // so throughput on the happy path is unchanged. + await rr.CopyToAsync(r, 81920, context.Request.TimeoutCancellationToken); } return true; diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpResponse.cs b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpResponse.cs index f01395eed..708596fe8 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpResponse.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpResponse.cs @@ -453,12 +453,24 @@ internal async Task FlushAsErrorAsync() /// The stream to copy. /// An optional content type to set. Throws an exception if the headers are already sent. public Task WriteAllAsync(Stream data, string contenttype = null) + => WriteAllAsync(data, System.Threading.CancellationToken.None, contenttype); + + /// + /// Copies the stream to the output while honouring the supplied cancellation token. Note that the stream is copied from the current position to the end, and the stream must report the length. + /// Same bug class as MTConnectPostResponseHandler.ReadRequestBytes (dime F-IMP-001): a client abort mid-response must short-circuit the copy rather than fully drain the response body into a disconnected socket. + /// Uses the universal (Stream, int bufferSize, CancellationToken) CopyToAsync overload (since .NET 4.5) so every supported TFM honours the token; the 81920 buffer matches the runtime default for the tokenless overload. + /// + /// The awaitable task + /// The stream to copy. + /// Cancellation token forwarded to the underlying CopyToAsync. + /// An optional content type to set. Throws an exception if the headers are already sent. + public Task WriteAllAsync(Stream data, System.Threading.CancellationToken cancellationToken, string contenttype = null) { if (contenttype != null) ContentType = contenttype; if (!HasSentHeaders) ContentLength = data.Length - data.Position; - return data.CopyToAsync(m_wrappedoutstream); + return data.CopyToAsync(m_wrappedoutstream, 81920, cancellationToken); } /// diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs index 0e173d38e..4e2c1956e 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs @@ -203,17 +203,31 @@ public bool WaitForStop(TimeSpan waitdelay) /// public int ActiveClients { get { return Controller.ActiveClients; } } -#if NET5_0_OR_GREATER /// /// Initializes the lifetime service. /// /// The lifetime service. + /// + /// The base is + /// only marked obsolete on .NET 5 and newer (CoreCLR removed the .NET + /// Remoting lifetime-service infrastructure there). On .NET Framework + /// the base is not obsolete, and applying + /// on the override would trigger CS0809 + /// (obsolete override of non-obsolete base). The attribute is therefore + /// conditioned on net5+ — silencing the net5+ CS0672 (non-obsolete + /// override of obsolete member) without forbidding the override on + /// net4x where remoting is still live. Returning null pins the + /// server object's lifetime to the process on every TFM (without this + /// override, .NET Framework Remoting hands the server a five-minute + /// default lease and may collect it after idle expiry). + /// +#if NET5_0_OR_GREATER [Obsolete("InitializeLifetimeService is obsolete in .NET 5+; the override exists for legacy AppDomain remoting compatibility.")] +#endif public override object InitializeLifetimeService() { return null; } -#endif } /// diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs index 779395338..684964882 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs @@ -153,11 +153,24 @@ public async Task DiscardAllAsync(System.Threading.CancellationToken cance var buf = new byte[1024 * 8]; while (m_bytesleft > 0) { -#if NET9_0_OR_GREATER - await ReadExactlyAsync(buf, 0, buf.Length, cancellationToken); -#else - await ReadAsync(buf, 0, buf.Length, cancellationToken); -#endif + // CA2022 short-read handling — TFM-uniform. Every supported TFM + // lands on the same shape: loop ReadAsync until the transport + // signals EOF (return 0) or the whole body is drained. ReadAsync + // may return fewer bytes than requested on multi-segment TCP + // arrivals; the loop keeps calling until m_bytesleft hits zero + // (drain complete → return true) or a 0-byte read indicates + // premature EOF (return false so the caller can propagate the + // drain failure to the outer HTTP handler). + // + // Do NOT re-introduce a ReadExactlyAsync-into-fixed-buffer shape + // on any TFM: when the remaining body is smaller than buf.Length + // (the common case on the final iteration and on any body + // smaller than 8 KB), ReadExactlyAsync throws + // EndOfStreamException and propagates up through the outer + // HttpServer catch, killing keep-alive and 500-ing the client. + var read = await ReadAsync(buf, 0, buf.Length, cancellationToken); + if (read == 0) + return false; } return true; diff --git a/libraries/MTConnect.NET-HTTP/Ceen/Mvc/RestApiHelper.cs b/libraries/MTConnect.NET-HTTP/Ceen/Mvc/RestApiHelper.cs index b82a8956f..5dd916a62 100644 --- a/libraries/MTConnect.NET-HTTP/Ceen/Mvc/RestApiHelper.cs +++ b/libraries/MTConnect.NET-HTTP/Ceen/Mvc/RestApiHelper.cs @@ -136,9 +136,24 @@ public virtual async Task Post(IHttpContext context) TData item; // TODO: Accept non-utf8 ? // TODO: Get the Json Async version + // Same bug class as MTConnectPostResponseHandler.ReadRequestBytes + // (dime F-IMP-001): the request-body drain honours the request-timeout / + // abort cancellation token surfaced on IHttpRequestInternal so a client + // abort short-circuits the read rather than blocking on the drained + // StreamReader. Precheck the token before starting so a + // pre-cancelled request short-circuits without allocating a reader; + // on .NET 7+, StreamReader.ReadToEndAsync(CancellationToken) then + // honours cancellation mid-read. Older TFMs (net4x, netstandard2.0, + // net6.0) only have the tokenless overload — the precheck is the + // best-effort surface until the vendored Ceen tree drops those TFMs. + context.Request.TimeoutCancellationToken.ThrowIfCancellationRequested(); using (var sr = new StreamReader(context.Request.Body, System.Text.Encoding.UTF8, false)) { +#if NET7_0_OR_GREATER + var str = await sr.ReadToEndAsync(context.Request.TimeoutCancellationToken); +#else var str = await sr.ReadToEndAsync(); +#endif item = JsonSerializer.Deserialize(str); } @@ -206,9 +221,24 @@ public virtual async Task PutDetail(IHttpContext context, TKey id) return Status(HttpStatusCode.BadRequest, "Invalid ID"); TData item; + // Same bug class as MTConnectPostResponseHandler.ReadRequestBytes + // (dime F-IMP-001): the request-body drain honours the request-timeout / + // abort cancellation token surfaced on IHttpRequestInternal so a client + // abort short-circuits the read rather than blocking on the drained + // StreamReader. Precheck the token before starting so a + // pre-cancelled request short-circuits without allocating a reader; + // on .NET 7+, StreamReader.ReadToEndAsync(CancellationToken) then + // honours cancellation mid-read. Older TFMs (net4x, netstandard2.0, + // net6.0) only have the tokenless overload — the precheck is the + // best-effort surface until the vendored Ceen tree drops those TFMs. + context.Request.TimeoutCancellationToken.ThrowIfCancellationRequested(); using (var sr = new StreamReader(context.Request.Body, System.Text.Encoding.UTF8, false)) { +#if NET7_0_OR_GREATER + var str = await sr.ReadToEndAsync(context.Request.TimeoutCancellationToken); +#else var str = await sr.ReadToEndAsync(); +#endif item = JsonSerializer.Deserialize(str); } diff --git a/libraries/MTConnect.NET-HTTP/Servers/MTConnectHttpResponseHandler.cs b/libraries/MTConnect.NET-HTTP/Servers/MTConnectHttpResponseHandler.cs index bad3ddd00..a65b23a32 100644 --- a/libraries/MTConnect.NET-HTTP/Servers/MTConnectHttpResponseHandler.cs +++ b/libraries/MTConnect.NET-HTTP/Servers/MTConnectHttpResponseHandler.cs @@ -64,7 +64,7 @@ public async Task HandleAsync(IHttpContext context, CancellationToken canc acceptEncodings = ProcessAcceptEncodings(acceptEncodings); var mtconnectResponse = await OnRequestReceived(context, cancellationToken); - mtconnectResponse.WriteDuration = await WriteResponse(mtconnectResponse, context.Response, acceptEncodings); + mtconnectResponse.WriteDuration = await WriteResponse(mtconnectResponse, context.Response, acceptEncodings, cancellationToken); ResponseSent.Raise(this, mtconnectResponse, ClientException); @@ -102,7 +102,7 @@ protected async virtual Task OnRequestReceived(IHttpConte /// /// Write a MTConnectHttpResponse to the HttpListenerResponse Output Stream /// - protected async Task WriteResponse(MTConnectHttpResponse mtconnectResponse, IHttpResponse httpResponse, IEnumerable acceptEncodings = null) + protected async Task WriteResponse(MTConnectHttpResponse mtconnectResponse, IHttpResponse httpResponse, IEnumerable acceptEncodings = null, CancellationToken cancellationToken = default) { var stpw = System.Diagnostics.Stopwatch.StartNew(); @@ -113,7 +113,7 @@ protected async Task WriteResponse(MTConnectHttpResponse mtconnectRespon httpResponse.ContentType = mtconnectResponse.ContentType; httpResponse.StatusCode = (Ceen.HttpStatusCode)mtconnectResponse.StatusCode; - await WriteToStream(mtconnectResponse.Content, httpResponse, acceptEncodings); + await WriteToStream(mtconnectResponse.Content, httpResponse, acceptEncodings, cancellationToken); } catch { } } @@ -125,7 +125,7 @@ protected async Task WriteResponse(MTConnectHttpResponse mtconnectRespon /// /// Write a string to the HttpListenerResponse Output Stream /// - protected async Task WriteResponse(string content, IHttpResponse httpResponse, Ceen.HttpStatusCode statusCode, string contentType = MimeTypes.XML, IEnumerable acceptEncodings = null) + protected async Task WriteResponse(string content, IHttpResponse httpResponse, Ceen.HttpStatusCode statusCode, string contentType = MimeTypes.XML, IEnumerable acceptEncodings = null, CancellationToken cancellationToken = default) { if (httpResponse != null) { @@ -135,13 +135,13 @@ protected async Task WriteResponse(string content, IHttpResponse httpResponse, C httpResponse.StatusCode = statusCode; var contentStream = new MemoryStream(Encoding.UTF8.GetBytes(content)); - await WriteToStream(contentStream, httpResponse, acceptEncodings); + await WriteToStream(contentStream, httpResponse, acceptEncodings, cancellationToken); } catch { } } } - protected async Task WriteToStream(Stream inputStream, IHttpResponse httpResponse, IEnumerable acceptEncodings = null) + protected async Task WriteToStream(Stream inputStream, IHttpResponse httpResponse, IEnumerable acceptEncodings = null, CancellationToken cancellationToken = default) { if (httpResponse != null && inputStream != null && inputStream.Length > 0) { @@ -163,7 +163,7 @@ protected async Task WriteToStream(Stream inputStream, IHttpResponse httpRespons inputStream.CopyTo(zip); } outputStream.Seek(0, SeekOrigin.Begin); - await httpResponse.WriteAllAsync(outputStream); + await httpResponse.WriteAllAsync(outputStream, cancellationToken); } #if NET5_0_OR_GREATER @@ -179,7 +179,7 @@ protected async Task WriteToStream(Stream inputStream, IHttpResponse httpRespons inputStream.CopyTo(zip); } outputStream.Seek(0, SeekOrigin.Begin); - await httpResponse.WriteAllAsync(outputStream); + await httpResponse.WriteAllAsync(outputStream, cancellationToken); } #endif @@ -195,12 +195,12 @@ protected async Task WriteToStream(Stream inputStream, IHttpResponse httpRespons inputStream.CopyTo(zip); } outputStream.Seek(0, SeekOrigin.Begin); - await httpResponse.WriteAllAsync(outputStream); + await httpResponse.WriteAllAsync(outputStream, cancellationToken); } else { - await httpResponse.WriteAllAsync(inputStream); + await httpResponse.WriteAllAsync(inputStream, cancellationToken); } } catch { } diff --git a/libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs b/libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs index 4040e4728..e449554f3 100644 --- a/libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs +++ b/libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs @@ -40,7 +40,7 @@ protected async override Task OnRequestReceived(IHttpCont if (httpRequest != null && httpRequest.Path != null && httpResponse != null) { - var requestBytes = await ReadRequestBytes(context.Request.Body); + var requestBytes = await ReadRequestBytes(context.Request.Body, cancellationToken); if (!requestBytes.IsNullOrEmpty()) { var urlSegments = GetUriSegments(httpRequest.Path); @@ -97,7 +97,7 @@ protected async override Task OnRequestReceived(IHttpCont return response; } - private static async Task ReadRequestBytes(Stream inputStream) + private static async Task ReadRequestBytes(Stream inputStream, CancellationToken cancellationToken) { if (inputStream != null) { @@ -106,27 +106,77 @@ private static async Task ReadRequestBytes(Stream inputStream) var bufferSize = 1048576 * 2; // 2 MB var bytes = new byte[bufferSize]; -#if NET9_0_OR_GREATER - await inputStream.ReadExactlyAsync(bytes, 0, bytes.Length); -#else - await inputStream.ReadAsync(bytes, 0, bytes.Length); -#endif - - return TrimEnd(bytes); + // CA2022 short-read accumulator — TFM-uniform. Every supported + // TFM lands on the same shape: loop ReadAsync until EOF or the + // buffer is full, then truncate to the actually-filled length. + // The underlying stream (Ceen.Httpd.LimitedBodyStream or the + // hosting server's request body) already respects Content-Length + // framing at a lower layer; the accumulator only needs to + // survive multi-segment TCP arrivals. Matches the boost::beast + // HTTP parser cppagent uses (src/mtconnect/sink/rest_sink/ + // session_impl.cpp:176-181), which likewise returns the exact + // body length rather than a zero-padded buffer needing TrimEnd. + // + // Do NOT re-introduce a ReadExactlyAsync-into-fixed-buffer + + // TrimEnd shape: ReadExactlyAsync on a body smaller than the + // buffer throws EndOfStreamException (silently swallowed by the + // outer catch, dropping the request), and TrimEnd would corrupt + // any payload whose final legitimate byte is 0x00. + // + // Cancellation is threaded to every ReadAsync so a client + // abort (Ceen surfaces it as the OnRequestReceived + // cancellationToken parameter) short-circuits the accumulator + // rather than draining the full 2 MB. Matches the sibling + // LimitedBodyStream.DiscardAllAsync which likewise takes a + // CancellationToken and forwards it to its ReadAsync loop. + var totalRead = 0; + while (totalRead < bytes.Length) + { + var read = await inputStream.ReadAsync(bytes, totalRead, bytes.Length - totalRead, cancellationToken); + if (read == 0) break; + totalRead += read; + } + if (totalRead == bytes.Length) + return bytes; + var result = new byte[totalRead]; + Array.Copy(bytes, 0, result, 0, totalRead); + return result; + } + catch (OperationCanceledException) + { + // Caller-driven cancellation is a legitimate signal, not a + // transport error. Propagate so the outer Ceen pipeline can + // honour the abort — swallowing here would translate the + // aborted request into a benign 404 / null-body response and + // mask the abort from telemetry and the request lifecycle. + throw; + } + catch (Exception ex) + { + // Transport / IO failures are intentionally swallowed to the + // caller's null-return contract (a malformed asset POST must + // not tear down the request pipeline), but leave a Trace + // breadcrumb naming the exception type so operators tailing + // a Trace listener can distinguish "aborted upstream" from + // "silent drop" without recompiling. Trace is BCL-only and + // costs nothing when no listener is attached; the original + // "swallow everything" semantics are preserved. + // + // Strip CR / LF from ex.Message before emitting so a nested + // exception whose Message carries a newline cannot split + // the trace line and forge a second-record entry when a + // TextWriterTraceListener / FileLogTraceListener is wired + // — OWASP A09 log-format-injection defence. + var safeMessage = ex.Message == null + ? string.Empty + : ex.Message.Replace('\r', ' ').Replace('\n', ' '); + System.Diagnostics.Trace.WriteLine( + $"MTConnectPostResponseHandler.ReadRequestBytes swallowed " + + $"{ex.GetType().FullName}: {safeMessage}"); } - catch { } } return null; } - - public static byte[] TrimEnd(byte[] array) - { - int lastIndex = Array.FindLastIndex(array, b => b != 0); - - Array.Resize(ref array, lastIndex + 1); - - return array; - } } } diff --git a/libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csproj b/libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csproj index 65be39a44..5b640d904 100644 --- a/libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csproj +++ b/libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csproj @@ -28,8 +28,17 @@ true - - true + + true snupkg diff --git a/tests/MTConnect.NET-Common-Tests/Http/CA2022NoTrimEndOnFixedBufferTests.cs b/tests/MTConnect.NET-Common-Tests/Http/CA2022NoTrimEndOnFixedBufferTests.cs new file mode 100644 index 000000000..2f9e4561e --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Http/CA2022NoTrimEndOnFixedBufferTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System.Linq; +using System.Reflection; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Http +{ + /// + /// Permanent regression guard for the F-CR-001 TrimEnd-on-fixed-buffer class of bug in + /// MTConnectPostResponseHandler.ReadRequestBytes. The pre-fix net9 branch called + /// Stream.ReadExactlyAsync(bytes, 0, 2 MB) then handed the buffer to a + /// TrimEnd helper that dropped trailing 0x00 bytes to work around the fact that + /// the buffer was never actually filled. That shape is wrong on two axes: ReadExactlyAsync + /// on a body smaller than the buffer throws + /// (silently swallowed by the outer catch, returning null and dropping the request); + /// and even where the exact-read semantics happen to work, TrimEnd corrupts payloads + /// whose final legitimate byte is 0x00 (binary MTConnect assets, UTF-8 documents padded + /// with NUL, and so on). + /// + /// The canonical shape — matching the boost::beast HTTP parser cppagent uses + /// (src/mtconnect/sink/rest_sink/session_impl.cpp:176-181) — is a short-read + /// accumulator that respects the underlying stream's Content-Length-aware framing and + /// truncates to the actually-filled length. The fix unifies every TFM on that pattern + /// and deletes the TrimEnd(byte[]) helper outright. + /// + /// This fixture asserts that the helper stays gone; if a future edit re-introduces it or + /// the branching guard around it, the fixture goes RED before the change lands. + /// + [TestFixture] + [Category("CA2022NoTrimEndOnFixedBuffer")] + public class CA2022NoTrimEndOnFixedBufferTests + { + /// Pins that the TrimEnd(byte[]) helper on MTConnectPostResponseHandler is deleted — re-adding it signals a TrimEnd-on-fixed-buffer regression. + [Test] + public void MTConnectPostResponseHandler_has_no_TrimEnd_helper() + { + var anchor = typeof(MTConnect.Servers.Http.MTConnectHttpServer); + var handlerType = anchor.Assembly.GetType( + "MTConnect.Servers.MTConnectPostResponseHandler", + throwOnError: false); + + Assert.That(handlerType, Is.Not.Null, + "MTConnectPostResponseHandler not visible via reflection — refactor may have renamed it."); + + var trimEnd = handlerType!.GetMethod( + "TrimEnd", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance, + binder: null, + types: new[] { typeof(byte[]) }, + modifiers: null); + + Assert.That(trimEnd, Is.Null, + "MTConnectPostResponseHandler.TrimEnd(byte[]) must not exist. The pre-fix helper dropped " + + "trailing 0x00 bytes from a 2 MB fixed buffer to compensate for a broken exact-read on " + + "net9 — corrupting payloads ending in a legitimate 0x00. The correct shape is a " + + "short-read accumulator (aligned with cppagent's boost::beast HTTP parser) that truncates " + + "to the actually-filled length. If this test fails, revert the TrimEnd re-addition and " + + "keep the accumulator."); + } + + /// Pins that ReadRequestBytes's IL contains no call to any TrimEnd method — a broader regression guard covering the class of bug beyond the specific helper name. + [Test] + public void ReadRequestBytes_IL_contains_no_TrimEnd_call() + { + var anchor = typeof(MTConnect.Servers.Http.MTConnectHttpServer); + var handlerType = anchor.Assembly.GetType( + "MTConnect.Servers.MTConnectPostResponseHandler", + throwOnError: false); + + Assert.That(handlerType, Is.Not.Null, + "MTConnectPostResponseHandler not visible via reflection — refactor may have renamed it."); + + var method = handlerType!.GetMethod( + "ReadRequestBytes", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance); + + Assert.That(method, Is.Not.Null, + "MTConnectPostResponseHandler.ReadRequestBytes not found — refactor may have renamed it."); + + // The compiler generates an async state machine; the real body lives on a nested + // struct named "d__N" with a MoveNext() method. Walk both surfaces. + var candidates = new System.Collections.Generic.List { method! }; + var stateMachineTypes = handlerType! + .GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic) + .Where(t => t.Name.Contains("ReadRequestBytes")); + foreach (var t in stateMachineTypes) + { + var moveNext = t.GetMethod("MoveNext", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + if (moveNext != null) candidates.Add(moveNext); + } + + foreach (var m in candidates) + { + var body = m.GetMethodBody(); + if (body == null) continue; + var il = body.GetILAsByteArray(); + if (il == null) continue; + // Scan for any method-token reference resolving to a method whose name is "TrimEnd". + var module = m.Module; + for (var i = 0; i + 4 < il.Length; i++) + { + var opcode = il[i]; + // call = 0x28, callvirt = 0x6F + if (opcode != 0x28 && opcode != 0x6F) continue; + var token = System.BitConverter.ToInt32(il, i + 1); + MethodBase? target = null; + try { target = module.ResolveMethod(token); } catch { } + if (target == null) continue; + Assert.That(target.Name, Is.Not.EqualTo("TrimEnd"), + "ReadRequestBytes calls TrimEnd — the F-CR-001 bug class has regressed. " + + "Remove the call and let the short-read accumulator's actual-length truncation " + + "carry the semantic."); + } + } + } + } +} diff --git a/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadEdgeCaseTests.cs b/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadEdgeCaseTests.cs new file mode 100644 index 000000000..0d1926553 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadEdgeCaseTests.cs @@ -0,0 +1,394 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Http +{ + /// + /// Boundary and failure-path coverage FLOOR (CONVENTIONS §1.0d-trigies-novodecies) + /// for the CA2022 short-read fix on + /// MTConnectPostResponseHandler.ReadRequestBytes. The sibling + /// CA2022ShortReadTests file pins the happy-path + /// "worst-case-one-byte-at-a-time-preserves-trailing-zero" contract but + /// leaves several input-class boundaries uncovered per the coverage + /// FLOOR panel: + /// + /// * empty body (0 bytes) — the loop exits immediately on the first + /// read; the returned array must be zero-length rather than the + /// pre-fix 2 MB of zero-padding. + /// * body length equals the 2 MB buffer exactly — the loop exits on + /// the buffer-full branch instead of on EOF; the returned array is + /// the original 2 MB verbatim (no truncation). + /// * pre-cancelled token — the reader honours cancellation at + /// entry; the accumulator propagates the OperationCanceledException + /// rather than swallowing it, so the outer Ceen pipeline can + /// surface the aborted-request signal to callers. + /// * cancelled mid-drip — a token cancelled while the reader is + /// awaiting the next chunk cancels the pending ReadAsync + /// within one read cycle and propagates OperationCanceledException. + /// * body larger than the 2 MB buffer — the loop stops at the buffer + /// size; the returned array is exactly the buffer size (extra + /// bytes discarded, no exception). + /// + [TestFixture] + [Category("CA2022ShortReadEdgeCase")] + public class CA2022ShortReadEdgeCaseTests + { + /// Pins the empty-body boundary: a stream that returns 0 on the first read produces a zero-length result — the pre-fix 2 MB zero-padded buffer never leaks out. + [Test] + public async Task ReadRequestBytes_returns_empty_array_on_empty_body() + { + using var empty = new ScriptedStream(new byte[0]); + var result = await Invoke(empty); + + Assert.That(result, Is.Not.Null, + "ReadRequestBytes must return an empty array — not null — for a legitimately empty body."); + Assert.That(result!.Length, Is.EqualTo(0), + "The 2 MB buffer must be truncated to the actually-filled length (0 for an empty body)."); + } + + /// Pins the buffer-fill boundary: a body whose length exactly matches the 2 MB internal buffer takes the "buffer full" exit branch rather than the EOF branch; the returned array is the original body verbatim, no truncation. + [Test] + public async Task ReadRequestBytes_returns_full_buffer_when_body_exactly_fills_buffer() + { + const int bufferSize = 2 * 1024 * 1024; + var body = new byte[bufferSize]; + for (var i = 0; i < body.Length; i++) + body[i] = (byte)(i % 251); + + using var full = new ScriptedStream(body, chunkSize: 4096); + var result = await Invoke(full); + + Assert.That(result, Is.Not.Null); + Assert.That(result!.Length, Is.EqualTo(bufferSize), + "Buffer-full exit branch must return the full 2 MB, not one byte short (off-by-one guard)."); + Assert.That(result, Is.EqualTo(body), + "Buffer-full body must be reconstructed byte-for-byte."); + } + + /// Pins the oversized-body boundary: a body larger than the 2 MB internal buffer is truncated at exactly the buffer size — the fix takes the "buffer full" branch and stops reading; no exception leaks. + [Test] + public async Task ReadRequestBytes_truncates_body_larger_than_buffer_without_throwing() + { + const int bufferSize = 2 * 1024 * 1024; + var body = new byte[bufferSize + 1024]; + for (var i = 0; i < body.Length; i++) + body[i] = (byte)((i + 1) % 251); + + using var big = new ScriptedStream(body, chunkSize: 8192); + var result = await Invoke(big); + + Assert.That(result, Is.Not.Null, + "Oversized body must return the truncated buffer, not null (the try/catch must not swallow a benign case)."); + Assert.That(result!.Length, Is.EqualTo(bufferSize), + "Oversized body must be truncated to exactly the 2 MB buffer size."); + } + + /// Pins the transport-error swallow contract: if the underlying stream throws a non-cancellation exception (e.g. an IOException from a broken transport, an InvalidDataException from a corrupt chunked-encoding envelope), the outer try/catch swallows and returns null so a malformed asset POST cannot tear down the request pipeline. Sibling ReadRequestBytes_cancelled_mid_drip_throws_within_next_read pins the inverted rule for cancellation — OperationCanceledException propagates rather than being swallowed here. + [Test] + public async Task ReadRequestBytes_returns_null_when_stream_throws() + { + using var throwing = new ThrowingStream(); + var result = await Invoke(throwing); + + Assert.That(result, Is.Null, + "The outer try/catch must swallow the underlying-stream exception and return null — the caller's null-as-error contract is stable."); + } + + /// Pins the null-input contract: passing a null Stream must not throw; the method must return null. The pre-fix method had the same shape via `if (inputStream != null)` — the fix preserves it. + [Test] + public async Task ReadRequestBytes_returns_null_for_null_input_stream() + { + var result = await Invoke(null); + + Assert.That(result, Is.Null, + "A null Stream input must be a benign null return, not a NullReferenceException."); + } + + /// Pins the pre-cancelled-token boundary: when the caller passes a token that is already cancelled at entry, ReadRequestBytes propagates OperationCanceledException rather than reading to completion or swallowing the cancellation into a benign null return. Pre-fix, the method's signature did not accept a token, so the token was ignored and the read proceeded — the assertion below fails RED on the pre-fix HEAD. + [Test] + public void ReadRequestBytes_pre_cancelled_token_throws_immediately() + { + var body = new byte[] { 0x01, 0x02, 0x03, 0x04 }; + using var scripted = new ScriptedStream(body); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + Assert.That( + async () => await Invoke(scripted, cts.Token), + Throws.InstanceOf(), + "A pre-cancelled token must surface as OperationCanceledException. " + + "Pre-fix, the accumulator ignored the token (signature took only Stream) " + + "and read the body to completion; post-fix, the token is honoured at the " + + "first ReadAsync and the outer catch preserves OperationCanceledException."); + } + + /// Pins the mid-drip cancellation boundary: when the token is cancelled while the reader is awaiting the next chunk, the pending ReadAsync cancels within one read cycle and OperationCanceledException propagates within ~200 ms rather than the full drip duration. Pre-fix, the token was ignored and the read completed after the full ~1 s drip — the timeout assertion fails RED on the pre-fix HEAD. + [Test] + public void ReadRequestBytes_cancelled_mid_drip_throws_within_next_read() + { + // 100-byte body, 1 byte per read with a 10 ms per-read delay + // → ~1 s to drain in the happy path. Cancel after ~50 ms and + // expect OperationCanceledException within ~200 ms. Pre-fix, + // the token was not threaded to the ReadAsync overload so the + // drip completes normally and the assertion times out RED. + var body = new byte[100]; + for (var i = 0; i < body.Length; i++) + body[i] = (byte)(i + 1); + using var slow = new ScriptedStream(body, chunkSize: 1, perReadDelay: TimeSpan.FromMilliseconds(10)); + using var cts = new CancellationTokenSource(); + + Assert.That( + async () => + { + var invocation = Invoke(slow, cts.Token); + // Give the reader time to start awaiting the first drip, + // then request cancellation. The next ReadAsync's + // Task.Delay(cancellationToken) throws immediately. + cts.CancelAfter(TimeSpan.FromMilliseconds(50)); + await invocation.WaitAsync(TimeSpan.FromMilliseconds(200)); + }, + Throws.InstanceOf(), + "A token cancelled mid-drip must surface as OperationCanceledException within " + + "the next ReadAsync cycle. Pre-fix, the accumulator ignored the token and " + + "read to completion after the full drip duration; post-fix, Task.Delay " + + "honours the token and the exception propagates through the outer catch."); + } + + // ----------------------------------------------------------------- + // Actually-real DiscardAllAsync exercise — the sibling fixture only + // pins the *shape* via a mirror loop. This one instantiates the + // internal `LimitedBodyStream` via reflection and asserts the true + // return contract on premature EOF. + // ----------------------------------------------------------------- + + /// Pins the actual real LimitedBodyStream.DiscardAllAsync return contract: when the underlying transport hits EOF before m_bytesleft reaches zero, DiscardAllAsync returns false and does NOT deadlock on the while (m_bytesleft > 0) gate. Uses reflection on the internal type in MTConnect.NET-HTTP. + [Test] + public async Task LimitedBodyStream_DiscardAllAsync_returns_false_on_premature_eof() + { + // Anchor on a public type from MTConnect.NET-HTTP.dll to force + // the assembly to load, then locate the internal + // Ceen.Httpd.LimitedBodyStream + Ceen.Httpd.BufferedStreamReader + // via GetType. + var anchor = typeof(MTConnect.Servers.Http.MTConnectHttpServer).Assembly; + var bodyStreamType = anchor.GetType("Ceen.Httpd.LimitedBodyStream", throwOnError: false); + var bufferedReaderType = anchor.GetType("Ceen.Httpd.BufferedStreamReader", throwOnError: false); + if (bodyStreamType == null || bufferedReaderType == null) + { + Assert.Inconclusive( + "Ceen.Httpd.LimitedBodyStream / BufferedStreamReader not visible via reflection — " + + "either the type was renamed or its assembly-internal access changed. " + + "Fixture skips gracefully; the sibling shape-mirror pin still runs."); + return; + } + + // BufferedStreamReader(Stream, timeouts...): find the ctor that takes a Stream first. + var readerCtor = bufferedReaderType.GetConstructors( + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance) + .FirstOrDefault(c => + { + var ps = c.GetParameters(); + return ps.Length >= 1 && typeof(Stream).IsAssignableFrom(ps[0].ParameterType); + }); + if (readerCtor == null) + { + Assert.Inconclusive("Ceen.Httpd.BufferedStreamReader has no Stream-first ctor via reflection."); + return; + } + + // Underlying stream: 8 bytes, but LimitedBodyStream is asked + // for 1 KB. So EOF hits after 8 bytes and DiscardAllAsync must + // return false (fix), not deadlock (pre-fix). + using var underlying = new MemoryStream(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }); + object bufferedReader; + try + { + var readerArgs = new object?[readerCtor.GetParameters().Length]; + readerArgs[0] = underlying; + for (var i = 1; i < readerArgs.Length; i++) + { + var pt = readerCtor.GetParameters()[i].ParameterType; + readerArgs[i] = pt.IsValueType ? Activator.CreateInstance(pt) : null; + } + bufferedReader = readerCtor.Invoke(readerArgs)!; + } + catch + { + Assert.Inconclusive("BufferedStreamReader could not be constructed via reflection."); + return; + } + + var bodyCtor = bodyStreamType.GetConstructor( + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, + binder: null, + types: new[] { bufferedReaderType, typeof(long), typeof(TimeSpan), typeof(Task), typeof(Task) }, + modifiers: null); + if (bodyCtor == null) + { + Assert.Inconclusive("LimitedBodyStream ctor signature has drifted; skipping this fixture."); + return; + } + var neverCompleting = new TaskCompletionSource().Task; + var body = (Stream)bodyCtor.Invoke(new object?[] + { + bufferedReader, (long)1024, TimeSpan.FromSeconds(5), neverCompleting, neverCompleting, + }); + + var discardMethod = bodyStreamType.GetMethod("DiscardAllAsync", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + Assert.That(discardMethod, Is.Not.Null, + "DiscardAllAsync method not found; refactor may have renamed it."); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + var task = (Task)discardMethod!.Invoke(body, new object?[] { cts.Token })!; + var completed = await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(10))); + + Assert.That(completed, Is.SameAs(task), + "DiscardAllAsync deadlocked past 10s on premature EOF — the CA2022 short-read fix regressed. " + + "Pre-fix, the loop spun forever on read==0 without decrementing m_bytesleft."); + var result = await task; + Assert.That(result, Is.False, + "DiscardAllAsync must return false on premature EOF so the caller can propagate the drain failure."); + } + + // ----------------------------------------------------------------- + // Helpers. + // ----------------------------------------------------------------- + + private static async Task Invoke(Stream? inputStream, CancellationToken cancellationToken = default) + { + var handlerType = typeof(MTConnect.Servers.Http.MTConnectHttpServer).Assembly + .GetType("MTConnect.Servers.MTConnectPostResponseHandler", throwOnError: false) + ?? throw new InvalidOperationException("MTConnectPostResponseHandler not visible via reflection."); + + // Prefer the post-fix (Stream, CancellationToken) signature so + // the cancellation-boundary tests exercise the real token path. + // Fall back to the pre-fix (Stream)-only shape so this fixture + // stays runnable on the parent commit while the RED tests + // deliberately fail against it — the fallback drives the RED + // outcome (token is ignored → OperationCanceledException never + // fires → the Throws assertion fails). + var tokenMethod = handlerType.GetMethod( + "ReadRequestBytes", + BindingFlags.NonPublic | BindingFlags.Static, + binder: null, + types: new[] { typeof(Stream), typeof(CancellationToken) }, + modifiers: null); + if (tokenMethod != null) + { + object? tokenTaskObj; + try + { + tokenTaskObj = tokenMethod.Invoke(null, new object?[] { inputStream, cancellationToken }); + } + catch (TargetInvocationException tie) when (tie.InnerException != null) + { + // Unwrap so callers can Assert.Throws on the real exception. + System.Runtime.ExceptionServices.ExceptionDispatchInfo + .Capture(tie.InnerException).Throw(); + throw; // unreachable + } + return await (Task)tokenTaskObj!; + } + + var legacyMethod = handlerType.GetMethod( + "ReadRequestBytes", + BindingFlags.NonPublic | BindingFlags.Static, + binder: null, + types: new[] { typeof(Stream) }, + modifiers: null) + ?? throw new InvalidOperationException("ReadRequestBytes not found via reflection."); + object? legacyTaskObj; + try + { + legacyTaskObj = legacyMethod.Invoke(null, new object?[] { inputStream }); + } + catch (TargetInvocationException tie) when (tie.InnerException != null) + { + System.Runtime.ExceptionServices.ExceptionDispatchInfo + .Capture(tie.InnerException).Throw(); + throw; // unreachable + } + return await (Task)legacyTaskObj!; + } + + /// + /// A Stream that returns its content in fixed-size chunks (or one + /// byte at a time by default) and then 0 on EOF. Mirrors real HTTP + /// request-body arrival patterns. When perReadDelay is + /// non-null the async read overload awaits that delay under the + /// supplied cancellation token, so mid-drip cancellation cancels + /// the pending Task.Delay and surfaces OperationCanceledException. + /// + private sealed class ScriptedStream : Stream + { + private readonly byte[] _content; + private readonly int _chunkSize; + private readonly TimeSpan? _perReadDelay; + private int _position; + + public ScriptedStream(byte[] content, int chunkSize = 1, TimeSpan? perReadDelay = null) + { + _content = content; + _chunkSize = Math.Max(1, chunkSize); + _perReadDelay = perReadDelay; + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => _content.Length; + public override long Position + { + get => _position; + set => throw new NotSupportedException(); + } + + public override void Flush() { } + + public override int Read(byte[] buffer, int offset, int count) + { + if (_position >= _content.Length || count == 0) return 0; + var take = Math.Min(count, Math.Min(_chunkSize, _content.Length - _position)); + Array.Copy(_content, _position, buffer, offset, take); + _position += take; + return take; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + if (_perReadDelay.HasValue) + await Task.Delay(_perReadDelay.Value, cancellationToken).ConfigureAwait(false); + return Read(buffer, offset, count); + } + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + + /// Stream whose ReadAsync always throws; models the transport-error / mid-drip cancellation path. + private sealed class ThrowingStream : Stream + { + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => 0; + public override long Position { get => 0; set => throw new NotSupportedException(); } + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) => throw new IOException("simulated transport error"); + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + => Task.FromException(new IOException("simulated transport error")); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + } +} diff --git a/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs b/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs new file mode 100644 index 000000000..8c4bba084 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs @@ -0,0 +1,180 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Http +{ + // Pins the CA2022 short-read fixes in + // - libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs + // (DiscardAllAsync — looping ReadAsync into a fixed buffer) + // - libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs + // (ReadRequestBytes — accumulating ReadAsync into a 2 MB buffer) + // + // CA2022 fires when `Stream.ReadAsync(buffer, offset, count)` is called + // without inspecting its return value. The stream contract allows + // short reads (returning fewer bytes than requested) — typical for + // HTTP request bodies arriving in multiple TCP segments. Pre-fix: + // * LimitedBodyStream.DiscardAllAsync looped on `m_bytesleft > 0` + // and would deadlock on a premature EOF + // * ReadRequestBytes called ReadAsync once and trusted the buffer + // was filled, then trimmed trailing 0x00 bytes — a body whose + // final byte legitimately was 0x00 was over-trimmed. + // + // Each test uses a custom Stream that returns its content one byte + // at a time (the worst-case short read), proving that the fix + // correctly accumulates the full payload. + /// Pins the CA2022 short-read accumulation fix on the HTTP request-body read path against a worst-case one-byte-per-call stream. + [TestFixture] + public class CA2022ShortReadTests + { + /// Pins that `MTConnectPostResponseHandler.ReadRequestBytes` accumulates the full body across short ReadAsync returns and preserves a legitimate trailing `0x00` byte (pre-fix `TrimEnd` over-truncated bodies whose final byte was zero). + /// An awaitable Task; the assertions inside drive the test outcome. + [Test] + public async Task ReadRequestBytes_accumulates_across_short_reads_and_preserves_trailing_zero() + { + // Body ends with a 0x00 byte. Pre-fix TrimEnd-on-zero would + // drop it. Post-fix the actual ReadAsync count drives the + // truncation; the 0x00 is preserved. + var body = new byte[] { 0x4D, 0x54, 0x43, 0x00 }; + using var oneByteAtATime = new OneByteAtATimeStream(body); + + var handlerType = LoadHandlerType(); + // Prefer the current (Stream, CancellationToken) signature — + // the F-IMP-001 fix threads the token — and fall back to the + // pre-fix (Stream) shape for source-tree resilience. Both + // shapes are exercised on the happy-path body below. + var tokenMethod = handlerType.GetMethod( + "ReadRequestBytes", + BindingFlags.NonPublic | BindingFlags.Static, + binder: null, + types: new[] { typeof(Stream), typeof(CancellationToken) }, + modifiers: null); + object taskObj; + if (tokenMethod != null) + { + taskObj = tokenMethod.Invoke(null, new object?[] { oneByteAtATime, CancellationToken.None }) + ?? throw new InvalidOperationException("ReadRequestBytes returned null Task."); + } + else + { + var legacyMethod = handlerType.GetMethod( + "ReadRequestBytes", + BindingFlags.NonPublic | BindingFlags.Static, + binder: null, + types: new[] { typeof(Stream) }, + modifiers: null) + ?? throw new InvalidOperationException( + "MTConnectPostResponseHandler.ReadRequestBytes not found via reflection."); + taskObj = legacyMethod.Invoke(null, new object?[] { oneByteAtATime }) + ?? throw new InvalidOperationException("ReadRequestBytes returned null Task."); + } + var task = (Task)taskObj; + var result = await task; + + Assert.That(result, Is.Not.Null); + Assert.That(result, Is.EqualTo(body)); + } + + /// Pins the contract behind the `LimitedBodyStream.DiscardAllAsync` short-read loop: a custom Stream returning one byte at a time then EOF must terminate the drain within a bounded number of iterations rather than deadlocking on the pre-fix `m_bytesleft > 0` guard. + /// An awaitable Task; the assertions inside drive the test outcome. + [Test] + public async Task DiscardAllAsync_terminates_on_premature_eof_short_read() + { + // LimitedBodyStream is internal to the Ceen.Httpd namespace + // and constructed via the request pipeline; testing it + // directly requires reflection on the constructor. Rather + // than fight that surface, we exercise the underlying contract + // — a custom Stream that returns 0 on EOF — and assert the + // shape of the fix via a wrapper that mirrors the production + // loop. This guards the regression class without coupling the + // test to the internal constructor's evolving parameter list. + using var truncated = new OneByteAtATimeStream(new byte[] { 0x01, 0x02, 0x03 }); + var buf = new byte[8]; + var iterations = 0; + var totalRead = 0; + while (iterations < 100) + { + iterations++; + var read = await truncated.ReadAsync(buf, 0, buf.Length, CancellationToken.None); + totalRead += read; + if (read == 0) + break; + } + + Assert.That(iterations, Is.LessThan(100), + "Drain loop ran 100 iterations without seeing EOF — short-read handling regressed."); + Assert.That(totalRead, Is.EqualTo(3)); + } + + private static Type LoadHandlerType() + { + // The handler is `internal sealed class + // MTConnect.Servers.MTConnectPostResponseHandler` inside + // MTConnect.NET-HTTP.dll. Force the assembly to load by + // anchoring on a public type from the same assembly + // (MTConnectHttpResponse), then GetType with the + // private-class name. + var anchor = typeof(MTConnect.Servers.Http.MTConnectHttpServer); + var asm = anchor.Assembly; + var t = asm.GetType("MTConnect.Servers.MTConnectPostResponseHandler", throwOnError: false); + if (t != null) + return t; + throw new InvalidOperationException( + "MTConnect.Servers.MTConnectPostResponseHandler not found in " + + asm.FullName + "; the class may have been renamed."); + } + + /// + /// A Stream that returns exactly one byte per ReadAsync call, then + /// 0 on EOF. Mirrors the worst-case short-read shape against which + /// CA2022 protects. + /// + private sealed class OneByteAtATimeStream : Stream + { + private readonly byte[] _content; + private int _position; + + public OneByteAtATimeStream(byte[] content) + { + _content = content; + _position = 0; + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => _content.Length; + public override long Position + { + get => _position; + set => throw new NotSupportedException(); + } + + public override void Flush() { } + + public override int Read(byte[] buffer, int offset, int count) + { + if (_position >= _content.Length || count == 0) + return 0; + buffer[offset] = _content[_position]; + _position++; + return 1; + } + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + return Task.FromResult(Read(buffer, offset, count)); + } + + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + } +} diff --git a/tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csproj b/tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csproj index e06ed7bd0..05dc9efdd 100644 --- a/tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csproj +++ b/tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csproj @@ -17,6 +17,22 @@ + + + + diff --git a/tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs b/tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs new file mode 100644 index 000000000..6b0be0174 --- /dev/null +++ b/tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs @@ -0,0 +1,184 @@ +// Copyright (c) 2026 TrakHound Inc., All Rights Reserved. +// TrakHound Inc. licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using MTConnect.Tls; +using NUnit.Framework; + +namespace MTConnect.NET_Common_Tests.Tls +{ + // Pins the SYSLIB0057 → X509CertificateLoader migration in + // libraries/MTConnect.NET-TLS/TlsConfiguration.cs. + // + // Each net9.0 call site (5 in total — three in GetPfxCertificate / + // GetPemCertificate, two in GetPemCertificateAuthority) replaced an + // obsolete X509Certificate2 byte/path constructor with an + // X509CertificateLoader.LoadPkcs12* / LoadCertificateFromFile call. + // The .NET 9 release notes state the loaders are behaviour-equivalent + // to the obsolete constructors for the password+path / bytes+password + // shapes — these tests round-trip a freshly-generated self-signed + // certificate through TlsConfiguration and assert the loaded + // certificate's thumbprint matches the original byte-for-byte. + // + // On net8.0 (the only TFM the test project targets) the production + // code still uses the legacy ctors — but the assembly under test is + // the multi-TFM MTConnect.NET-TLS.dll. The test guarantees the + // legacy path remains functional after the conditional refactor; + // the .NET 9 path is exercised by the Release-pack CI gate, which + // builds the same source with the X509CertificateLoader path active. + /// Pins the SYSLIB0057 migration on `TlsConfiguration.GetCertificate` / `GetCertificateAuthority`: every supported cert source (PFX with / without password, PEM cert + key, PEM CA-only) round-trips through the new `X509CertificateLoader` path without losing subject or thumbprint. + [TestFixture] + public class TlsCertificateLoaderTests + { + private string? _tempDir; + + /// Allocates a fresh per-test temp directory under the system temp root so each test owns its own PFX / PEM files and cannot race siblings. + [SetUp] + public void SetUp() + { + _tempDir = Path.Combine(Path.GetTempPath(), "mtc-tls-tests-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_tempDir); + } + + /// Tears down the per-test temp directory; failures are swallowed because cert files may briefly hold OS file locks on Windows even after the cert handle is disposed. + [TearDown] + public void TearDown() + { + if (_tempDir != null && Directory.Exists(_tempDir)) + { + try { Directory.Delete(_tempDir, recursive: true); } catch { /* best-effort */ } + } + } + + /// Pins that a password-less PFX loaded via `TlsConfiguration.GetCertificate` round-trips the original certificate's thumbprint and subject — the legacy `new X509Certificate2(byte[])` ctor that SYSLIB0057 obsoleted produced the same result; the new `X509CertificateLoader.LoadCertificate(...)` path must too. + [Test] + public void GetCertificate_pfx_without_password_round_trips_thumbprint() + { + using var original = CreateSelfSignedCertificate("CN=mtc-tls-pfx-nopw"); + var pfxBytes = original.Export(X509ContentType.Pkcs12); + var pfxPath = Path.Combine(_tempDir!, "no-pw.pfx"); + File.WriteAllBytes(pfxPath, pfxBytes); + + var config = new TlsConfiguration + { + Pfx = new PfxCertificateConfiguration { CertificatePath = pfxPath } + }; + + var result = config.GetCertificate(); + + Assert.That(result.Success, Is.True, () => "GetCertificate failed: " + result.Exception); + Assert.That(result.Certificate!.Thumbprint, Is.EqualTo(original.Thumbprint)); + Assert.That(result.Certificate.Subject, Is.EqualTo(original.Subject)); + } + + /// Pins that a password-protected PFX loaded via `TlsConfiguration.GetCertificate` correctly decrypts and round-trips the original thumbprint and subject — the new `X509CertificateLoader.LoadPkcs12FromFile(path, password, ...)` path must preserve the legacy obsolete-ctor's password semantics. + [Test] + public void GetCertificate_pfx_with_password_round_trips_thumbprint() + { + using var original = CreateSelfSignedCertificate("CN=mtc-tls-pfx-pw"); + const string password = "test-pw-9f3a"; + var pfxBytes = original.Export(X509ContentType.Pkcs12, password); + var pfxPath = Path.Combine(_tempDir!, "pw.pfx"); + File.WriteAllBytes(pfxPath, pfxBytes); + + var config = new TlsConfiguration + { + Pfx = new PfxCertificateConfiguration + { + CertificatePath = pfxPath, + CertificatePassword = password, + } + }; + + var result = config.GetCertificate(); + + Assert.That(result.Success, Is.True, () => "GetCertificate failed: " + result.Exception); + Assert.That(result.Certificate!.Thumbprint, Is.EqualTo(original.Thumbprint)); + Assert.That(result.Certificate.Subject, Is.EqualTo(original.Subject)); + } + + /// Pins that a PEM-encoded certificate + matching private-key file pair loaded via `TlsConfiguration.GetCertificate` round-trips the original thumbprint and subject through the PEM → in-memory PKCS#12 → loader path that the SYSLIB0057 migration introduced. + [Test] + public void GetCertificate_pem_with_private_key_round_trips_subject() + { + using var original = CreateSelfSignedCertificate("CN=mtc-tls-pem"); + var pemCertPath = Path.Combine(_tempDir!, "cert.pem"); + var pemKeyPath = Path.Combine(_tempDir!, "key.pem"); + + File.WriteAllText(pemCertPath, ExportCertificateToPem(original)); + File.WriteAllText(pemKeyPath, ExportPrivateKeyToPem(original)); + + var config = new TlsConfiguration + { + Pem = new PemCertificateConfiguration + { + CertificatePath = pemCertPath, + PrivateKeyPath = pemKeyPath, + } + }; + + var result = config.GetCertificate(); + + Assert.That(result.Success, Is.True, () => "GetCertificate failed: " + result.Exception); + // The PEM path re-exports through PKCS#12 and re-imports; the + // re-imported certificate must retain the original subject and + // thumbprint (the cert bytes are unchanged by the round-trip). + Assert.That(result.Certificate!.Subject, Is.EqualTo(original.Subject)); + Assert.That(result.Certificate.Thumbprint, Is.EqualTo(original.Thumbprint)); + } + + /// Pins that a PEM-encoded CA certificate (cert-only, no private key) loaded via `TlsConfiguration.GetCertificateAuthority` round-trips the original thumbprint and subject — the CA path uses `X509CertificateLoader.LoadCertificateFromFile(...)` which differs from the cert+key path used by `GetCertificate`. + [Test] + public void GetCertificateAuthority_pem_round_trips_subject() + { + using var original = CreateSelfSignedCertificate("CN=mtc-tls-ca"); + var caPath = Path.Combine(_tempDir!, "ca.pem"); + File.WriteAllText(caPath, ExportCertificateToPem(original)); + + var config = new TlsConfiguration + { + Pem = new PemCertificateConfiguration { CertificateAuthority = caPath } + }; + + var result = config.GetCertificateAuthority(); + + Assert.That(result.Success, Is.True, () => "GetCertificateAuthority failed: " + result.Exception); + Assert.That(result.Certificate!.Subject, Is.EqualTo(original.Subject)); + Assert.That(result.Certificate.Thumbprint, Is.EqualTo(original.Thumbprint)); + } + + private static X509Certificate2 CreateSelfSignedCertificate(string subject) + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest( + subject, + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + return request.CreateSelfSigned( + DateTimeOffset.UtcNow.AddDays(-1), + DateTimeOffset.UtcNow.AddDays(30)); + } + + private static string ExportCertificateToPem(X509Certificate2 certificate) + { + var der = certificate.Export(X509ContentType.Cert); + return "-----BEGIN CERTIFICATE-----\n" + + Convert.ToBase64String(der, Base64FormattingOptions.InsertLineBreaks) + + "\n-----END CERTIFICATE-----\n"; + } + + private static string ExportPrivateKeyToPem(X509Certificate2 certificate) + { + using var rsa = certificate.GetRSAPrivateKey() + ?? throw new InvalidOperationException("certificate has no RSA private key"); + var pkcs8 = rsa.ExportPkcs8PrivateKey(); + return "-----BEGIN PRIVATE KEY-----\n" + + Convert.ToBase64String(pkcs8, Base64FormattingOptions.InsertLineBreaks) + + "\n-----END PRIVATE KEY-----\n"; + } + } +}