This is an automated email from the ASF dual-hosted git repository. Croway pushed a commit to branch camel-4.22.x in repository https://gitbox.apache.org/repos/asf/camel.git
commit 270b6adb19214175b20a31a5c40e8f7b5cf6d3ea Author: Andrea Cosentino <[email protected]> AuthorDate: Mon Aug 31 10:17:58 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]> (cherry picked from commit c52d3cfa1c343a49261d77a5213dba92244a38f6) Co-authored-by: Claude Opus 5 (1M context) <[email protected]> --- components/camel-thrift/pom.xml | 5 ++ .../camel/dataformat/thrift/ThriftDataFormat.java | 14 +++- .../thrift/ThriftUnmarshalIsolationTest.java | 89 ++++++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/components/camel-thrift/pom.xml b/components/camel-thrift/pom.xml index 5731a0e117fd..c8eeba8d8a8d 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); + } + }; + } +}
