This is an automated email from the ASF dual-hosted git repository.
Croway pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new c52d3cfa1c34 CAMEL-24442: camel-thrift - unmarshal into a copy instead
of the shared defaultInstance (#25823)
c52d3cfa1c34 is described below
commit c52d3cfa1c343a49261d77a5213dba92244a38f6
Author: Andrea Cosentino <[email protected]>
AuthorDate: Fri Aug 28 11:50:15 2026 +0200
CAMEL-24442: camel-thrift - unmarshal into a copy instead of the shared
defaultInstance (#25823)
* CAMEL-24442: camel-thrift - unmarshal into a copy instead of the shared
defaultInstance
ThriftDataFormat.unmarshal() deserialized into the defaultInstance field
and returned
that same object. The data format is shared by every exchange on the route,
and
Thrift's TBase.read() assigns only the fields present in the incoming bytes
without
clearing the object first, so:
- a message that omitted an optional field kept the value left there by the
previous
message - deterministic, no concurrency needed;
- concurrent unmarshals interleaved field writes into the one object;
- every in-flight body was literally the same reference.
Deserialize into defaultInstance.deepCopy() and return that.
ProtobufDataFormat
already builds a new instance per unmarshal. As a side effect
defaultInstance is left
untouched and now works as the template its name promises: values preset on
it are
visible on every message, where before the first message overwrote them.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
* CAMEL-24442: Clear copied Thrift instance before unmarshal
---------
Signed-off-by: Andrea Cosentino <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Croway <[email protected]>
---
components/camel-thrift/pom.xml | 5 ++
.../camel/dataformat/thrift/ThriftDataFormat.java | 14 +++-
.../thrift/ThriftUnmarshalIsolationTest.java | 89 ++++++++++++++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 14 ++++
4 files changed, 119 insertions(+), 3 deletions(-)
diff --git a/components/camel-thrift/pom.xml b/components/camel-thrift/pom.xml
index 7c290fe7bc0c..d59a03ab2384 100644
--- a/components/camel-thrift/pom.xml
+++ b/components/camel-thrift/pom.xml
@@ -75,6 +75,11 @@
<artifactId>gson</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git
a/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java
b/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java
index 3ab4a716be25..83530bfafef2 100644
---
a/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java
+++
b/components/camel-thrift/src/main/java/org/apache/camel/dataformat/thrift/ThriftDataFormat.java
@@ -158,23 +158,31 @@ public class ThriftDataFormat extends ServiceSupport
}
@Override
+ @SuppressWarnings("rawtypes")
public Object unmarshal(final Exchange exchange, final InputStream
inputStream) throws Exception {
TDeserializer deserializer;
ObjectHelper.notNull(defaultInstance, "defaultInstance or
instanceClassName must be set", this);
+ // The data format is shared by every exchange on the route and
TBase.read() only assigns the
+ // fields present in the incoming bytes, so deserializing into
defaultInstance would let one
+ // message read or overwrite another's fields. Deserialize into a copy
instead, clearing it first
+ // so values set on defaultInstance are not inherited when they are
absent from the input.
+ TBase instance = defaultInstance.deepCopy();
+ instance.clear();
+
if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_JSON)) {
deserializer = new TDeserializer(new TJSONProtocol.Factory());
- deserializer.deserialize(defaultInstance,
IOUtils.toByteArray(inputStream));
+ deserializer.deserialize(instance,
IOUtils.toByteArray(inputStream));
} else if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_BINARY)) {
deserializer = new TDeserializer(new TBinaryProtocol.Factory());
- deserializer.deserialize(defaultInstance,
IOUtils.toByteArray(inputStream));
+ deserializer.deserialize(instance,
IOUtils.toByteArray(inputStream));
} else if (contentTypeFormat.equals(CONTENT_TYPE_FORMAT_SIMPLE_JSON)) {
throw new CamelException("Simple JSON format is avalable for the
message marshalling only");
} else {
throw new CamelException("Invalid thrift content type format: " +
contentTypeFormat);
}
- return defaultInstance;
+ return instance;
}
@SuppressWarnings("rawtypes")
diff --git
a/components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java
b/components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java
new file mode 100644
index 000000000000..0a879be46b2c
--- /dev/null
+++
b/components/camel-thrift/src/test/java/org/apache/camel/dataformat/thrift/ThriftUnmarshalIsolationTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.dataformat.thrift;
+
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.dataformat.thrift.generated.Operation;
+import org.apache.camel.dataformat.thrift.generated.Work;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * A data format instance is shared by every exchange on the route, and
Thrift's {@code TBase.read()} assigns only the
+ * fields present in the incoming bytes. Two messages unmarshalled through the
same data format must therefore not be
+ * able to observe each other's fields - including through an optional field
that the second message omits.
+ */
+class ThriftUnmarshalIsolationTest extends CamelTestSupport {
+
+ @Test
+ void anOmittedOptionalFieldDoesNotInheritThePreviousMessageValue() {
+ Work withComment = new Work();
+ withComment.num1 = 1;
+ withComment.num2 = 2;
+ withComment.op = Operation.ADD;
+ withComment.comment = "first message";
+
+ Work withoutComment = new Work();
+ withoutComment.num1 = 3;
+ withoutComment.num2 = 4;
+ withoutComment.op = Operation.SUBTRACT;
+
+ Object firstBytes = template.requestBody("direct:marshal",
withComment);
+ Object secondBytes = template.requestBody("direct:marshal",
withoutComment);
+
+ Work first = (Work) template.requestBody("direct:unmarshal",
firstBytes);
+ Work second = (Work) template.requestBody("direct:unmarshal",
secondBytes);
+
+ assertThat(first.getComment()).isEqualTo("first message");
+ assertThat(second.getComment()).isNull();
+ assertThat(second.getNum1()).isEqualTo(3);
+ assertThat(first).isNotSameAs(second);
+ }
+
+ @Test
+ void anOmittedOptionalFieldDoesNotInheritTheDefaultInstanceValue() {
+ Work withoutComment = new Work();
+ withoutComment.num1 = 3;
+ withoutComment.num2 = 4;
+ withoutComment.op = Operation.SUBTRACT;
+
+ Object bytes = template.requestBody("direct:marshal", withoutComment);
+
+ Work result = (Work)
template.requestBody("direct:unmarshal-populated-default", bytes);
+
+ assertThat(result.getComment()).isNull();
+ assertThat(result.getNum1()).isEqualTo(3);
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ ThriftDataFormat format = new ThriftDataFormat(new Work());
+ Work populatedDefault = new Work();
+ populatedDefault.comment = "default value";
+ ThriftDataFormat populatedDefaultFormat = new
ThriftDataFormat(populatedDefault);
+ from("direct:marshal").marshal(format);
+ from("direct:unmarshal").unmarshal(format);
+
from("direct:unmarshal-populated-default").unmarshal(populatedDefaultFormat);
+ }
+ };
+ }
+}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index 802926eeedab..3fcc5b76c9b6 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -667,3 +667,17 @@ Audience validation already behaved this way; the two are
now consistent.
Deployments whose introspection endpoint omits `iss` must either have it
include the claim, or turn
`validateIssuer` off. The locally verified JWT path is unaffected — it uses
Keycloak's own
`TokenVerifier.RealmUrlCheck`.
+
+=== camel-thrift
+
+The `thrift` data format used to deserialize into its own `defaultInstance`
and return that object to
+every exchange. Because Thrift's `TBase.read()` assigns only the fields
present in the incoming bytes,
+a message that omitted an optional field kept the value left there by the
previous message, concurrent
+unmarshals interleaved into the same object, and all in-flight bodies were the
same reference.
+
+`unmarshal` now creates a copy of `defaultInstance`, clears it to the
generated type's default-constructor
+state, and deserializes into that copy. Each exchange therefore gets its own
object, omitted fields do not
+inherit values set on `defaultInstance`, and `defaultInstance` itself remains
untouched.
+
+Routes that compared unmarshalled bodies by identity, or that mutated one body
expecting the change to
+be visible on another, must be updated.
