allthingssecurity opened a new pull request, #29043: URL: https://github.com/apache/camel/pull/29043
# Description [CAMEL-26433](https://issues.apache.org/jira/browse/CAMEL-26433) Follow-up of CAMEL-26090 (#28758), mentioned in its review: the same size limit for the HL7 MLLP MINA codec. CAMEL-26090 added `maxFrameLength` to the Netty decoder (`HL7MLLPNettyDecoderFactory`). The MINA codec (`HL7MLLPCodec`, used as `mina:tcp://...?codec=#hl7codec`) had no limit: `HL7MLLPDecoder` is a MINA `CumulativeProtocolDecoder`, which keeps the bytes of a session until it sees both end bytes, so a client that sends data without the end bytes made the buffer of its session grow without bound. ## Changes - `HL7MLLPCodec.setMaxFrameLength(int)` / `getMaxFrameLength()`, stored in `HL7MLLPConfig` (which has it since CAMEL-26090, with its check that it is positive). The default stays `Integer.MAX_VALUE` (no limit), so existing `mina:tcp` routes behave the same (see below for `mina:udp` and `mina:vm`). - `HL7MLLPDecoder` counts the bytes of the current message from the start of its data (after the end bytes of the previous message, or the start of the session's data), the start byte included and the end bytes not, as the Netty decoder does. Bytes before the start byte count too, as in the Netty decoder (its frame is everything up to the end bytes). As soon as that count is over the limit, without waiting for the end bytes: - the data received so far is discarded (with the rest of the current read), and MINA's buffer of the session is dropped (`CumulativeProtocolDecoder.dispose`), because the decoder throws and MINA then keeps that buffer as it is, so its bytes would be read again; - the decoder throws a `ProtocolDecoderException` ("HL7 MLLP message is longer than maxFrameLength of N bytes: discarded"); MINA calls the handler's `exceptionCaught`, where the camel-mina consumer closes the session with a WARN (and rethrows, which MINA logs at WARN too); a MINA producer fails the exchange when a reply is longer. The exception has an empty hex dump: `ProtocolCodecFilter` adds a hex dump of the whole read (here, the message, which can hold patient data) to a decoder exception whose `getHexdump()` is null, and that dump would end up in both WARNs; the Netty `TooLongFrameException` has no data either; - when the end bytes of the long message are not in the discarded data, the decoder skips the next reads up to them (over several reads too), and decodes the next message after them, as the Netty decoder does in its discarding mode. - The decoder keeps only the data of the current message between two reads. Before, it returned with the position at 0 whenever a read did not end exactly at the end of a message, so MINA kept every byte since the last read that did: the messages decoded before, and end bytes without a start byte, piled up in the buffer of the session (also on main, and also without a limit; with `maxFrameLength` alone the buffer would still grow, as the limit counts from the last end bytes). It now returns with the position at the start of the data of the current message (MINA keeps the bytes from there) and moves its stored positions down by as much. The data of a message that is not complete yet is kept as before. - The decoder calls `setTransportMetadataFragmentation(false)`, so `CumulativeProtocolDecoder` keeps the data of an incomplete message between reads also on a session whose transport MINA does not regard as fragmenting (`mina:udp`, `mina:vm`; camel-mina sets the codec on both). Without it, MINA hands each read to the decoder on its own and keeps nothing, while the decoder keeps positions into that read: on main a datagram with a message and the start of the next one made `ProtocolCodecFilter` loop on it for good (the message delivered once, then the I/O thread spinning); with the new positions alone it would deliver the message again and again. Now such a read gives each message once (test below). This changes main for HL7 over UDP or the VM pipe: the start of a message is now kept for the next datagram of the same session instead of the spin; HL7 MLLP is a stream protocol, so this is the behaviour of the TCP transport. The partial message at the end of a datagram is joined with t he next datagram of the same peer (`\x0bA\x1c\r\x0bB` then `\x0bC\x1c\r` gives `A` and `B\x0bC`), and the kept bytes are bounded by `maxFrameLength` when set, and dropped when the session goes. - One difference from the Netty decoder, on purpose: an end byte 1 is not counted until the next byte shows whether it is the end byte 2, so a message of exactly `maxFrameLength` bytes is always decoded, also when its two end bytes arrive in two reads. (The Netty `DelimiterBasedFrameDecoder` counts it, so there such a message fails when its first end byte arrives without the second; the docs keep that note for Netty.) - `hl7-dataformat.adoc` (and its catalog copy): `maxFrameLength` in the options table now describes both codecs, and the MINA section has an example; the javadoc of `HL7MLLPConfig.setMaxFrameLength` names both. No upgrade guide entry: a new option whose default keeps the old behaviour. Noticed while reading the decoder, not changed here (all on main as well; follow-up JIRA I will file): - `HL7MLLPDecoder` compares each byte (signed) with the configured start and end bytes (`char`), so a start or end byte above `0x7F` never matches in the MINA decoder; CAMEL-26090 fixed the same kind of problem in the Netty decoder (its delimiter was encoded with the default charset). - A start byte, and end bytes without a start, do not reset the previous byte, so `\x1c\x0b\r` (or `\x1c\r\x0b\r`) is taken as a message of two bytes and fails with an `IllegalArgumentException` ("fieldSize cannot be negative") with `produceString`, or a `NegativeArraySizeException` for a byte array; the MINA consumer closes that sender's session. ## Tests New `HL7MLLPDecoderMaxFrameLengthTest` (decoder level, with MINA's `ProtocolCodecSession`, which reports no fragmentation: the decoder accumulates there as on a TCP session, the same path of `CumulativeProtocolDecoder`; JUnit assertions as in the module, the same shapes as `HL7MLLPNettyDecoderMaxFrameLengthTest`), 13 tests: - a message at the limit is decoded, also with its end bytes in two reads; - a message over the limit fails (with an empty hex dump, and no dump in its message), and the next message is decoded; - a message over the limit over two reads (the first part kept by MINA's session buffer, the second with the end bytes) fails, and the next message is decoded (nothing of the long message is read again); - data without end bytes fails once over the limit (over two reads), the rest is discarded up to its end bytes over three more reads, and the next message is decoded; - bytes before the start byte count; - each of several messages in one read (and a message that ends in the next read) has the limit from the end of the previous one; - 1000 rounds of reads that each end in the middle of a message, and 100 reads of end bytes without a start byte: MINA keeps at most 50 bytes for the session, and the messages are decoded; - through MINA's `ProtocolCodecFilter` on that session without fragmentation: a read with a message and the start of the next one, then the end of it, gives each message once (bounded with a JUnit `@Timeout` in a separate thread, as on main the filter loops for good, and the recorder fails on a third message, as without the change in this PR the message is delivered again and again); - the codec applies its limit to its decoders, the default is no limit (a 1 MB message over two reads), the limit must be positive. New `HL7MLLPMinaMaxFrameLengthRouteTest`: a `mina:tcp` HL7 listener with `maxFrameLength=1024` closes the connection of a client that sends 2000 bytes without the end bytes, and then answers another client's message (as `HL7MLLPNettyMaxFrameLengthRouteTest`). Without the fix (the decoder of main with the new codec option), 11 of the 13 decoder tests fail (main does not accumulate on the test session, and has no limit), and the route test fails (the connection is not closed); the 2 others pass (a message at the limit, the positive check of the config). Mutants, each failing at least one test: no `dispose` of MINA's session buffer (the two-read test), counting an end byte 1 (the messages at the limit), no new start of the data after a decoded message (several messages in one read, the kept-data test), position 0 as before instead of the start of the current data (the two kept-data tests), a plain `ProtocolDecoderException` (the hex dump assertion), no `setTransportMetadataFragmentation(false)` (the filter test and the tests over several reads). - `HL7MLLPDecoderMaxFrameLengthTest` 20 times in one JVM: 260 tests, 0 failures; `HL7MLLPMinaMaxFrameLengthRouteTest` 20 times: 20 tests, 0 failures. - camel-hl7 tests (all of the module, with the existing MINA codec tests `HL7MLLPCodec*Test` and `HL7RouteTest`): 106 tests, 0 failures. The tests ran against mina-core 2.2.9 (Camel pins 2.2.10, not available offline here); CI runs them with 2.2.10. - Formatter and import-sort: no changes; `hl7-dataformat.adoc` copied to the catalog (identical). # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL-26433) filed for the change (usually before you start working on it): [CAMEL-26433](https://issues.apache.org/jira/browse/CAMEL-26433). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built camel-hl7 with the modules it needs and ran its tests, including the formatter and import-sort plugins, and copied `hl7-dataformat.adoc` to the catalog. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
