Files
ersatztv/ErsatzTV.Mcp/BoundedLineReader.cs
T
timothy 4f68805d9a security(#289): review fixes — body-read timeout, transport-error response, bounded stdin, env clamps
Folds in findings from the independent reviews of the initial hardening diff
(a cold adversarial pass + a Codex pass — complementary catches).

Fork (HIGH/MED):
- Body-read timeout regression: ResponseHeadersRead moved the body read outside
  HttpClient.Timeout and it used CancellationToken.None, so a slow-drip upstream
  hung the single-threaded session. Now a per-request linked CTS (options
  .EffectiveRequestTimeout) covers headers + body; HttpClient.Timeout set to
  Infinite so one timer owns it. Verified live against a black-hole upstream:
  1s timeout → -32603 in ~1s, no hang.
- Transport/timeout exceptions escaped HandleAsync's catch filter → no response
  for the id → client hangs. Added a broad final catch → -32603 when hasId.

Codex (env/input robustness):
- ERSATZTV_REQUEST_TIMEOUT_SECONDS / ERSATZTV_MAX_RESPONSE_BYTES at int.MaxValue
  crashed at startup / overflowed `cap+1` to a negative alloc. ParseInt now
  clamps to [min,max]; the client also hard-ceils the cap (MaxAllowedResponseBytes).
- A ".."/"." path param collapsed the URL onto another route under Uri
  canonicalization — rejected in BuildPath.

Both reviewers (stdin OOM): ReadLineAsync buffered an unbounded line before any
guard. New BoundedLineReader drains+drops oversized lines (memory-bounded),
keeping subsequent lines aligned.

Nits: case-insensitive ParseBool fallback; UTF-8 boundary backoff so a mid-
codepoint truncation doesn't emit U+FFFD; renamed a misleading test.

34 tests (was 26); +8 covering the transport-error path, dot-segment rejection,
cap-overflow fallback, UTF-8 seam, and the bounded reader.

Refs #289 #197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 01:20:10 +02:00

57 lines
1.8 KiB
C#

namespace ErsatzTV.Mcp;
/// <summary>
/// Reads newline-delimited lines from a <see cref="TextReader"/> with a hard character cap, so a
/// hostile client cannot exhaust memory by sending an enormous line with no newline. A line longer
/// than the cap is drained (not buffered) and reported as overflowed rather than returned.
/// </summary>
public static class BoundedLineReader
{
public readonly record struct Line(bool EndOfStream, bool Overflowed, string Text);
public const int DefaultMaxChars = 1024 * 1024;
public static async Task<Line> ReadLineAsync(TextReader reader, int maxChars = DefaultMaxChars)
{
int cap = maxChars > 0 ? maxChars : DefaultMaxChars;
var builder = new System.Text.StringBuilder();
var buffer = new char[1];
bool sawAny = false;
bool overflowed = false;
while (await reader.ReadAsync(buffer, 0, 1) == 1)
{
sawAny = true;
char c = buffer[0];
if (c == '\n')
{
return new Line(false, overflowed, overflowed ? string.Empty : builder.ToString());
}
if (c == '\r')
{
continue;
}
if (overflowed || builder.Length >= cap)
{
// Past the cap: stop buffering and free what we have, but keep draining to the
// newline so the next line stays aligned.
overflowed = true;
builder.Clear();
continue;
}
builder.Append(c);
}
if (!sawAny)
{
return new Line(true, false, string.Empty);
}
// Final line with no trailing newline.
return new Line(false, overflowed, overflowed ? string.Empty : builder.ToString());
}
}