From f0940d7dc0a4448b384a0cf9428d0d3b599656b6 Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Sat, 17 Feb 2024 11:00:44 +0530 Subject: [PATCH 1/7] Skipping over groovy metadata class + groovy test. --- jr-objects/pom.xml | 9 +++++--- .../jr/ob/impl/BeanPropertyIntrospector.java | 18 +++++++++------- .../fasterxml/jackson/jr/GroovyTest.groovy | 21 +++++++++++++++++++ 3 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy diff --git a/jr-objects/pom.xml b/jr-objects/pom.xml index 08625dc8..c69235cd 100644 --- a/jr-objects/pom.xml +++ b/jr-objects/pom.xml @@ -40,6 +40,12 @@ has no other dependencies, and provides additional builder-style content generat jackson-core ${jackson.version.core} + + org.codehaus.groovy + groovy + 3.0.18 + test + @@ -64,9 +70,6 @@ has no other dependencies, and provides additional builder-style content generat replace generate-sources - diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java index 1eb5815f..b5e602cb 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java @@ -88,7 +88,7 @@ private POJODefinition _construct(Class beanType, int features) private static void _introspect(Class currType, Map props, int features) { - if (currType == null || currType == Object.class) { + if (currType == null || currType == Object.class || isGroovyMetaClass(currType)) { return; } // First, check base type @@ -158,12 +158,7 @@ private static void _introspect(Class currType, Map prop } private static PropBuilder _propFrom(Map props, String name) { - PropBuilder prop = props.get(name); - if (prop == null) { - prop = Prop.builder(name); - props.put(name, prop); - } - return prop; + return props.computeIfAbsent(name, Prop::builder); } private static String decap(String name) { @@ -182,4 +177,13 @@ private static String decap(String name) { return name; } + /** + * Another helper method to deal with Groovy's problematic metadata accessors + * + * @implNote Groovy MetaClass have cyclic reference, and hence the class containing it should not be serialised without + * either removing that reference, or skipping over such references. + */ + protected static boolean isGroovyMetaClass(Class clazz) { + return clazz.getName().startsWith("groovy.lang"); + } } diff --git a/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy b/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy new file mode 100644 index 00000000..16e2e63e --- /dev/null +++ b/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy @@ -0,0 +1,21 @@ +package com.fasterxml.jackson.jr + +import com.fasterxml.jackson.jr.ob.JSON +import com.fasterxml.jackson.jr.ob.TestBase + +class GroovyTest extends TestBase { + + void testSimpleObject() throws Exception { + var data = JSON.std.asString(new MyClass()) + var expected = "{\"aDouble\":0.0,\"aStr\":\"stringData\",\"anInt\":0,\"metaClass\":{}}"; + assertEquals(data, expected) + } + + private class MyClass { + public int anInt; //testing groovy primitive + public String aStr = "stringData"; //testing allocated object + + public double aDouble; // + public Double aDoublesd; //testing boxing object + } +} From b1c4ee20336b4ca56c77d3bcd5519242de8ff25a Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Wed, 21 Feb 2024 00:08:24 +0530 Subject: [PATCH 2/7] adding Groovy check for Method retrival. --- .../jr/ob/impl/BeanPropertyIntrospector.java | 2 +- .../fasterxml/jackson/jr/GroovyTest.groovy | 47 +++++++++++++++++-- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java index b5e602cb..af983b86 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java @@ -117,7 +117,7 @@ private static void _introspect(Class currType, Map prop // 13-Jun-2015, tatu: Skip synthetic, bridge methods altogether, for now // at least (add more complex handling only if absolutely necessary) if (Modifier.isStatic(flags) - || m.isSynthetic() || m.isBridge()) { + || m.isSynthetic() || m.isBridge() || isGroovyMetaClass(m.getReturnType())) { continue; } Class argTypes[] = m.getParameterTypes(); diff --git a/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy b/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy index 16e2e63e..70a6e816 100644 --- a/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy +++ b/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy @@ -7,15 +7,52 @@ class GroovyTest extends TestBase { void testSimpleObject() throws Exception { var data = JSON.std.asString(new MyClass()) - var expected = "{\"aDouble\":0.0,\"aStr\":\"stringData\",\"anInt\":0,\"metaClass\":{}}"; + var expected = """{"AAAAA_A_Field_Starting_With_Two_Capital_Letters":"XYZ","aDouble":0.0,"aPublicInitializedInteger":56,"aPublicInitializedIntegerObject":1516,"aPublicUninitializedInteger":0,"anInitializedIntegerObject":1112,"anInitializedPublicString":"stringData","anInitializedString":"ABC","anInteger":0,"anIntegerWithValue":12}""" assertEquals(data, expected) } private class MyClass { - public int anInt; //testing groovy primitive - public String aStr = "stringData"; //testing allocated object + int anInteger + int anIntegerWithValue = 12 - public double aDouble; // - public Double aDoublesd; //testing boxing object + static int anStaticInteger = 34 + static int anStaticIntegerWithValue = 34 + + public int aPublicUninitializedInteger + public int aPublicInitializedInteger = 56 + + private int aPrivateUninitializedInteger + private int aPrivateInitializedInteger = 78 + + public static int aPublicStaticUninitializedInteger + public static int aPublicStaticInitializedInteger = 910 + + Integer anIntegerObject + Integer anInitializedIntegerObject = 1112 + + static Integer aStaticIntegerObject + static Integer aStaticInitializedIntegerObject = 1314 + + public Integer aPublicUninitializedIntegerObject + public Integer aPublicInitializedIntegerObject = 1516 + + public static Integer aPublicStaticUninitializedIntegerObject + public static Integer aPublicStaticInitializedIntegerObject = 1718 + + String aString + String anInitializedString = "ABC" + + static String aStaticString = "jacksonJR" + + public String aPublicString + public String anInitializedPublicString = "stringData" + + public String AAAAA_A_Field_Starting_With_Two_Capital_Letters = "XYZ" + //Other Items + public static String staticStr = "jacksonJR" // Public Static Object + static int anStaticInt // Uninitialized Static Object + public double aDouble // uninitialized primitive + public Double aDoubleObject // testing boxing object + private int hiddenvalue = 123 // private value } } From 3b11f5b43cf2df2c96f7b2161d25053df97880dc Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 20 Feb 2024 17:15:30 -0800 Subject: [PATCH 3/7] Move groovy test to under `src/test/groovy` --- jr-objects/pom.xml | 2 +- .../{java => groovy}/com/fasterxml/jackson/jr/GroovyTest.groovy | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename jr-objects/src/test/{java => groovy}/com/fasterxml/jackson/jr/GroovyTest.groovy (100%) diff --git a/jr-objects/pom.xml b/jr-objects/pom.xml index c69235cd..9a011684 100644 --- a/jr-objects/pom.xml +++ b/jr-objects/pom.xml @@ -43,7 +43,7 @@ has no other dependencies, and provides additional builder-style content generat org.codehaus.groovy groovy - 3.0.18 + 3.0.20 test diff --git a/jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy b/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy similarity index 100% rename from jr-objects/src/test/java/com/fasterxml/jackson/jr/GroovyTest.groovy rename to jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy From 2ea10d9e362c626b0fa10e09e0d8c9aba4756910 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 20 Feb 2024 17:53:02 -0800 Subject: [PATCH 4/7] Changes to make junit-on-groovy test run --- jr-objects/pom.xml | 17 ++++++++++++++++- .../com/fasterxml/jackson/jr/GroovyTest.groovy | 18 +++++++++++------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/jr-objects/pom.xml b/jr-objects/pom.xml index 9a011684..8bcb63b6 100644 --- a/jr-objects/pom.xml +++ b/jr-objects/pom.xml @@ -55,11 +55,26 @@ has no other dependencies, and provides additional builder-style content generat maven-javadoc-plugin - https://javadoc.io/doc/com.fasterxml.jackson.core/jackson-core/2.13/ + https://javadoc.io/doc/com.fasterxml.jackson.core/jackson-core/2.17/ + + + + org.codehaus.gmaven + gmaven-plugin + 1.5 + + + + testCompile + + + + com.google.code.maven-replacer-plugin replacer diff --git a/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy b/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy index 70a6e816..ff064714 100644 --- a/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy +++ b/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy @@ -1,17 +1,22 @@ package com.fasterxml.jackson.jr +import org.junit.Test +import org.junit.Assert + import com.fasterxml.jackson.jr.ob.JSON import com.fasterxml.jackson.jr.ob.TestBase -class GroovyTest extends TestBase { - +class GroovyTest +{ + @Test void testSimpleObject() throws Exception { - var data = JSON.std.asString(new MyClass()) - var expected = """{"AAAAA_A_Field_Starting_With_Two_Capital_Letters":"XYZ","aDouble":0.0,"aPublicInitializedInteger":56,"aPublicInitializedIntegerObject":1516,"aPublicUninitializedInteger":0,"anInitializedIntegerObject":1112,"anInitializedPublicString":"stringData","anInitializedString":"ABC","anInteger":0,"anIntegerWithValue":12}""" - assertEquals(data, expected) + def json = JSON.std.asString(new GroovyOb()) + def expected = """{"AAAAA_A_Field_Starting_With_Two_Capital_Letters":"XYZ","aDouble":0.0,"aPublicInitializedInteger":56,"aPublicInitializedIntegerObject":1516,"aPublicUninitializedInteger":0,"anInitializedIntegerObject":1112,"anInitializedPublicString":"stringData","anInitializedString":"ABC","anInteger":0,"anIntegerWithValue":12}""" + Assert.assertEquals(json, expected) } +} - private class MyClass { +class GroovyOb { int anInteger int anIntegerWithValue = 12 @@ -54,5 +59,4 @@ class GroovyTest extends TestBase { public double aDouble // uninitialized primitive public Double aDoubleObject // testing boxing object private int hiddenvalue = 123 // private value - } } From b2fb57c45d021af6849c380ef8e356c114618497 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 20 Feb 2024 18:12:36 -0800 Subject: [PATCH 5/7] Merge pom.xml changes from 2.17 --- jr-objects/pom.xml | 41 ----------------------------------------- 1 file changed, 41 deletions(-) diff --git a/jr-objects/pom.xml b/jr-objects/pom.xml index 8bcb63b6..9bd97dac 100644 --- a/jr-objects/pom.xml +++ b/jr-objects/pom.xml @@ -78,47 +78,6 @@ has no other dependencies, and provides additional builder-style content generat com.google.code.maven-replacer-plugin replacer - - - process-packageVersion - - replace - - generate-sources - - - - ${packageVersion.template.input} - ${packageVersion.template.output} - - - @package@ - ${packageVersion.package} - - - @projectversion@ - ${project.version} - - - @projectgroupid@ - ${project.groupId} - - - @projectartifactid@ - ${project.artifactId} - - - - - - org.apache.maven.plugins - maven-surefire-plugin - - ${surefire.redirectTestOutputToFile} - - **/failing/*.java - - From 0566e93f8c5df5b7e0b47795d661de5719237e9e Mon Sep 17 00:00:00 2001 From: Shalnark <65479699+Shounaks@users.noreply.github.com> Date: Wed, 21 Feb 2024 08:19:11 +0530 Subject: [PATCH 6/7] Adding Class Name Condition replicating Jackson-databind. --- .../jackson/jr/ob/impl/BeanPropertyIntrospector.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java index af983b86..3279b978 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java @@ -88,7 +88,7 @@ private POJODefinition _construct(Class beanType, int features) private static void _introspect(Class currType, Map props, int features) { - if (currType == null || currType == Object.class || isGroovyMetaClass(currType)) { + if (currType == null || currType == Object.class) { return; } // First, check base type @@ -116,8 +116,7 @@ private static void _introspect(Class currType, Map prop final int flags = m.getModifiers(); // 13-Jun-2015, tatu: Skip synthetic, bridge methods altogether, for now // at least (add more complex handling only if absolutely necessary) - if (Modifier.isStatic(flags) - || m.isSynthetic() || m.isBridge() || isGroovyMetaClass(m.getReturnType())) { + if (Modifier.isStatic(flags) || m.isSynthetic() || m.isBridge() || isGroovyMetaClass(m.getReturnType())) { continue; } Class argTypes[] = m.getParameterTypes(); @@ -184,6 +183,6 @@ private static String decap(String name) { * either removing that reference, or skipping over such references. */ protected static boolean isGroovyMetaClass(Class clazz) { - return clazz.getName().startsWith("groovy.lang"); + return clazz.getName().startsWith("groovy.lang") && clazz.getSimpleName().equals("MetaClass"); } } From 0f96433667f0e5c8b9cd1f4fcf6e286d67c6299b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 20 Feb 2024 20:28:19 -0800 Subject: [PATCH 7/7] Mix clean up, release notes, update groovy dep to 4.x --- jr-objects/pom.xml | 4 ++-- .../jr/ob/impl/BeanPropertyIntrospector.java | 4 ++-- .../fasterxml/jackson/jr/GroovyTest.groovy | 2 +- .../stree/util/JrsTreeTraversingParser.java | 22 +++++++++++++++++-- release-notes/CREDITS-2.x | 11 +++++++++- release-notes/VERSION-2.x | 3 +++ 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/jr-objects/pom.xml b/jr-objects/pom.xml index 9bd97dac..e966a445 100644 --- a/jr-objects/pom.xml +++ b/jr-objects/pom.xml @@ -41,9 +41,9 @@ has no other dependencies, and provides additional builder-style content generat ${jackson.version.core} - org.codehaus.groovy + org.apache.groovy groovy - 3.0.20 + 4.0.18 test diff --git a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java index 3279b978..375d7098 100644 --- a/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java +++ b/jr-objects/src/main/java/com/fasterxml/jackson/jr/ob/impl/BeanPropertyIntrospector.java @@ -177,12 +177,12 @@ private static String decap(String name) { } /** - * Another helper method to deal with Groovy's problematic metadata accessors + * Helper method to detect Groovy's problematic metadata accessor type. * * @implNote Groovy MetaClass have cyclic reference, and hence the class containing it should not be serialised without * either removing that reference, or skipping over such references. */ protected static boolean isGroovyMetaClass(Class clazz) { - return clazz.getName().startsWith("groovy.lang") && clazz.getSimpleName().equals("MetaClass"); + return "groovy.lang.MetaClass".equals(clazz.getName()); } } diff --git a/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy b/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy index ff064714..0af7b401 100644 --- a/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy +++ b/jr-objects/src/test/groovy/com/fasterxml/jackson/jr/GroovyTest.groovy @@ -9,7 +9,7 @@ import com.fasterxml.jackson.jr.ob.TestBase class GroovyTest { @Test - void testSimpleObject() throws Exception { + void testSimpleGroovyObject() throws Exception { def json = JSON.std.asString(new GroovyOb()) def expected = """{"AAAAA_A_Field_Starting_With_Two_Capital_Letters":"XYZ","aDouble":0.0,"aPublicInitializedInteger":56,"aPublicInitializedIntegerObject":1516,"aPublicUninitializedInteger":0,"anInitializedIntegerObject":1112,"anInitializedPublicString":"stringData","anInitializedString":"ABC","anInteger":0,"anIntegerWithValue":12}""" Assert.assertEquals(json, expected) diff --git a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/util/JrsTreeTraversingParser.java b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/util/JrsTreeTraversingParser.java index b321d556..ab8ee04f 100644 --- a/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/util/JrsTreeTraversingParser.java +++ b/jr-stree/src/main/java/com/fasterxml/jackson/jr/stree/util/JrsTreeTraversingParser.java @@ -193,10 +193,16 @@ public boolean isClosed() { */ @Override - public String getCurrentName() { + public String currentName() { return (_nodeCursor == null) ? null : _nodeCursor.getCurrentName(); } + @Override + @Deprecated // since 2.17 + public String getCurrentName() { + return currentName(); + } + @Override public void overrideCurrentName(String name) { @@ -211,13 +217,25 @@ public JsonStreamContext getParsingContext() { } @Override + public JsonLocation currentTokenLocation() { + return JsonLocation.NA; + } + + @Override + @Deprecated // since 2.17 public JsonLocation getTokenLocation() { + return currentTokenLocation(); + } + + @Override + public JsonLocation currentLocation() { return JsonLocation.NA; } @Override + @Deprecated // since 2.17 public JsonLocation getCurrentLocation() { - return JsonLocation.NA; + return currentLocation(); } /* diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index d6c182ee..64d2bc62 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -23,6 +23,12 @@ Jonas Konrad (@yawkat) * Suggested #88: Make `jr-stree` dependency to `jr-objects` optional (2.13.0) +Nikolay Chashnikov (@chashnikov) + +* Requested #93: Skip serialization of `groovy.lang.MetaClass` + values to avoid `StackOverflowError` + (2.17.0) + Gerben Oolbekkink (@github) * Reported #98: `module-info.java` of `jr-stree` refers to module `com.fasterxml.jackson.jr.ob.api`, @@ -42,5 +48,8 @@ Julian Honnen (@jhonnen) @Shounaks -* Contributed implf ro #100: Add support for `java.time` (Java 8 date/time) types +* Contributed fix for #93: Skip serialization of `groovy.lang.MetaClass` values + to avoid `StackOverflowError` + (2.17.0) +* Contributed impl for #100: Add support for `java.time` (Java 8 date/time) types (2.17.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 59199792..3fd427f5 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -15,6 +15,9 @@ Modules: #78: Deserializes "null" to "0.0" for `java.lang.Double` (wrapper) (reported by @bill-phast) +#93: Skip serialization of `groovy.lang.MetaClass` values to avoid `StackOverflowError` + (requested by Nikolay C) + (fix contributed by @Shounaks) #100: Add support for `java.time` (Java 8 date/time) types (requested by @sebastian-zero) (contributed by @Shounaks)