Skip to content

Commit

Permalink
[JBRULES-3683] prevent StackOverflow when printing an instance of a r…
Browse files Browse the repository at this point in the history 
…ecursive type declaration
  • Loading branch information
@mariofusco
mariofusco committed Nov 8, 2012
1 parent df91b93 commit 5e7b1ce
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 125 deletions.
20 changes: 17 additions & 3 deletions drools-compiler/src/main/java/org/drools/compiler/PackageBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2132,7 +2132,8 @@ List<TypeDefinition> processTypeDeclarations(PackageRegistry pkgRegistry, Packag

generateDeclaredBean( typeDescr,
type,
pkgRegistry );
pkgRegistry,
unresolvedTypeDefinitions );

Class clazz = pkgRegistry.getTypeResolver().resolveType(typeDescr.getType().getFullName());
type.setTypeClass( clazz );
Expand Down Expand Up @@ -2368,7 +2369,8 @@ private void buildFieldAccessors( final TypeDeclaration type,
*/
private void generateDeclaredBean( AbstractClassTypeDeclarationDescr typeDescr,
TypeDeclaration type,
PackageRegistry pkgRegistry ) {
PackageRegistry pkgRegistry,
List<TypeDefinition> unresolvedTypeDefinitions ) {

// extracts type, supertype and interfaces
String fullName = typeDescr.getType().getFullName();
Expand Down Expand Up @@ -2455,6 +2457,14 @@ private void generateDeclaredBean( AbstractClassTypeDeclarationDescr typeDescr,
pkgRegistry );
while (!fieldDefs.isEmpty()) {
FieldDefinition fld = fieldDefs.poll();
if (unresolvedTypeDefinitions != null) {
for (TypeDefinition typeDef : unresolvedTypeDefinitions) {
if (fld.getTypeName().equals(typeDef.getTypeClassName())) {
fld.setRecursive(true);
break;
}
}
}
def.addField( fld );
}
}
Expand Down Expand Up @@ -2699,7 +2709,7 @@ private void generateDeclaredBean( AbstractClassTypeDeclarationDescr typeDescr,
* @return
*/
private PriorityQueue<FieldDefinition> sortFields( Map<String, TypeFieldDescr> flds,
PackageRegistry pkgRegistry ) {
PackageRegistry pkgRegistry ) {
PriorityQueue<FieldDefinition> queue = new PriorityQueue<FieldDefinition>();
int last = 0;

Expand Down Expand Up @@ -3518,6 +3528,10 @@ private TypeDefinition( TypeDeclaration type, AbstractClassTypeDeclarationDescr
this.type = type;
this.typeDescr = typeDescr;
}

public String getTypeClassName() {
return type.getTypeClassName();
}
}

public void registerBuildResource(final Resource resource) {
Expand Down
16 changes: 0 additions & 16 deletions drools-compiler/src/main/java/org/drools/rule/builder/dialect/mvel/MVELExprAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ public MVELExprAnalyzer() {
*
* @param expr
* The expression to analyze.
<<<<<<< HEAD
* @param availableIdentifiers
=======
* @param availDecls
>>>>>>> JBRULES-3414 Segment based unlinking
* Total set of declarations available.
*
* @return The <code>Set</code> of declarations used by the expression.
Expand Down Expand Up @@ -253,18 +249,6 @@ public MVELAnalysisResult analyzeExpression(final PackageBuildContext context,

/**
* Analyse an expression.
<<<<<<< HEAD
*
=======
*
* @param availDecls
* Total set of declarations available.
* @param ast
* The AST for the expression.
*
* @return The <code>Set</code> of declarations used by the expression.
*
>>>>>>> JBRULES-3414 Segment based unlinking
* @throws RecognitionException
* If an error occurs in the parser.
*/
Expand Down
5 changes: 5 additions & 0 deletions drools-compiler/src/test/java/org/drools/integrationtests/KnowledgeBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ public void testCompositeKnowledgeBuilder() throws Exception {
Object b = bType.newInstance();
aType.set( a, "fieldB", b );
bType.set( b, "fieldA", a );

// JBRULES-3683 - check that the recurisive type declaration doesn't cause a StackOverflowError
a.toString();
b.toString();

ksession.insert( a );
ksession.insert( b );

Expand Down
195 changes: 89 additions & 106 deletions drools-core/src/main/java/org/drools/factmodel/DefaultBeanClassBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ public byte[] buildClass( ClassDefinition classDef ) throws IOException,

cw.visitEnd();

byte[] serializedClass = cw.toByteArray();

return serializedClass;
return cw.toByteArray();
}

private void buildSerializationMethods(ClassWriter cw, ClassDefinition classDef) {
Expand Down Expand Up @@ -507,12 +505,11 @@ protected void buildClassHeader(ClassVisitor cw,
*/
protected void buildField(ClassVisitor cw,
FieldDefinition fieldDef) {
FieldVisitor fv;
fv = cw.visitField( Opcodes.ACC_PRIVATE,
fieldDef.getName(),
BuildUtils.getTypeDescriptor( fieldDef.getTypeName() ),
null,
null );
FieldVisitor fv = cw.visitField(Opcodes.ACC_PRIVATE,
fieldDef.getName(),
BuildUtils.getTypeDescriptor(fieldDef.getTypeName()),
null,
null);


buildFieldAnnotations(fieldDef, fv);
Expand Down Expand Up @@ -1293,10 +1290,10 @@ protected void buildToString(ClassVisitor cw,
MethodVisitor mv;
{
mv = cw.visitMethod( Opcodes.ACC_PUBLIC,
"toString",
"()Ljava/lang/String;",
null,
null );
"toString",
"()Ljava/lang/String;",
null,
null );
mv.visitCode();

Label l0 = null;
Expand All @@ -1307,145 +1304,131 @@ protected void buildToString(ClassVisitor cw,

// StringBuilder buf = new StringBuilder();
mv.visitTypeInsn( Opcodes.NEW,
Type.getInternalName( StringBuilder.class ) );
Type.getInternalName( StringBuilder.class ) );
mv.visitInsn( Opcodes.DUP );
mv.visitMethodInsn( Opcodes.INVOKESPECIAL,
Type.getInternalName( StringBuilder.class ),
"<init>",
"()V" );
Type.getInternalName( StringBuilder.class ),
"<init>",
"()V" );
mv.visitVarInsn( Opcodes.ASTORE,
1 );
1 );

// buf.append(this.getClass().getSimpleName())
mv.visitVarInsn( Opcodes.ALOAD,
1 );
1 );
mv.visitVarInsn( Opcodes.ALOAD,
0 );
0 );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
BuildUtils.getInternalType( classDef.getClassName() ),
"getClass",
"()Ljava/lang/Class;" );
BuildUtils.getInternalType( classDef.getClassName() ),
"getClass",
"()Ljava/lang/Class;" );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( Class.class ),
"getSimpleName",
"()Ljava/lang/String;" );
Type.getInternalName( Class.class ),
"getSimpleName",
"()Ljava/lang/String;" );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );

// buf.append("( ");
mv.visitLdcInsn( "( " );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
appendToStringBuilder(mv, "( ");
buildFieldsToString( classDef, mv, false );
appendToStringBuilder(mv, " )");

boolean previous = false;

previous = buildFieldsToString( classDef, mv, previous );

mv.visitLdcInsn( " )" );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"toString",
"()Ljava/lang/String;" );
Type.getInternalName( StringBuilder.class ),
"toString",
"()Ljava/lang/String;" );
mv.visitInsn( Opcodes.ARETURN );

Label lastLabel = null;
if ( this.debug ) {
lastLabel = new Label();
mv.visitLabel( lastLabel );
mv.visitLocalVariable( "this",
BuildUtils.getTypeDescriptor( classDef.getClassName() ),
null,
l0,
lastLabel,
0 );
BuildUtils.getTypeDescriptor( classDef.getClassName() ),
null,
l0,
lastLabel,
0 );
mv.visitLocalVariable( "buf",
Type.getDescriptor( StringBuilder.class ),
null,
l0,
lastLabel,
1 );
Type.getDescriptor( StringBuilder.class ),
null,
l0,
lastLabel,
1 );
}
mv.visitMaxs( 0,
0 );
mv.visitMaxs( 0, 0 );
mv.visitEnd();
}
}

protected boolean buildFieldsToString( ClassDefinition classDef, MethodVisitor mv, boolean previous ) {

boolean first = true;
for ( FieldDefinition field : classDef.getFieldsDefinitions() ) {
previous = buildFieldToString( field, classDef, mv, previous );
buildFieldToString( field, classDef, mv, first );
first = false;
}
return previous;
}

protected boolean buildFieldToString(FieldDefinition field, ClassDefinition classDef, MethodVisitor mv, boolean previous) {
if ( previous ) {
protected void buildFieldToString(FieldDefinition field, ClassDefinition classDef, MethodVisitor mv, boolean first) {
if ( !first ) {
// buf.append(", ");
mv.visitLdcInsn( ", " );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
appendToStringBuilder(mv, ", ");
}

// buf.append(attrName)
mv.visitLdcInsn( field.getName() );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
appendToStringBuilder(mv, field.getName());

// buf.append("=");
mv.visitLdcInsn( "=" );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
appendToStringBuilder(mv, "=");

// buf.append(attrValue)
mv.visitVarInsn(Opcodes.ALOAD,
0);

visitFieldOrGetter( mv, classDef, field );
if (field.isRecursive()) {
appendToStringBuilder(mv, field.getTypeName() + " [recursive]");
} else {
mv.visitVarInsn(Opcodes.ALOAD,
0);

visitFieldOrGetter( mv, classDef, field );

if ( BuildUtils.isPrimitive(field.getTypeName()) ) {
String type = field.getTypeName().matches( "(byte|short)" ) ? "int" : field.getTypeName();
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
Type.getMethodDescriptor( Type.getType( StringBuilder.class ),
new Type[]{Type.getType( BuildUtils.getTypeDescriptor( type ) )} ) );
} else if ( BuildUtils.isArray( field.getTypeName() ) && BuildUtils.arrayDimSize( field.getTypeName() ) == 1 ) {
if ( BuildUtils.isPrimitive(field.getTypeName()) ) {
String type = field.getTypeName().matches( "(byte|short)" ) ? "int" : field.getTypeName();
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
Type.getMethodDescriptor( Type.getType( StringBuilder.class ),
new Type[]{Type.getType( BuildUtils.getTypeDescriptor( type ) )} ) );
} else if ( BuildUtils.isArray( field.getTypeName() ) && BuildUtils.arrayDimSize( field.getTypeName() ) == 1 ) {


mv.visitMethodInsn( INVOKESTATIC,
"java/util/Arrays",
"toString",
"(" + BuildUtils.getTypeDescriptor( BuildUtils.arrayType( field.getTypeName() ) ) + ")Ljava/lang/String;" );
mv.visitMethodInsn( INVOKESTATIC,
"java/util/Arrays",
"toString",
"(" + BuildUtils.getTypeDescriptor( BuildUtils.arrayType( field.getTypeName() ) ) + ")Ljava/lang/String;" );

mv.visitMethodInsn( INVOKEVIRTUAL,
"java/lang/StringBuilder",
"append",
"(Ljava/lang/Object;)Ljava/lang/StringBuilder;" );
mv.visitMethodInsn( INVOKEVIRTUAL,
"java/lang/StringBuilder",
"append",
"(Ljava/lang/Object;)Ljava/lang/StringBuilder;" );

} else {
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
Type.getMethodDescriptor( Type.getType( StringBuilder.class ),
new Type[]{Type.getType( Object.class )} ) );
} else {
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName( StringBuilder.class ),
"append",
Type.getMethodDescriptor( Type.getType( StringBuilder.class ),
new Type[]{Type.getType( Object.class )} ) );
}
}
previous = true;
return previous;
}

private void appendToStringBuilder(MethodVisitor mv, String s) {
mv.visitLdcInsn( s );
mv.visitMethodInsn( Opcodes.INVOKEVIRTUAL,
Type.getInternalName(StringBuilder.class),
"append",
"(Ljava/lang/String;)Ljava/lang/StringBuilder;" );
}


Expand Down
9 changes: 9 additions & 0 deletions drools-core/src/main/java/org/drools/factmodel/FieldDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class FieldDefinition
private boolean inherited = false;
private int index = -1;
private String initExpr = null;
private boolean recursive = false;

private List<AnnotationDefinition> annotations;

Expand Down Expand Up @@ -464,4 +465,12 @@ public boolean hasAlias() {
}
return false;
}

public boolean isRecursive() {
return recursive;
}

public void setRecursive(boolean recursive) {
this.recursive = recursive;
}
}

0 comments on commit 5e7b1ce

Please sign in to comment.