This is an automated email from the ASF dual-hosted git repository.
jark pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fluss.git
The following commit(s) were added to refs/heads/main by this push:
new e6d877b46 [protogen] Fix infinite recursion in RecordsFieldFinder
(#1282)
e6d877b46 is described below
commit e6d877b466919d331aba01cb983b739d197af5a6
Author: Yang Wang <[email protected]>
AuthorDate: Sun Sep 14 20:35:59 2025 +0800
[protogen] Fix infinite recursion in RecordsFieldFinder (#1282)
---
.../generator/generator/ProtobufMessage.java | 2 +-
.../generator/generator/RecordsFieldFinder.java | 15 ++-
fluss-protogen/fluss-protogen-tests/pom.xml | 7 ++
.../src/main/proto/circular_reference.proto | 64 ++++++++++
.../protogen/tests/RecordsFieldFinderTest.java | 137 +++++++++++++++++++++
5 files changed, 221 insertions(+), 4 deletions(-)
diff --git
a/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/ProtobufMessage.java
b/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/ProtobufMessage.java
index 8e10bfb40..1df1d6cb6 100644
---
a/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/ProtobufMessage.java
+++
b/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/ProtobufMessage.java
@@ -172,7 +172,7 @@ public class ProtobufMessage {
w.format(" public boolean isLazilyParsed() {\n");
w.format(
" return %s;\n",
- RecordsFieldFinder.hasRecordsField(message) ? "true" :
"false");
+ new RecordsFieldFinder().hasRecordsField(message) ? "true" :
"false");
w.format(" }\n");
}
diff --git
a/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/RecordsFieldFinder.java
b/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/RecordsFieldFinder.java
index eb504b6e7..6fba174b3 100644
---
a/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/RecordsFieldFinder.java
+++
b/fluss-protogen/fluss-protogen-generator/src/main/java/org/apache/fluss/protogen/generator/generator/RecordsFieldFinder.java
@@ -21,17 +21,26 @@ import io.protostuff.parser.Field;
import io.protostuff.parser.Message;
import io.protostuff.parser.MessageField;
+import java.util.HashSet;
+import java.util.Set;
+
/** Finder to find <code>byte[] records</code> protobuf fields. */
public class RecordsFieldFinder {
public static final String RECORDS_FIELD_NAME = "records";
- public static boolean hasRecordsField(Message message) {
- return
message.getFields().stream().anyMatch(RecordsFieldFinder::hasRecordsField);
+ private Set<Message> visited = new HashSet<>();
+
+ public boolean hasRecordsField(Message message) {
+ visited.add(message);
+ return message.getFields().stream().anyMatch(this::hasRecordsField);
}
- private static boolean hasRecordsField(Field<?> field) {
+ private boolean hasRecordsField(Field<?> field) {
if (field instanceof MessageField) {
+ if (visited.contains(((MessageField) field).getMessage())) {
+ return false;
+ }
return hasRecordsField(((MessageField) field).getMessage());
} else if (field instanceof Field.Bytes) {
return !field.isRepeated() &&
field.getName().equals(RECORDS_FIELD_NAME);
diff --git a/fluss-protogen/fluss-protogen-tests/pom.xml
b/fluss-protogen/fluss-protogen-tests/pom.xml
index b92e6e1b5..992baa793 100644
--- a/fluss-protogen/fluss-protogen-tests/pom.xml
+++ b/fluss-protogen/fluss-protogen-tests/pom.xml
@@ -59,6 +59,13 @@
<artifactId>fluss-test-utils</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.fluss</groupId>
+ <artifactId>fluss-protogen-generator</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+
<!-- Tests depends on ByteBufChannel -->
<dependency>
<groupId>org.apache.fluss</groupId>
diff --git
a/fluss-protogen/fluss-protogen-tests/src/main/proto/circular_reference.proto
b/fluss-protogen/fluss-protogen-tests/src/main/proto/circular_reference.proto
new file mode 100644
index 000000000..eb61524bc
--- /dev/null
+++
b/fluss-protogen/fluss-protogen-tests/src/main/proto/circular_reference.proto
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+syntax = "proto2";
+package org.apache.fluss.protogen.tests;
+
+// Message with circular reference - A references B, B references A
+message CircularA {
+ optional string name = 1;
+ optional CircularB b_ref = 2;
+}
+
+message CircularB {
+ optional int32 id = 1;
+ optional CircularA a_ref = 2;
+}
+
+// Message with records field but no circular reference
+message SimpleRecordsMessage {
+ optional bytes records = 1;
+ optional string metadata = 2;
+}
+
+// Message with circular reference that contains records field
+message CircularRecordsA {
+ optional bytes records = 1;
+ optional CircularRecordsB nested = 2;
+}
+
+message CircularRecordsB {
+ optional string info = 1;
+ optional CircularRecordsA parent = 2;
+}
+
+// Complex circular reference with multiple levels
+message ComplexA {
+ optional string value = 1;
+ optional ComplexB b = 2;
+ optional ComplexC c = 3;
+}
+
+message ComplexB {
+ optional int32 number = 1;
+ optional ComplexA a = 2;
+ optional bytes records = 3;
+}
+
+message ComplexC {
+ optional bool flag = 1;
+ optional ComplexA a = 2;
+}
diff --git
a/fluss-protogen/fluss-protogen-tests/src/test/java/org/apache/fluss/protogen/tests/RecordsFieldFinderTest.java
b/fluss-protogen/fluss-protogen-tests/src/test/java/org/apache/fluss/protogen/tests/RecordsFieldFinderTest.java
new file mode 100644
index 000000000..3d3b552e8
--- /dev/null
+++
b/fluss-protogen/fluss-protogen-tests/src/test/java/org/apache/fluss/protogen/tests/RecordsFieldFinderTest.java
@@ -0,0 +1,137 @@
+/*
+ * 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.fluss.protogen.tests;
+
+import org.apache.fluss.protogen.generator.generator.RecordsFieldFinder;
+
+import io.protostuff.parser.Message;
+import io.protostuff.parser.Proto;
+import io.protostuff.parser.ProtoUtil;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link RecordsFieldFinder}. */
+public class RecordsFieldFinderTest {
+
+ @Test
+ public void testHasRecordsFieldForSimpleMessage() throws Exception {
+ Proto proto = parseProtoFile("bytes.proto");
+ Message rdMessage = findMessage(proto, "RD");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(rdMessage))
+ .as("RD message should have records field")
+ .isTrue();
+ }
+
+ @Test
+ public void testHasRecordsFieldForNestedMessage() throws Exception {
+ Proto proto = parseProtoFile("bytes.proto");
+ Message ordMessage = findMessage(proto, "ORD");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(ordMessage))
+ .as("ORD message should have records field through nested RD")
+ .isTrue();
+ }
+
+ @Test
+ public void testHasRecordsFieldForMessageWithoutRecords() throws Exception
{
+ Proto proto = parseProtoFile("messages.proto");
+ Message xMessage = findMessage(proto, "X");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(xMessage))
+ .as("X message should not have records field")
+ .isFalse();
+ }
+
+ @Test
+ public void testCircularReferenceWithoutRecords() throws Exception {
+ Proto proto = parseProtoFile("circular_reference.proto");
+ Message circularAMessage = findMessage(proto, "CircularA");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(circularAMessage))
+ .as("CircularA message should not have records field")
+ .isFalse();
+ }
+
+ @Test
+ public void testCircularReferenceWithRecords() throws Exception {
+ Proto proto = parseProtoFile("circular_reference.proto");
+ Message circularRecordsAMessage = findMessage(proto,
"CircularRecordsA");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(circularRecordsAMessage))
+ .as("CircularRecordsA message should have records field")
+ .isTrue();
+ }
+
+ @Test
+ public void testSimpleRecordsMessage() throws Exception {
+ Proto proto = parseProtoFile("circular_reference.proto");
+ Message simpleRecordsMessage = findMessage(proto,
"SimpleRecordsMessage");
+
+ RecordsFieldFinder finder = new RecordsFieldFinder();
+ assertThat(finder.hasRecordsField(simpleRecordsMessage))
+ .as("SimpleRecordsMessage should have records field")
+ .isTrue();
+ }
+
+ @Test
+ public void testNewFinderInstanceForEachCall() throws Exception {
+ Proto proto = parseProtoFile("circular_reference.proto");
+ Message circularRecordsAMessage = findMessage(proto,
"CircularRecordsA");
+
+ // Test with first finder instance
+ RecordsFieldFinder finder1 = new RecordsFieldFinder();
+ assertThat(finder1.hasRecordsField(circularRecordsAMessage))
+ .as("CircularRecordsA should have records field (finder1)")
+ .isTrue();
+
+ // Test with second finder instance (should work independently)
+ RecordsFieldFinder finder2 = new RecordsFieldFinder();
+ assertThat(finder2.hasRecordsField(circularRecordsAMessage))
+ .as("CircularRecordsA should have records field (finder2)")
+ .isTrue();
+ }
+
+ private Proto parseProtoFile(String filename) throws Exception {
+ File protoFile = new File("src/main/proto/" + filename);
+ if (!protoFile.exists()) {
+ throw new IOException("Cannot find proto file: " + filename);
+ }
+
+ Proto proto = new Proto();
+ ProtoUtil.loadFrom(protoFile, proto);
+ return proto;
+ }
+
+ private Message findMessage(Proto proto, String messageName) {
+ return proto.getMessages().stream()
+ .filter(message -> message.getName().equals(messageName))
+ .findFirst()
+ .orElseThrow(
+ () -> new IllegalArgumentException("Message not found:
" + messageName));
+ }
+}
