HV-1091 Using iterative instead of recursive approach for message par…

…sing
hibernate · Sep 1, 2016 · 7204953 · 7204953
1 parent c63c057
commit 7204953
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 67 deletions.
diff --git a/.../java/org/hibernate/validator/internal/engine/messageinterpolation/parser/BeginState.java b/.../java/org/hibernate/validator/internal/engine/messageinterpolation/parser/BeginState.java
@@ -18,19 +18,14 @@ public class BeginState implements ParserState {
 
 	@Override
 	public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
-	}
-
-	@Override
-	public void start(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
-		tokenCollector.next();
+		tokenCollector.terminateToken();
 	}
 
 	@Override
 	public void handleNonMetaCharacter(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
 		tokenCollector.transitionState( new MessageState() );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -43,7 +38,6 @@ public void handleBeginTerm(char character, TokenCollector tokenCollector) throw
 			tokenCollector.makeParameterToken();
 		}
 		tokenCollector.transitionState( new InterpolationTermState() );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -56,7 +50,6 @@ public void handleEscapeCharacter(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
 		tokenCollector.transitionState( new EscapedState( this ) );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -68,9 +61,6 @@ public void handleELDesignator(char character, TokenCollector tokenCollector)
 		else {
 			ParserState state = new ELState();
 			tokenCollector.transitionState( state );
-			tokenCollector.next();
 		}
 	}
 }
-
-
diff --git a/...ain/java/org/hibernate/validator/internal/engine/messageinterpolation/parser/ELState.java b/...ain/java/org/hibernate/validator/internal/engine/messageinterpolation/parser/ELState.java
@@ -15,11 +15,6 @@
 public class ELState implements ParserState {
 	private static final Log log = LoggerFactory.make();
 
-	@Override
-	public void start(TokenCollector tokenCollector) {
-		throw new IllegalStateException( "Parsing of message descriptor cannot start in this state" );
-	}
-
 	@Override
 	public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( TokenCollector.EL_DESIGNATOR );
@@ -33,7 +28,6 @@ public void handleNonMetaCharacter(char character, TokenCollector tokenCollector
 		tokenCollector.appendToToken( character );
 		tokenCollector.terminateToken();
 		tokenCollector.transitionState( new BeginState() );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -44,7 +38,6 @@ public void handleBeginTerm(char character, TokenCollector tokenCollector) throw
 		tokenCollector.appendToToken( character );
 		tokenCollector.makeELToken();
 		tokenCollector.transitionState( new InterpolationTermState() );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -59,7 +52,6 @@ public void handleEndTerm(char character, TokenCollector tokenCollector) throws
 	public void handleEscapeCharacter(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.transitionState( new EscapedState( this ) );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -68,5 +60,3 @@ public void handleELDesignator(char character, TokenCollector tokenCollector)
 		handleNonMetaCharacter( character, tokenCollector );
 	}
 }
-
-
diff --git a/...ava/org/hibernate/validator/internal/engine/messageinterpolation/parser/EscapedState.java b/...ava/org/hibernate/validator/internal/engine/messageinterpolation/parser/EscapedState.java
@@ -16,11 +16,6 @@ public EscapedState(ParserState previousState) {
 		this.previousState = previousState;
 	}
 
-	@Override
-	public void start(TokenCollector tokenCollector) {
-		throw new IllegalStateException( "Parsing of message descriptor cannot start in this state" );
-	}
-
 	@Override
 	public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
 		tokenCollector.terminateToken();
@@ -58,9 +53,5 @@ private void handleEscapedCharacter(char character, TokenCollector tokenCollecto
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
 		tokenCollector.transitionState( previousState );
-		tokenCollector.next();
 	}
 }
-
-
-
diff --git a/...bernate/validator/internal/engine/messageinterpolation/parser/InterpolationTermState.java b/...bernate/validator/internal/engine/messageinterpolation/parser/InterpolationTermState.java
@@ -15,11 +15,6 @@
 public class InterpolationTermState implements ParserState {
 	private static final Log log = LoggerFactory.make();
 
-	@Override
-	public void start(TokenCollector tokenCollector) {
-		throw new IllegalStateException( "Parsing of message descriptor cannot start in this state" );
-	}
-
 	@Override
 	public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
 		throw log.getNonTerminatedParameterException(
@@ -32,7 +27,6 @@ public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFor
 	public void handleNonMetaCharacter(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -46,7 +40,6 @@ public void handleEndTerm(char character, TokenCollector tokenCollector) throws
 		tokenCollector.terminateToken();
 		BeginState beginState = new BeginState();
 		tokenCollector.transitionState( beginState );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -55,16 +48,11 @@ public void handleEscapeCharacter(char character, TokenCollector tokenCollector)
 		tokenCollector.appendToToken( character );
 		ParserState state = new EscapedState( this );
 		tokenCollector.transitionState( state );
-		tokenCollector.next();
-
 	}
 
 	@Override
 	public void handleELDesignator(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
-		tokenCollector.next();
 	}
 }
-
-
diff --git a/...ava/org/hibernate/validator/internal/engine/messageinterpolation/parser/MessageState.java b/...ava/org/hibernate/validator/internal/engine/messageinterpolation/parser/MessageState.java
@@ -16,11 +16,6 @@
 public class MessageState implements ParserState {
 	private static final Log log = LoggerFactory.make();
 
-	@Override
-	public void start(TokenCollector tokenCollector) {
-		throw new IllegalStateException( "The parsing of the message descriptor cannot start in this state." );
-	}
-
 	@Override
 	public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException {
 		tokenCollector.terminateToken();
@@ -30,7 +25,6 @@ public void terminate(TokenCollector tokenCollector) throws MessageDescriptorFor
 	public void handleNonMetaCharacter(char character, TokenCollector tokenCollector)
 			throws MessageDescriptorFormatException {
 		tokenCollector.appendToToken( character );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -42,7 +36,6 @@ public void handleBeginTerm(char character, TokenCollector tokenCollector) throw
 			tokenCollector.makeParameterToken();
 		}
 		tokenCollector.transitionState( new InterpolationTermState() );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -59,7 +52,6 @@ public void handleEscapeCharacter(char character, TokenCollector tokenCollector)
 		tokenCollector.appendToToken( character );
 
 		tokenCollector.transitionState( new EscapedState( this ) );
-		tokenCollector.next();
 	}
 
 	@Override
@@ -70,9 +62,6 @@ public void handleELDesignator(char character, TokenCollector tokenCollector)
 		}
 		else {
 			tokenCollector.transitionState( new ELState() );
-			tokenCollector.next();
 		}
 	}
 }
-
-
diff --git a/...java/org/hibernate/validator/internal/engine/messageinterpolation/parser/ParserState.java b/...java/org/hibernate/validator/internal/engine/messageinterpolation/parser/ParserState.java
@@ -13,7 +13,6 @@
  * @author Hardy Ferentschik
  */
 public interface ParserState {
-	void start(TokenCollector tokenCollector) throws MessageDescriptorFormatException;
 
 	void terminate(TokenCollector tokenCollector) throws MessageDescriptorFormatException;
 
@@ -27,4 +26,3 @@ public interface ParserState {
 
 	void handleELDesignator(char character, TokenCollector tokenCollector) throws MessageDescriptorFormatException;
 }
-
diff --git a/...a/org/hibernate/validator/internal/engine/messageinterpolation/parser/TokenCollector.java b/...a/org/hibernate/validator/internal/engine/messageinterpolation/parser/TokenCollector.java
@@ -6,13 +6,13 @@
  */
 package org.hibernate.validator.internal.engine.messageinterpolation.parser;
 
+import static org.hibernate.validator.internal.util.CollectionHelper.newArrayList;
+
 import java.util.Collections;
 import java.util.List;
 
 import org.hibernate.validator.internal.engine.messageinterpolation.InterpolationTermType;
 
-import static org.hibernate.validator.internal.util.CollectionHelper.newArrayList;
-
 /**
  * Used to creates a list of tokens from a message descriptor.
  *
@@ -28,7 +28,7 @@ public class TokenCollector {
 	private final String originalMessageDescriptor;
 	private final InterpolationTermType interpolationTermType;
 
-	private List<Token> tokenList;
+	private final List<Token> tokenList;
 	private ParserState currentParserState;
 	private int currentPosition;
 	private Token currentToken;
@@ -69,10 +69,11 @@ public void makeELToken() {
 		currentToken.makeELToken();
 	}
 
-	public void next() throws MessageDescriptorFormatException {
+	private void next() throws MessageDescriptorFormatException {
 		if ( currentPosition == originalMessageDescriptor.length() ) {
 			// give the current context the chance to complete
 			currentParserState.terminate( this );
+			currentPosition++;
 			return;
 		}
 		char currentCharacter = originalMessageDescriptor.charAt( currentPosition );
@@ -98,12 +99,12 @@ public void next() throws MessageDescriptorFormatException {
 				currentParserState.handleNonMetaCharacter( currentCharacter, this );
 			}
 		}
-		// make sure the last token is terminated
-		terminateToken();
 	}
 
 	public final void parse() throws MessageDescriptorFormatException {
-		currentParserState.start( this );
+		while ( currentPosition <= originalMessageDescriptor.length() ) {
+			next();
+		}
 	}
 
 	public void transitionState(ParserState newState) {
@@ -122,4 +123,3 @@ public String getOriginalMessageDescriptor() {
 		return originalMessageDescriptor;
 	}
 }
-
diff --git a/engine/src/test/java/org/hibernate/validator/bugs/TooBigMessageTest.java b/engine/src/test/java/org/hibernate/validator/bugs/TooBigMessageTest.java
@@ -0,0 +1,90 @@
+/*
+ * Hibernate Validator, declare and validate application constraints
+ *
+ * License: Apache License, Version 2.0
+ * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
+ */
+package org.hibernate.validator.bugs;
+
+import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertCorrectConstraintViolationMessages;
+import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertNumberOfViolations;
+
+import java.util.Set;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.constraints.NotNull;
+
+import org.hibernate.validator.testutil.TestForIssue;
+import org.hibernate.validator.testutils.ValidatorUtil;
+import org.junit.Test;
+
+/**
+ * Ensure large error messages can be interpolated.
+ *
+ * @author Gunnar Morling
+ */
+public class TooBigMessageTest {
+
+	/**
+	 * Large enough to trigger a stack overflow with the recursive scheme, assuming default settings
+	 */
+	private static final String LARGE_MESSAGE =
+			"12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890"
+			+ "12345678901234567890123456789012345678901234567890123456789012345678901234567890";
+
+	@Test
+	@TestForIssue(jiraKey = "HV-1091")
+	public void largeMessageCanBeInterpolated() {
+		Validator validator = ValidatorUtil.getValidator();
+		GoldFish fish = new GoldFish();
+
+		Set<ConstraintViolation<GoldFish>> constraintViolations = validator.validate( fish );
+		assertNumberOfViolations( constraintViolations, 1 );
+		assertCorrectConstraintViolationMessages( constraintViolations, LARGE_MESSAGE );
+	}
+
+	private static class GoldFish {
+
+		@NotNull(message = LARGE_MESSAGE)
+		String name;
+	}
+}
diff --git a/.../org/hibernate/validator/test/internal/engine/messageinterpolation/TokenIteratorTest.java b/.../org/hibernate/validator/test/internal/engine/messageinterpolation/TokenIteratorTest.java
@@ -6,15 +6,15 @@
  */
 package org.hibernate.validator.test.internal.engine.messageinterpolation;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
 import org.hibernate.validator.internal.engine.messageinterpolation.InterpolationTermType;
 import org.hibernate.validator.internal.engine.messageinterpolation.parser.TokenCollector;
 import org.hibernate.validator.internal.engine.messageinterpolation.parser.TokenIterator;
 import org.testng.annotations.Test;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertFalse;
-import static org.testng.Assert.assertTrue;
-
 /**
  * Tests for {@code TokenIterator}.
  *