From 733400342729656c50a57698796a92527c024259 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Thu, 28 May 2026 02:03:51 +0200 Subject: [PATCH] THRIFT-6054: Limit recursion depth in Kotlin struct read/write Client: kotlin Add recursion depth checking to TProtocol.readStruct() and TProtocol.writeStruct(), the helper methods used by Kotlin-generated struct serialization code. The depth counter and limit (64) are managed by the existing incrementRecursionDepth() / decrementRecursionDepth() infrastructure in TProtocol; the decrement is now guaranteed via a try/finally block. Co-Authored-By: Claude Sonnet 4.6 --- .../org/apache/thrift/protocol/TProtocol.java | 24 +++-- .../org/apache/thrift/RecursionDepthTest.kt | 93 +++++++++++++++++++ 2 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 lib/kotlin/src/test/kotlin/org/apache/thrift/RecursionDepthTest.kt diff --git a/lib/java/src/main/java/org/apache/thrift/protocol/TProtocol.java b/lib/java/src/main/java/org/apache/thrift/protocol/TProtocol.java index b91c4cfb16c..d1a07683474 100644 --- a/lib/java/src/main/java/org/apache/thrift/protocol/TProtocol.java +++ b/lib/java/src/main/java/org/apache/thrift/protocol/TProtocol.java @@ -153,9 +153,14 @@ public final void writeField(TField field, WriteCallback callback) throws } public final void writeStruct(TStruct struct, WriteCallback callback) throws TException { - writeStructBegin(struct); - callback.call(null); - writeStructEnd(); + incrementRecursionDepth(); + try { + writeStructBegin(struct); + callback.call(null); + writeStructEnd(); + } finally { + decrementRecursionDepth(); + } } public final void writeMessage(TMessage message, WriteCallback callback) throws TException { @@ -190,10 +195,15 @@ public final T readMessage(ReadCallback callback) throws TExcep * @throws TException when any sub-operation failed */ public final T readStruct(ReadCallback callback) throws TException { - TStruct tStruct = readStructBegin(); - T t = callback.accept(tStruct); - readStructEnd(); - return t; + incrementRecursionDepth(); + try { + TStruct tStruct = readStructBegin(); + T t = callback.accept(tStruct); + readStructEnd(); + return t; + } finally { + decrementRecursionDepth(); + } } /** diff --git a/lib/kotlin/src/test/kotlin/org/apache/thrift/RecursionDepthTest.kt b/lib/kotlin/src/test/kotlin/org/apache/thrift/RecursionDepthTest.kt new file mode 100644 index 00000000000..45c06a7405d --- /dev/null +++ b/lib/kotlin/src/test/kotlin/org/apache/thrift/RecursionDepthTest.kt @@ -0,0 +1,93 @@ +/* + * 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.thrift + +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull +import org.apache.thrift.protocol.TBinaryProtocol +import org.apache.thrift.protocol.TProtocol +import org.apache.thrift.protocol.TProtocolException +import org.apache.thrift.transport.TMemoryInputTransport +import org.junit.jupiter.api.Test + +internal class RecursionDepthTest { + + private fun makeProtocol(): TProtocol { + val transport = TMemoryInputTransport(ByteArray(0)) + return TBinaryProtocol(transport) + } + + @Test + fun testReadStructDepthLimitThrows() { + val proto = makeProtocol() + // Drive depth to the limit by direct increments + repeat(64) { proto.incrementRecursionDepth() } + // Next readStruct must throw DEPTH_LIMIT + val ex = assertFailsWith { proto.readStruct { _ -> null } } + assertEquals(TProtocolException.DEPTH_LIMIT, ex.type) + // Depth counter must be restored by the finally block + repeat(64) { proto.decrementRecursionDepth() } + } + + @Test + fun testWriteStructDepthLimitThrows() { + val proto = makeProtocol() + repeat(64) { proto.incrementRecursionDepth() } + val struct = org.apache.thrift.protocol.TStruct("Test") + val ex = assertFailsWith { + proto.writeStruct(struct) {} + } + assertEquals(TProtocolException.DEPTH_LIMIT, ex.type) + repeat(64) { proto.decrementRecursionDepth() } + } + + @Test + fun testReadStructDepthLimitMessageNonEmpty() { + val proto = makeProtocol() + repeat(64) { proto.incrementRecursionDepth() } + val ex = assertFailsWith { proto.readStruct { _ -> null } } + assertNotNull(ex.message) + repeat(64) { proto.decrementRecursionDepth() } + } + + @Test + fun testDepthCounterRestoredAfterLimitOnRead() { + val proto = makeProtocol() + repeat(63) { proto.incrementRecursionDepth() } + // At depth 63, one more increment should work + proto.incrementRecursionDepth() + // Now at 64 — next readStruct must throw and restore + assertFailsWith { proto.readStruct { _ -> null } } + // After the throw, depth is still 64 (we manually decremented all ours after) + repeat(64) { proto.decrementRecursionDepth() } + // Depth now 0 — a readStruct should no longer throw + proto.readStruct { _ -> null } + } + + @Test + fun testReadStructSucceedsUnderLimit() { + val proto = makeProtocol() + // 63 nested increments — 64th readStruct should still succeed + repeat(63) { proto.incrementRecursionDepth() } + proto.readStruct { _ -> null } // must not throw (depth goes 64 → 63 after) + repeat(63) { proto.decrementRecursionDepth() } + } +}