Object compression path: three defects in the zlib loops - #89
Closed
jordijoangimenez wants to merge 7 commits into
Closed
Object compression path: three defects in the zlib loops#89jordijoangimenez wants to merge 7 commits into
jordijoangimenez wants to merge 7 commits into
Conversation
The Receiver-side counterpart to Transmitter's existing udp_tunnel_address() support. Per the issue discussion, the library has no business knowing about any particular encapsulation format -- that is entirely the controlling application's concern: "Stripping the GTP-U header doesn't feel like something that a generic FLUTE library should be asked to do... A better design pattern would be for the controlling application to pass in a 'helper' function that the library invokes to do application-specific mangling of packets before the generic code in the library starts processing the ALC/LCT payload." -- rjb1000 Adds three new optional constructor parameters: - tunnel_address: if given, ALSO bind a plain unicast UDP socket to this local endpoint and accept tunnelled datagrams there, in addition to the normal multicast join. The two paths are independent and both feed the same session state -- deliberately not an either/or choice like Transmitter's tunnel mode, since a Receiver has no way to know in advance which path will actually work in a given deployment. - tunnel_source: if given, only accept tunnel datagrams from this source address -- the tunnel-socket equivalent of the existing source_address parameter's SSM admit-only-this-source semantics. This is the "extra address checking" the library itself does, on top of whatever packet_modifier does; source-address admission is a generic, encapsulation-agnostic concept the library can reasonably own, unlike parsing any particular header format. - packet_modifier: required whenever tunnel_address is set. Given the whole received datagram (as a mutable vector, so a modifier can also decrypt/rewrite in place, not just locate the payload), returns the byte offset at which the ALC/LCT payload begins -- an offset >= the buffer's size means "discard, nothing usable here". No default implementation is provided, since any default would itself bake an encapsulation assumption into the library. This mirrors the de-tunnelling logic that already exists, hand-written, in tests/test_end_to_end.cpp's run_tunnel_bridge() (added alongside Transmitter's own tunnel mode in #56): a std::thread there receives on a plain UDP socket, manually parses a hand-built inner IPv4+UDP header out of the payload, and forwards just the FLUTE bytes onward over loopback to a receiver with no tunnel-awareness at all. This moves that capability inside Receiver proper as a caller-supplied, protocol-agnostic hook, so any deployment's actual encapsulation (Transmitter's own wrapper, real GTP-U, or anything else) is expressed purely by what modifier is passed in -- enabling rt-mbs-client (referenced in the issue as "MBSTF Client") to take advantage of it for reception paths where local multicast delivery isn't available at all. Confirmed live end-to-end: real Service Announcement content broadcast over an actual gNB/UE radio link, captured on the UE's TUN device via a tunnel_address + packet_modifier pairing, correctly parsed into a complete FDT and announcement bundle.
handle_receive_from() and handle_tunnel_receive_from() only called arm_receive()/arm_tunnel_receive() again in the success branch -- a single transient socket error (e.g. an ICMP port-unreachable surfacing as a UDP socket error on a subsequent read, hit live via the raw capture relay's loopback sendto() path) permanently killed reception for the rest of the process's life, with just one log line and no way to recover short of restarting the client. (Attempted as a cross-repo cherry-pick of jordijoangimenez/rt-libflute commit d453c05, 'Fix FDT/TOI reassembly corruption, multicast bind/join, ...' -- turned out this branch already independently has every other fix from that commit (FDT-instance-discard reassembly logic, catch(const char*), the INADDR_ANY bind, the per-interface multicast join, the array-specialised shared_ptr scratch buffers); the re-arm bug above was the only genuinely missing piece, found here by live strace/tcpdump debugging, not by that commit.)
…f hanging Problem calculate_partitioning() divides the transfer length by encoding_symbol_length, then the symbol count by max_source_block_length. Both come straight from the FEC OTI and neither was checked. A default-constructed FecOti leaves both 0, so the first division yields inf, the block count yields inf, and block creation becomes effectively unbounded: the File constructor hangs rather than reporting anything. Reachable by a caller, not only internally: the public File constructors accept a FecOti and never inspect it. Observed as a ctest timeout, not a crash, which is the harder failure to diagnose from a log. [code-derived, no spec claim] Basis No clause governs this; it is an unchecked divisor. Fixable under RULES.md rule 2 with no specification claim. On refusing rather than substituting a default: rule 12 prefers failing loudly, and any value invented here would be a bound resting on nothing. Raised by Writing a test for a different finding on this branch. The test constructed a File from a FileDescription without configuring its FEC OTI, and hung instead of failing, which is how this surfaced. Change Reject a FEC OTI whose encoding_symbol_length or max_source_block_length is zero, before either is used as a divisor, naming both fields in the message. Verification T1: builds clean, ctest at the build root 44/44, up from 40. Four cases added, using the direct File constructor, which is the public path that accepts an unchecked FecOti. The three refusal cases were each confirmed to TIME OUT with the guard removed and to throw with it restored, so they reproduce the hang rather than describe it. The fourth uses a usable FEC OTI and passes either way, holding the guard to refusing only what it should. Not in this change No default substituted for either field, and no validation of the other FEC OTI members. The Raptor partitioning path has its own divisors and is not touched here.
…hout an encoding Problem The File element parser fell back to Content-Length whenever Transfer-Length was absent, without regard to whether a content encoding had been applied. For an encoded object the two are different quantities, so the fallback handed the decompressor the decoded length as its input size, wrong by however much the encoding changed, and decoding failed on a well-formed session. Observed at src/FileDeliveryTable.cpp, File element parse. [code-derived] Basis RFC 3926 clause 3.4.2: "If the file is not content encoded before transport (and thus the "Content-Encoding" attribute is not used) then the transfer length is the length of the original file, and in this case the "Content-Length" is also the transfer length." The substitution is authorised for that case and no other. Raised by reading the parser while tracing why a gzip object failed to decode Change Content-Encoding is now parsed before the transfer length, and the fallback applies only when no encoding was declared. With an encoding applied and no Transfer-Length carried, the transfer length is left at 0, meaning not known from this FDT, rather than guessed. Verification T0: builds clean and the suite passes. The test that pins this rule in both directions arrives with the change that makes the unknown case decodable, since on this commit alone an encoded object has no other source for its length. Not in this change Supplying the length by another route. Withholding a wrong value stops the silent mis-decode; it does not by itself make an encoded object decodable under the MBMS Download Profile, which needs the object's own EXT_FTI.
…har* Problem On a compression failure File::encode() formatted zs.msg through spdlog and then threw it. zlib leaves zs.msg NULL for Z_STREAM_ERROR, so the log call received a null char* (spdlog reports "string pointer is null") and the throw sent a raw char* that only catch(const char*) could take, whose handler would then dereference null. The zlib stream and, where owned, the decompressed buffer were both leaked on the way out. Observed: a transmitter aborting with "terminate called after throwing an instance of 'char*'". Code-derived, no spec claim. Raised by reading the error branch while working on the compression path Change Substitutes a fixed string when zlib supplies no message, logs the zlib status code alongside it, releases the stream and any owned buffer, and throws std::runtime_error so an ordinary catch(std::exception&) handles it. Verification T0: builds clean, and the surrounding suite passes. The branch this lands on has no test able to force a zlib stream error, which would need a fault injection point inside encode() that does not exist. Not in this change The deflate call the loop was missing, which reached development separately.
Problem File::encode() passed windowBits 15|16 to deflateInit2 unconditionally, which is gzip framing, while File::decode() passes 15|((encoding == "gzip") ? 16 : 0) to inflateInit2 and so selects on the declared encoding. A file declared with Content-Encoding "deflate" was therefore written with gzip framing and could not be read back by this same library, and the framing on the wire did not match what the declared encoding means. Observed: src/File.cpp, encode() and decode() disagreeing about the same file. [code-derived, against the clause below] Basis RFC 9110 clause 8.4.1.2: "The "deflate" coding is a "zlib" data format [RFC1950] containing a "deflate" compressed data stream [RFC1951] that uses a combination of the Lempel-Ziv (LZ77) compression algorithm and Huffman coding." So "deflate" means the zlib wrapper, windowBits 15, and gzip is 15 with 16 added. The decode side already implemented this correctly; only the encode side did not. Raised by Auditing the compression path while fixing the loop in the same function. Change Compute windowBits from the declared encoding and use it for deflateInit2, making the two halves symmetric. General FLUTE in practice: the MBMS Download Profile permits no Content-Encoding other than gzip, and a session on that profile now refuses anything else outright, so the deflate branch is only ever taken by a non-3GPP caller. It is still wrong to write one framing and read another. Verification T0. The change is a one-expression edit whose correctness rests on the quoted clause and on matching the decode side, which was already right, and the suite still passes at 44 cases. No unit test, for the reason recorded two commits earlier: encode() runs inside the File constructor and reaching it with a usable FEC OTI requires Transmitter::FileDescription, whose merge_fec_oti() is protected, so a test cannot configure one without a Transmitter with sockets and an io_context. A round-trip test over both framings is the right test and is blocked on that gap, which is recorded rather than worked around. Not in this change No change to decode(), which was already correct, and no change to which encodings are accepted.
Problem Any content-encoded object larger than the decompression buffer failed to decode. File::decode() called inflate() with Z_FINISH, which asserts to zlib that the output buffer can take the whole remaining stream; with a fixed 16384-byte buffer that is untrue for any larger object, so inflate returned an error and the object was discarded. Observed: a 100000-byte gzip object logs a decompression error and never completes. Code-derived, no spec claim. Raised by live testing of the compression path Change Both inflate() calls now pass Z_NO_FLUSH, which is the mode for feeding a stream through a buffer smaller than its output, and the loop already handles the partial-output case. Also substitutes a fixed string when zlib supplies no message and releases the stream before throwing, so a decode failure reports rather than formatting a null pointer. Verification T2: a 100000-byte gzip object reaches Z_STREAM_END in six iterations with all 100000 bytes recovered, where before it failed on the first. Not in this change The Content-Length substitution in the FDT parser, which is a separate defect and the next commit.
jordijoangimenez
force-pushed
the
feature/issue66-receiver-tunnel-mode
branch
from
August 22, 2026 13:16
fe9b80e to
7190377
Compare
Contributor
Author
This was referenced Aug 23, 2026
FLUTE version 2 (RFC 6726) signalling, off by default: step one of three, no conformance claimed
#65
Open
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
1 participant
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.
Closes #77.
Change type: crash fix.
msgthrough the logger and threw it. zlib leaves that fieldnull for
Z_STREAM_ERROR, so the log call received a nullchar*and the throw sent a rawchar*,which
catch (std::exception&)does not catch: the process terminated, leaking the stream and anylibrary-owned buffer;
inflate()was called withZ_FINISH, which asserts the output buffer can take the whole remainingstream. The buffer is a fixed 16384 bytes, so any larger object errored out and was discarded although
its bytes were intact;
Content-Encoding: deflatecould not be read back by this library.Separate from #68 on purpose. PR #69, merged 21 August, corrected which zlib function the compression
loop calls. That was one line and covers none of the three above, on
developmentor anywhere else.Baseline: RFC 9110 clause 8.4.1.2 for the deflate framing; the other two are code-derived.
Verification. T1: 43 cases passing. T2: a gzip object above the buffer size received and decoded,
where before it was discarded.