This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch 2.2.X
in repository https://gitbox.apache.org/repos/asf/mina.git
The following commit(s) were added to refs/heads/2.2.X by this push:
new ec995d1cc Don't replay already consumed data when an error is thrown
while decoding data
ec995d1cc is described below
commit ec995d1ccf538b693ff51a157df2d2a59ad38a41
Author: Emmanuel Lécharny <[email protected]>
AuthorDate: Tue Sep 8 15:19:46 2026 +0200
Don't replay already consumed data when an error is thrown while
decoding data
---
.../filter/codec/CumulativeProtocolDecoder.java | 39 +++++++++----
.../codec/CumulativeProtocolDecoderTest.java | 68 ++++++++++++++++++++++
2 files changed, 96 insertions(+), 11 deletions(-)
diff --git
a/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
b/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
index 7eddfc0fc..2a4530645 100644
---
a/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
+++
b/mina-core/src/main/java/org/apache/mina/filter/codec/CumulativeProtocolDecoder.java
@@ -172,20 +172,37 @@ public abstract class CumulativeProtocolDecoder extends
ProtocolDecoderAdapter {
usingSessionBuffer = false;
}
- for (;;) {
- int oldPos = buf.position();
- boolean decoded = doDecode(session, buf, out);
- if (decoded) {
- if (buf.position() == oldPos) {
- throw new IllegalStateException("doDecode() can't return
true when buffer is not consumed.");
- }
-
- if (!buf.hasRemaining()) {
+ try {
+ for (;;) {
+ int oldPos = buf.position();
+ boolean decoded = doDecode(session, buf, out);
+ if (decoded) {
+ if (buf.position() == oldPos) {
+ throw new IllegalStateException("doDecode() can't
return true when buffer is not consumed.");
+ }
+
+ if (!buf.hasRemaining()) {
+ break;
+ }
+ } else {
break;
}
- } else {
- break;
}
+ } catch (Exception | Error e) {
+ // doDecode() threw: the cumulative buffer is still flipped in
+ // read mode, with the messages decoded (and delivered) by the
+ // earlier iterations before its position. If we left it stored
+ // in the session, the next decode() call would append new data
+ // at the current position and flip the buffer again, re-reading
+ // - and re-delivering to the handler - those already-consumed
+ // messages. A decoding error may cost the accumulated remainder,
+ // but it must never replay consumed input, so discard the buffer
+ // before propagating the exception.
+ if (usingSessionBuffer) {
+ removeSessionBuffer(session);
+ }
+
+ throw e;
}
// if there is any data left that cannot be decoded, we store
diff --git
a/mina-core/src/test/java/org/apache/mina/filter/codec/CumulativeProtocolDecoderTest.java
b/mina-core/src/test/java/org/apache/mina/filter/codec/CumulativeProtocolDecoderTest.java
index 055b892b4..602a3c0f2 100644
---
a/mina-core/src/test/java/org/apache/mina/filter/codec/CumulativeProtocolDecoderTest.java
+++
b/mina-core/src/test/java/org/apache/mina/filter/codec/CumulativeProtocolDecoderTest.java
@@ -143,6 +143,49 @@ public class CumulativeProtocolDecoderTest {
}
}
+ @Test
+ public void testDecoderExceptionDiscardsSessionBuffer() throws Exception {
+ FaultyIntegerDecoder faultyDecoder = new FaultyIntegerDecoder();
+
+ // First chunk: an incomplete integer, stored in the session buffer.
+ buf.putShort((short) 0);
+ buf.flip();
+ faultyDecoder.decode(session, buf, session.getDecoderOutput());
+ assertEquals(0, session.getDecoderOutputQueue().size());
+
+ // Second chunk: completes integer 1 (decoded and delivered), then
+ // the poisoned integer 0xBAD that makes doDecode() throw after the
+ // first message was already delivered.
+ buf.clear();
+ buf.putShort((short) 1);
+ buf.putInt(0xBAD);
+ buf.flip();
+
+ try {
+ faultyDecoder.decode(session, buf, session.getDecoderOutput());
+ fail("The poisoned integer should have made the decoder throw");
+ } catch (ProtocolDecoderException e) {
+ // OK
+ }
+
+ assertEquals(1, session.getDecoderOutputQueue().size());
+ assertEquals(1, session.getDecoderOutputQueue().poll());
+
+ // Further data must NOT re-deliver integer 1: the cumulative buffer
+ // was left flipped by the exception and must have been discarded,
+ // not replayed on the next decode.
+ buf.clear();
+ buf.putInt(7);
+ buf.flip();
+ faultyDecoder.decode(session, buf, session.getDecoderOutput());
+
+ assertEquals(1, session.getDecoderOutputQueue().size());
+ assertEquals(7, session.getDecoderOutputQueue().poll());
+
+ faultyDecoder.dispose(session);
+ }
+
+
private static class IntegerDecoder extends CumulativeProtocolDecoder {
/**
* Default constructor
@@ -164,6 +207,31 @@ public class CumulativeProtocolDecoderTest {
}
}
+ private static class FaultyIntegerDecoder extends
CumulativeProtocolDecoder {
+ /**
+ * Default constructor
+ */
+ public FaultyIntegerDecoder() {
+ super();
+ }
+
+ @Override
+ protected boolean doDecode(IoSession session, IoBuffer in,
ProtocolDecoderOutput out) throws Exception {
+ if (in.remaining() < 4) {
+ return false;
+ }
+
+ int value = in.getInt();
+
+ if (value == 0xBAD) {
+ throw new ProtocolDecoderException("poisoned integer");
+ }
+
+ out.write(value);
+ return true;
+ }
+ }
+
private static class WrongDecoder extends CumulativeProtocolDecoder {
/**
* Default constructor
