Fix serializer recursion handling and bump version to 1.0.3 by karle0wne · Pull Request #40 · valitydev/geck

karle0wne · 2026-03-24T10:16:02Z

Стабилизируем serializer и migrator в тех местах, где библиотека вела себя непредсказуемо на сложных данных. Несколько исправлений: циклические thrift-структуры, рекурсивные схемы при генерации mock-объектов, передача данных через цепочку миграций и разбор некорректного входа в разных представлениях. Раньше такие случаи могли заканчиваться переполнением стека, бесконечным обходом структуры или просто неверной обработкой данных, поэтому задача этого патча именно в том, чтобы сделать поведение явным и безопасным.

В TBaseProcessor добавлена проверка на зацикленные ссылки, в MockTBaseProcessor такая же защита появилась для рекурсивных thrift-схем, в BaseMigrationManager исправлена логика последовательной миграции, чтобы каждый следующий шаг получал уже преобразованный результат предыдущего шага. Заодно приведена в порядок обработка отдельных пограничных случаев в ObjectProcessor, JsonProcessor и XMLProcessor: исправлено чтение ByteBuffer, убран ошибочный проход мимо ветки SET, и для неизвестных типов библиотека теперь сразу сообщает об ошибке понятным исключением.

strug · 2026-03-25T12:24:04Z

migrator/src/test/java/dev/vality/geck/migrator/kit/TestBaseMigrationManager.java

+    private static class AccumulatingMigrator implements Migrator {
+
+        @Override
+        public <I, O> O migrate(I data, MigrationPoint mPoint, SerializerSpec<I, O> serializerSpec) {
+            return (O) (String.valueOf(data) + ((MigrationSpecImpl) mPoint.getMigrationSpec()).getSpec().replace("TEST", ""));
+        }
+
+        @Override
+        public String getMigrationType() {
+            return "TEST";
+        }
+    }


Добавь контекста, что проверяется, в стиле Панкрашкина ничего не понять

добавил комментов к тестам и названия тестов, тут суть что у нас есть способ в геке мигрировать трифт , но был баг что он брал промежуточный результат предыдущего шага всегда исходное и миграция по сути не работала и тесты проверяют работу BaseMigrationManager

strug · 2026-03-25T12:24:38Z

serializer/src/main/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessor.java

    private ValueGenerator valueGenerator;

    private Map<String, FieldHandler> fieldHandlers = new HashMap<>();
+    private final Deque<Class<? extends TBase>> structPath = new ArrayDeque<>();


Почему выбрал Deque?

поправил на сет , первоначально попал сюда как идея хранить путь рекурсии , но тк нам тут нужна только проверка встречался ли тип в текущей ветки или нет и этого хватит и через сет

strug · 2026-03-25T12:32:53Z

serializer/src/main/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessor.java


    private void processStruct(StructMetaData structMetaData, StructHandler handler) throws IOException {
+        Class<? extends TBase> structClass = structMetaData.getStructClass();
+        if (structPath.contains(structClass)) {


Правильно понимаю, что мы просто StackOv меняем на другое исключение, мб мы хотим просто пропустить ее и догенерить то что можем? Только ли это используется в тестах или может сломать и основные процессы?

да, подробнее расписал в шапке ответа , затрагивает и TBaseProcessor.java и MockTBaseProcessor.java , то есть и основу

strug · 2026-03-25T12:39:06Z

serializer/src/main/java/dev/vality/geck/serializer/kit/object/ObjectProcessor.java

+                    ByteBuffer duplicate = ((ByteBuffer) value).duplicate();
+                    byte[] bytes = new byte[duplicate.remaining()];
+                    duplicate.get(bytes);
+                    handler.value(bytes);


Какой то набор приведений, поясни что тут было сделано и зачем

Какой то набор приведений, поясни что тут было сделано и зачем

Лучше не следовать стилю Панкрашкина, у него ничего не понятно он сишник

поправил . цель корректно обработать ByteBuffer при сериализации

strug · 2026-03-25T12:46:40Z

serializer/src/main/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessor.java

        } catch (InstantiationException | IllegalAccessException ex) {
            throw new IOException(ex);
+        } finally {
+            structPath.pop();


Точно ли нужно создавать Deque для того чтобы проверить что новый объект который тут же создается и тот который пришел будут одного типа, на сколько я понял логику? почему бы просто это и не проверить, для чего структура на уровне корня объекта с такой сложной логикой?

да, не нужен , выше написал, нужна только проверка и ииспользуем сет . структура хранения не локальна потому что отлавливаем граф как A -> A так и A -> B -> A в данной ветке , нужен для всей ветки обхода, но обернут в ThreadLocal для изоляции и живет в контексте одного запуска process

strug · 2026-03-25T13:06:14Z

serializer/src/test/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessorTest.java

+
+        @Override
+        public boolean equals(Object other) {
+            return this == other;


почему такой метод equels?

ошибка, сделал RecursiveStruct

strug · 2026-03-25T13:06:57Z

serializer/src/test/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessorTest.java

+            return META_DATA_MAP;
+        }
+
+        public enum _Fields implements TFieldIdEnum {


почему тут еще и enum прям в тесте описан? Для чего он нужен

вынес RecursiveStruct отдельно, TBase требует описание трифт полей, в код гене также

strug · 2026-03-25T13:07:08Z

serializer/src/test/java/dev/vality/geck/serializer/kit/mock/MockTBaseProcessorTest.java

+            }
+
+            public static _Fields findByThriftId(int fieldId) {
+                return fieldId == 1 ? NEXT : null;


что делает этот метод?

маппинг в энам для fieldForId , это все имитация какого то интересного трифт объекта с рекурсией

strug · 2026-03-25T13:08:59Z

serializer/src/test/java/dev/vality/geck/serializer/kit/tbase/TBaseProcessorTest.java

+        BinaryTest binaryTest = new BinaryTest();
+        ByteBuffer buffer = ByteBuffer.wrap(new byte[]{9, 1, 2, 8});
+        buffer.position(1);
+        buffer.limit(3);
+
+        binaryTest.setData(buffer);
+        binaryTest.setDataInList(Collections.emptyList());
+        binaryTest.setDataInSet(Collections.emptySet());
+        binaryTest.setDataInMap(Collections.emptyMap());
+
+        BinaryCapturingHandler handler = new BinaryCapturingHandler();
+        new TBaseProcessor().process(binaryTest, handler);


что тут проверяется? и для чего

это тест по багу с ByteBuffer , смотрим что в сериализацию не попадает мусор

strug · 2026-03-25T13:09:55Z

serializer/src/test/java/dev/vality/geck/serializer/kit/tbase/TBaseProcessorTest.java

+        );
+    }
+
+    private static class BinaryCapturingHandler implements StructHandler<byte[]> {


еще какой-то страшный класс внутри теста, точно нужен тут?

strug · 2026-03-25T13:11:15Z

Стабилизируем serializer и migrator в тех местах, где библиотека вела себя непредсказуемо на сложных данных. Несколько исправлений: циклические thrift-структуры, рекурсивные схемы при генерации mock-объектов, передача данных через цепочку миграций и разбор некорректного входа в разных представлениях. Раньше такие случаи могли заканчиваться переполнением стека, бесконечным обходом структуры или просто неверной обработкой данных, поэтому задача этого патча именно в том, чтобы сделать поведение явным и безопасным.

В TBaseProcessor добавлена проверка на зацикленные ссылки, в MockTBaseProcessor такая же защита появилась для рекурсивных thrift-схем, в BaseMigrationManager исправлена логика последовательной миграции, чтобы каждый следующий шаг получал уже преобразованный результат предыдущего шага. Заодно приведена в порядок обработка отдельных пограничных случаев в ObjectProcessor, JsonProcessor и XMLProcessor: исправлено чтение ByteBuffer, убран ошибочный проход мимо ветки SET, и для неизвестных типов библиотека теперь сразу сообщает об ошибке понятным исключением.

Напоминает результат работы ИИ, в кратце что хотели сделать?

karle0wne

... хотели ...

Да, без водкии в геке не разбраться.
в корневом описании хотел показать основные точки правок, но попробую еще раз сформулировать

TBaseProcessor.java MockTBaseProcessor.java могут уходить в бесконечную рекурсию и выдавать StackOverflowError и этот патч не решает корень этой проблемы (далее объясню) а делает явно перехват сигнатур похожих на рекурсию и выкидывает исключения , помимо этого по мелочам чистит баги, также тесты которые проверяют что эти кейсы закрылись

не может решить проблему в корне потому что сам гек имеет архитектурные ограничения (трифт джава ген это граф, json это дерево), чтоб это решить нужно вводить доп структуры где хранятся референсы на объекты которые проходили (как то подружить дерево и граф) , и судя по всему для этого нужно было бы сделать чуть ли не гек в2 , что не целесообразно. потому что там много форматов msdpack, xml, json и для каждого надо отдельно прорабатывать , поэтому остановились пока на такой небольшой доработки

то есть если резюмировать план работ

перехват рекурсий
баг с ByteBuffer когда нужен захват только активной части - он мог сериализовывать лишние байты (есть тест на эту ситуацию)
баг когда не останавливалось выполенение в ObjectProcessor.java когда нашел set
баг BaseMigrationManager.java что он не получал следующий шаг в миграции трифт схемы а всегда обращался к исходным данным
тесты
по мелочи

Fix serializer recursion handling and bump version

a8038e5

karle0wne requested a review from a team as a code owner March 24, 2026 10:16

strug reviewed Mar 25, 2026

View reviewed changes

karle0wne added 2 commits March 26, 2026 18:54

Simplify serializer recursion tests and helpers

bd980da

review fixes

d810d35

karle0wne commented Mar 26, 2026

View reviewed changes

review fixes

511ee5e

karle0wne requested a review from strug March 26, 2026 17:38

Conversation

karle0wne commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karle0wne Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karle0wne Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karle0wne Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

strug commented Mar 25, 2026

Uh oh!

karle0wne left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karle0wne commented Mar 24, 2026 •

edited

Loading

karle0wne Mar 26, 2026 •

edited

Loading

karle0wne Mar 26, 2026 •

edited

Loading

karle0wne Mar 26, 2026 •

edited

Loading