Provider reconciliation: FPC-trunk compat, IOCP/Epoll/HTTP.sys fixes, streaming robustness

[freitasjca]

Provider reconciliation: FPC-trunk compat, IOCP/Epoll/HTTP.sys bug fixes, and streaming robustness

From: freitasjca/horse:master → Into: HashLoad/horse:master

---

Summary

While re-syncing a downstream, CrossSocket-compatible fork onto the recent
streaming + WebSocket merge (Res.SendStream / IHorseStreamWriter), we ran the
full provider test matrix against every shipped transport and root-caused a set
of real, reproducible bugs in the providers themselves. This PR brings those fixes
back upstream so the main repo benefits directly.

Every change here is additive or a localized bug fix — no public API is renamed,
removed, or re-signed. Without any of the relevant compiler defines, Horse compiles
and behaves exactly as before. The fixes span four areas:

• FPC (Lazarus / fphttpserver) — trunk (3.3.1) compilation + HTTP/1.1 keep-alive +
  correct string-body length + streaming writer selection.
• IOCP — request-body handling, chunked request bodies, cookie parsing,
  Content-Type, RemoteAddr, duplicate Set-Cookie, concurrent-stream integrity.
• Epoll — blocking Listen contract, duplicate Set-Cookie, streamed
  Content-Type.
• HTTP.sys — response wiring, query/header parsing, duplicate Set-Cookie,
  streaming completion (keep-alive after a stream).

All fixes were validated on two independent HTTP clients (see Validation
methodology below) so that server behaviour is confirmed, not a client artifact.

The fixes are logically independent and can be split into per-provider PRs if the
maintainers prefer smaller reviews — see Notes for reviewers at the end.

---

Validation methodology — why two test clients

The evidence below comes from samples/tests/, exercised by two deliberately
different HTTP clients hitting the same server on port 9010. Using two independent
stacks is the core of the methodology:

1. HorseNetHttpTestClient.dpr — System.Net.HttpClient (WinHTTP)

• Zero dependency on Delphi-Cross-Socket — compiles against the plain RTL, so a
  client-side bug cannot silently distort server results.
• Buffers request bodies and sends an explicit Content-Length; de-chunks
  responses transparently; auto-adds Content-Length: 0 on empty PUT/POST.
• It is the reference consumer for the HTTP.sys provider (same Microsoft HTTP
  stack on both ends).

2. HorseIndyTestClient.dpr — TCrossHttpClient (pooling, keep-alive)

• Created with TCrossHttpClient.Create(2 {IoThreads}) — a connection pool of only
  2 keep-alive sockets that are aggressively reused and multiplexed.
• Streams multipart uploads with Transfer-Encoding: chunked (no
  Content-Length).
• Fires concurrent requests before awaiting any of them.

Why both are necessary

A single client hides transport bugs. The two stacks stress different server code
paths, and several bugs in this PR are visible to only one of them:

Bug class	WinHTTP	TCrossHttpClient (pooling)
Chunked request body (multipart upload)	buffered → Content-Length (masks it)	exposes it (no CL)
Keep-alive connection reuse desync	rarely reuses	exposes it (2-conn pool)
Stale-dispatch on partial read	complete requests only	stresses it
Concurrent-stream integrity (test 36)	serves fine	exposes partial-send truncation
HTTP.sys response semantics	reference consumer	second opinion

Passing both clients demonstrates the fixes are genuine server-side behaviour.
(This is not hypothetical: a prior TCrossHttpClient-only hang on a 200 +
Content-Length: 0 response — a client-library bug WinHTTP handled correctly — is
exactly why a WinHTTP reference client exists.)

---

Test evidence (samples/tests/, both clients)

Provider	HorseNetHttpTestClient (WinHTTP)	HorseIndyTestClient (TCrossHttpClient)
IOCP (Windows)	101 / 101	101 / 101
HTTP.sys (Windows)	101 / 101	98 / 98
Epoll (Linux)	97–101 / all pass	94 / 94
FPC default (fphttpserver)	94 / 94	94 / 94

(Counts differ per provider because some tests are transport-specific — e.g. the
FPC/Linux builds decline real streaming with a clean 501, which the suite accepts;
Windows engine transports stream for real.)

The suite (per provider) covers: all HTTP methods, path/query params, cookies
(including duplicate Set-Cookie), multipart upload, file download, 64 KB / 65 536-byte
bodies, request isolation under 4- and 8-way concurrency, explicit 400/500, large
responses, the RawWebRequest/RawWebResponse adapter surfaces, CORS preflight, and
push streaming (/stream/pull, /stream/content-type, /stream/empty, and 2×
concurrent streams) with a post-stream /ping health probe to catch keep-alive
wedges.

---

Fixes by provider

FPC (Lazarus / fphttpserver)

1. Horse.Provider.FPC.HTTPApplication.pas — HTTP/1.1 keep-alive never enabled (threaded mode).
TFPCustomHttpServer.KeepConnections defaults to False, and in threaded mode
TFPHTTPConnectionThread's keep-alive loop is gated on WaitUntil > 0, which is 0
whenever KeepConnectionTimeout <= 0 (the default) — so even KeepConnections := True
alone still closes after one response. Every HTTP/1.1 response was closing the TCP
connection; pooling clients saw a reconnect per request. Fix: after Initialize,
set both KeepConnections := True and KeepConnectionTimeout := 15000 on the
embedded server (reached via same-unit descendant casts, since neither
THTTPApplication nor TFPHTTPServerHandler forwards them).

2. Horse.WebModule.pas — write requests 500 on FPC trunk (CSRF/session).
FPC trunk's TCustomFPWebModule.HandleRequest now runs a CSRF/session check
(IsWriteRequest → TSessionHTTPModule.GetSession) before calling DoOnRequest,
and FSessionRequest is never assigned on this path → every POST/PUT/DELETE/PATCH
fails with "Default session not available outside handlerequest". Fix: override
HandleRequest directly and dispatch straight into the Horse pipeline, bypassing the
session/CSRF/action machinery Horse doesn't use (WebSocket integration in
HandlerAction is preserved).

3. Horse.Core.Param.Header.pas — FPC-trunk generics guard won't compile.
The CONST_GENERIC guard is {$IF DEFINED(CPU64) AND DEFINED(WINDOWS)} const {$ELSE} constref — so on FPC-trunk Linux it selects constref, but trunk's rtl-generics
IEqualityComparer<T> now uses const on every platform → "No matching
implementation for interface method Equals". Fix: add an
{$IF FPC_FULLVERSION >= 30301} const branch first (trunk = const everywhere),
keeping the CPU64/WINDOWS branch for older FPC.

4. Horse.Response.pas (FPC string-body path) — trailing newline on every string body.
fpWeb's TResponse.Content is backed by TStrings; the wire path emits
FContents.Text, which appends a LineEnding — 'pong' arrives as 'pong'\n, a
65 536-byte body as 65 537 bytes, and it cannot be trimmed post-hoc. Fix: on the
FPC fpWeb-native path, send via ContentStream := TStringStream.Create(LContent) +
explicit ContentLength instead of Content.

5. Horse.Provider.FPC.HTTPApplication.pas — SendStream hangs (wrong default writer).
Horse.Response.pas registers the Indy WebBroker stream writer as the global
default (FStreamWriterFactory, last-wins); real providers override it, but the FPC
HTTPApplication provider registered nothing, inheriting the Indy writer, which does
Indy WriteClient on a non-Indy connection and blocks. Fix: the FPC provider
registers its own factory (clean 501 refusal for the non-engine transport; a real
fpWeb chunked writer is a future option). Upstream may also want to guard the
WebBroker default to Indy-only.

IOCP (Windows)

6. 65 536-byte POST closed mid-upload + keep-alive cascade (one coupled defect).
(a) The DoS guard if (not Processing) and (LNewLen > 16384) then CloseConnection
ran on the accumulated body, not just headers (Processing isn't set until the
whole body arrives), so any multi-read body > 16 KB was closed mid-upload. (b) On an
incomplete body, ProcessClientRead called PostRead and Execute then called
PostRead again → two overlapped WSARecv on the same buffer; when the close
path freed the context, the second completion dereferenced freed memory → worker
crash / heap corruption → an all-timeout wedge that persisted across client runs.
Fix: scope the 16 KB cap to the header section (apply in the parse-failed branch)
and bound the body by an explicit IOCP_MAX_BODY_SIZE (16 MB); make
ProcessClientRead the sole owner of re-arming (removed Execute's duplicate
PostRead) — exactly one outstanding WSARecv per connection.

7. No Transfer-Encoding: chunked request-body support → keep-alive desync.
TryParseRequest recognised only Content-Length. TCrossHttpClient streams
multipart uploads chunked (no CL), so the server parsed length 0, dispatched an
empty body, never consumed the chunk bytes, then read them as the next request →
permanent keep-alive desync (every reused-connection request timed out). WinHTTP
buffers multipart and sends Content-Length, so it never hit this. Fix: detect
Transfer-Encoding: chunked; wait for the terminating 0-chunk, then de-chunk to a
contiguous known-length body before dispatch (mirrors the Content-Length wait).

8. Request Cookie header — final cookie loses its last character.
user=tester; session=abc123 → session=abc12. The segment splitter used
Copy(LCookies, LStart, I - LStart) for both the ;-delimited case and the
end-of-string case, where the length must be I - LStart + 1 to include the last
char. Fix: branch on the terminator (; → I - LStart, end → I - LStart + 1).

9. Res.ContentType dropped (everything defaults to text/plain).
The response builder populated headers only from ARes.CustomHeaders; it never read
ARes.CSContentType. Fix: inject CSContentType when Content-Type is absent —
applied on both the normal (SendResponse) and streaming (SendRawHeaders) header
paths.

10. RemoteAddr always empty.
IOCP resolves the GetAcceptExSockaddrs extension pointer but never calls it, so
the connection's ClientIP is never set. Fix: call it in the ioAccept
completion and set ClientIP from the remote sockaddr.

11. Duplicate Set-Cookie collapsed + RawWebResponse.SetCustomHeader dropped.
IOCP funnels all headers through a TDictionary (dedups → duplicate Set-Cookie
folds) and reads only the shadow header store (not the adapter store where
SetCustomHeader writes). Fix (additive, regression-safe): in Horse.Response.pas
add a lazily-allocated RepeatHeaders: TStringList that AddHeader also appends to
only for Set-Cookie (RFC 6265 §3 — the header that must not fold); the provider
emits each RepeatHeaders entry directly (bypassing the dedup dict) and reads the
union of shadow + adapter stores. The shared Horse.Response.pas change is purely
additive — other providers compile and behave identically.

12. Concurrent-stream corruption (2× /stream/pull).
The merged stream writer did a single send() and ignored the return (headers +
each chunk). Under concurrent load a partial send truncated a chunk, corrupting the
response and leaving the just-streamed keep-alive connection unusable on reuse (only
the 2-connection pooling client exposed it). Fix: a SendFully loop (send until
all bytes go, stop on error) on the header and body writes.

Epoll (Linux)

13. InternalListen doesn't block — console server exits at startup.
Merged InternalListen spawns worker threads, sets FRunning := True, fires
DoOnListen, and returns — unlike every other provider, which honours the
blocking Listen contract with if IsConsole then while FRunning do Sleep(100). The
console server printed its banner and immediately exited. Fix: re-add the blocking
wait loop after DoOnListen.

14. Duplicate Set-Cookie + adapter SetCustomHeader + streamed Content-Type.
Same classes as IOCP #11 and #9, applied to the Epoll header-emit path (read the
adapter store as a second emit; emit RepeatHeaders verbatim; pass CSContentType
into the streaming SendHeaders).

Also identified (not included here): a latent stale-dispatch on parse failure
— on a partial read over a reused keep-alive connection, the context retains the
previous request's parsed fields and re-dispatches it. It did not manifest on either
test client (both send complete, well-formed requests), so we deliberately did not
apply the 3-site restructure that would risk the passing suite. Flagged here as a
defensive-hardening candidate for a separate PR.

HTTP.sys (Windows)

15. Response wired to the Indy path but read from the CS shadow.
The response was created via THorseResponse.Create(LWebResponse) (Indy
FWebResponse path), but SendResponse reads the CS shadow (CSContentType /
CustomHeaders) — so Res.ContentType / AddHeader wrote to FWebResponse while
SendResponse read the empty shadow (Content-Type → text/html, test 23; OPTIONS
status, test 26). Fix: Create(nil) + SetCSRawWebResponse at both dispatch sites
(matching IOCP/Epoll) and remove the now double-free LWebResponse.Free (ownership
transfers to the response).

16. Query string parsed with the leading ?.
GetQueryString returned HTTP.sys's CookedUrl.pQueryString with the ?, so the
first key became ?name → Req.Query['name'] = ''. Fix: strip the leading ? at
the source.

17. Request header separator mismatch.
GetHeadersList sets NameValueSeparator := ':' then calls PopulateHeaders, but the
code hard-coded Name=Value, so Req.Headers['X-…'] (parsed with :) never matched
→ empty. Fix: honour ADest.NameValueSeparator.

18. Duplicate Set-Cookie (REPEATHDR treatment).
Same class as IOCP #11, applied to the UnknownHeaders array build (running index;
skip Set-Cookie from shadow + adapter; emit each RepeatHeaders entry as its own
UnknownHeader).

19. Streaming double-send → HttpSendHttpResponse error 22.
After SendStream responds, the dispatch called SendResponse unconditionally →
a second HttpSendHttpResponse on the same RequestId → ERROR_INVALID_PARAMETER.
Fix: a FResponseSent flag (set by the stream writer, checked by SendResponse).

20. Streamed response never closes → keep-alive wedge + concurrent-stream failure.
The base Close sends the 0-terminator via WriteRawBytes, which always sets
MORE_DATA, so HTTP.sys never completes the response — WinHTTP tolerates it (completes
off the chunked terminator, doesn't reuse), but keep-alive clients wedge on the next
request. Fix: override the writer's Close to send the terminator as the final
entity body without MORE_DATA, completing the response so keep-alive returns clean
(this also fixed the 2× concurrent-stream test).

---

Compatibility

• No behavioural change without the relevant path. The shared Horse.Response.pas
  additions (RepeatHeaders, FPC stream-body) are additive; providers not using them
  compile and behave identically.
• Delphi + Lazarus/FPC, Windows / Linux / macOS. FPC items (1–5) reproduce on FPC
  trunk 3.3.1 — please build with that version to verify.
• No .dproj / .dpk / .lpi changes required by these edits.

---

Notes for reviewers

• The fixes are logically independent; happy to split into per-provider PRs
  (FPC / IOCP / Epoll / HTTP.sys) or even per-bug if that eases review.
• The Horse.Response.pas RepeatHeaders change is the only shared-code touch — it is
  additive and guarded, but it is worth reviewing across all providers at once (it is
  consumed by IOCP/Epoll/HTTP.sys and ignored by the rest).
• The test programs (samples/tests/) that produced the evidence above can be included
  or kept downstream, as preferred — they depend only on the RTL (HorseNetHttpTestClient)
  and on Delphi-Cross-Socket (HorseIndyTestClient).
• The latent Epoll stale-dispatch hardening is intentionally excluded from this PR
  and offered separately, to keep this change set fully validated.