Repository navigation
Integration-test Stream doubles do not honour the close-on-failure half of the contract #727
Description
Problem
SolidSyslogStreamDefinition.h requires a Stream implementation to close internally on a failed Send and before returning a negative Read, so the caller can reopen and store-and-forward can replay.
The two integration-test transports — Tests/MbedTlsIntegration/SocketStream.c and Tests/OpenSslIntegration/BioPairStream.c — do not do this. They are the transports the TLS integration tests run over, so the tests exercise the adapter against a peer that does not behave as the contract says a Stream behaves.
Surfaced by a CodeRabbit comment on PR #711.
Why it is worth doing
Not a product defect: these are test doubles and nothing ships them. It matters because they sit underneath contract tests. A double that is more forgiving than the contract can hide a reconnect path that would misbehave against a conforming one, which is the failure mode these tests exist to catch.
Options
- Make both doubles conform — close on failed
Sendand before a negativeRead. Preferred; it is a few lines each and makes the test peer honest. - Leave them and state in each file that the double is deliberately permissive and which clauses it does not implement, so the next reader does not mistake it for a reference.
Option 1 unless it turns out the tests depend on the permissive behaviour, in which case option 2 with the reason recorded.
Acceptance criteria
- Both doubles either honour the close-on-failure clauses or carry a comment naming the clauses they deliberately do not implement, and why.
- The OpenSSL and mbedTLS integration lanes stay green.
- No production code changes.
Activity
DavidCozens commentedon Aug 17, 2026
Milestoned to 0.2.0, and option 2 is the expected outcome rather than the fallback
Reviewed against the tree during the pre-0.1.0 issue audit. The finding holds:
SolidSyslogStreamDefinition.hrequires a Stream to close internally on a failedSendand before returning a negativeRead, and neither double does.BioPairStream::Sendreturnswritten == sizeand stops,SocketStream_Sendreturns false mid-loop,
SocketStream_Readmaps EOF to -1, and none of the fourcalls
Close.What this issue did not know is that both doubles are deliberately built so the
harness owns the connection lifecycle, and both say so in the file.
Tests/MbedTlsIntegration/SocketStream.copens with:SocketStream_Closecloses that fd. MakingSendcall it on failure handslifecycle back to the double, against the design the comment states.
BioPairStreamis the stronger case: closing on a failedSendwould tear downthe BIO pair while the paired peer is still attached to it.
So option 1 is not "a few lines each" as this issue estimated. The escape hatch it
wrote for itself — option 1 unless the tests depend on the permissive behaviour
— is the live branch, and the evidence is in the files rather than a suspicion.
Expect option 2. Roughly ten lines across the two files, naming the clauses
each double deliberately does not implement and why, so the next reader does not
mistake a permissive double for a reference implementation. That is where the
value is: the risk was never the doubles misbehaving, it was someone reading one
as a conforming Stream.
Confirm by running the two integration lanes before writing the comments, not
after — if conforming turns out to be harmless for one of them, do it for that one
and document the other.