feat: Res.Send<TStream>(S) now writes stream bytes to the response body - #540
Open
freitasjca wants to merge 1 commit into
Open
feat: Res.Send<TStream>(S) now writes stream bytes to the response body#540freitasjca wants to merge 1 commit into
freitasjca wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Res.Send<TStream>(SomeStream).Status(200)currently producesHTTP 200 Content-Length: 0on every Horse provider (Indy, HTTP.sys, IOCP, Epoll). This is becauseTHorseResponse.Send<T>only assignsFContent := AContent— aTObjectslot intended for content-type middleware (horse-jhonson, etc.) — and no provider bridge ever readsFContentas a stream.This PR adds a
DoSendStreamhelper and a runtimeis TStreambranch inSend<T>so that stream bodies are copied into the existingFCSContentStream/FWebResponse.ContentStreamslots that every provider already reads. A non-generic typedSend(TStream)overload is also added soRes.Send(S)works without an explicit generic type argument.Third-party providers (CrossSocket, mORMot, nghttp2) benefit automatically because the fix operates on the same shared shadow-field slots — no provider-side changes needed anywhere in the ecosystem.
The user-visible change
Before:
After:
Design
Three additions to
src/Horse.Response.pas:Private
DoSendStream(AStream: TStream)— copies the source stream into the active body slot:FCSContentStream(HTTP.sys, IOCP, Epoll, and third-party providers via the shadow-field path)FWebResponse.ContentStream(Indy / WebBroker — the traditional TWebResponse path)Follows the exact copy-and-own pattern already used by the existing
SendFile(TStream, ...)method. DefaultsContent-Typetoapplication/octet-streamif the caller hasn't set one. ResetsPosition := 0before copying so the common "position at end after writing" gotcha doesn't produce empty bodies.Non-generic overload
Send(const AContent: TStream): THorseResponse— declared alongside the existingSend(string)andSend(TBytes)overloads. Compiler picks it when the argument's static type isTStream(or a known subclass), soRes.Send(MyStream)works without<TStream>.Modified
Send<T>body — runtimeTObject(AContent) is TStreambranch routes toDoSendStream; the else-pathFContent := AContentis unchanged, so all existing non-streamSend<T>callers (JSON serializers viahorse-jhonson, etc.) keep working identically.Ownership contract
Horse takes ownership of the stream —
DoSendStreamcopies the bytes into an internalTMemoryStreamand then callsAStream.Free. Matches the historicalSend<T>contract whereClear/DestroyfreedFContent. Callers must NOT wrap intry/finally S.Free(double-free).SendFile(AStream, name, type)is unchanged — it still does NOT take ownership. ExistingSendFilecode continues to work identically.Backward compatibility
Send<T>callers hit the unchangedelse FContent := AContentbranch — JSON middleware and any otherTObject-slot users see identical behavior.Send<TStream>(S1).Send<TStream>(S2)on the same response replaces the first body with the second (via the existingFCSOwnsContentStreamfree-first guard on the shadow path; viaFWebResponse.ContentStream :=overwrite on the Indy path).Res.Send(nil)is a no-op via nil-guard inDoSendStream(matchesSend('')behavior).ContentStream.Behavioral change to be aware of
Any code that currently calls
Res.Send<TStream>(S)and expected an empty body (either as a workaround or because they tested against the broken behavior) will now receive the stream contents. This is the intended fix, but downstream test mocks that relied on receiving empty responses may need updating.Verification
A comprehensive reproducer + regression suite is available at
patches/horse-provider-crosssocket/samples/stream-demo/in freitasjca/horse-crosssocket-workspace. Runs on the CrossSocket provider by default; the same handler code works on any Horse provider once this PR is merged.Test coverage in the demo:
Send<TStream>(S)withTMemoryStream, position-at-end, position-reset, 30 bytes, 1024-byte pattern, 64 KB payload,TFDMemTableinsfBinary/sfXML/sfJSONSendFile(unchanged) that produce byte-identical output to the newSend<TStream>path — validates the fix produces the same wire bytes as the pre-existing working APIResult: 47/47 assertions PASS on CrossSocket Win32 (Delphi 12.1).
Files changed
src/Horse.Response.pas— one file, 90 lines added, 1 line modified (only the genericSend<T>body changes)Base version
Branched off
masterat Horse 3.3.2 (commit72cc45f). Applies cleanly on top of the current release.