LDEV-4866 fix query parser out of bounds

core/src/main/java/lucee/commons/lang/ParserString.java

@@ -57,7 +57,17 @@ public final class ParserString {
 	 * @param text CFML Code
 	 */
 	public ParserString(String text) {
-		init(text);
+		init(text, false);
+	}
+
+	/**
+	 * This constructor allows stripping comments from SQL Text
+	 * 
+	 * @param text SQL Text
+	 * @param doIgnoreComments strip sql comments from text
+	 */
+	public ParserString(String text, boolean doIgnoreComments) {
+		init(text, doIgnoreComments);
 	}
 
 	/**
@@ -66,7 +76,8 @@ public ParserString(String text) {
 	 * 
 	 * @param str
 	 */
-	protected void init(String str) {
+	protected void init(String str, boolean doIgnoreComments) {
+		if (doIgnoreComments) str = stripSqlComments(str);
 		int len = str.length();
 		text = new char[len];
 		lcText = new char[len];
@@ -700,6 +711,42 @@ public boolean removeSpace() {
 		}
 		return (start < pos);
 	}
+	/**
+	 * Strip out all sql comments
+	 * 
+	 * @return SQL text with comments stripped out
+	 */
+	public String stripSqlComments(String sql) {
+		char c;
+		int sqlLen = sql.length();
+		StringBuilder sb = new StringBuilder();
+
+		for (int i = 0; i < sqlLen; i++) {
+			c = sql.charAt(i);
+			if ( i < (sqlLen - 1)) {
+				// handle multi line comment
+				if (c == '/' && sql.charAt(i + 1) == '*') {
+					int end = sql.indexOf("*/", i + 2);
+					if (end != -1) {
+						i = end + 1;
+						continue;
+					}
+				}
+
+				// handle single line comment
+				if (c == '-' && i < (sqlLen - 1) && sql.charAt(i + 1) == '-') {
+					int end = sql.indexOf('\n', i + 1);
+					if (end == -1) {
+						break; // end of sql string
+					}
+					i = end;
+					continue;
+				}
+			}
+			sb.append(c);
+		}
+		return sb.toString().trim();
+	}
 
 	public void revertRemoveSpace() {
 		while (hasSpaceBefore()) {

core/src/main/java/lucee/runtime/db/HSQLDBHandler.java

@@ -267,7 +267,8 @@ public QueryImpl execute(PageContext pc, final SQL sql, int maxrows, int fetchsi
 			if (spe.getCause() != null && spe.getCause() instanceof IllegalQoQException) {
 				throw Caster.toPageException(spe);
 			}
-			prettySQL = SQLPrettyfier.prettyfie(sql.getSQLString());
+			prettySQL = SQLPrettyfier.prettyfie(sql.getSQLString(), true);
+
 			try {
 				QueryImpl query = executer.execute(pc, sql, prettySQL, maxrows);
 				query.setExecutionTime(stopwatch.time());
@@ -326,7 +327,7 @@ public QueryImpl execute(PageContext pc, final SQL sql, int maxrows, int fetchsi
 				tables = hsql2.getInvokedTables();
 			}
 			else {
-				if (prettySQL == null) prettySQL = SQLPrettyfier.prettyfie(sql.getSQLString());
+				if (prettySQL == null) prettySQL = SQLPrettyfier.prettyfie(sql.getSQLString(),true);
 				HSQLUtil hsql = new HSQLUtil(prettySQL);
 				tables = hsql.getInvokedTables();
 				isUnion = hsql.isUnion();
@@ -339,7 +340,8 @@ public QueryImpl execute(PageContext pc, final SQL sql, int maxrows, int fetchsi
 
 		}
 		catch (ParseException e) {
-			throw new DatabaseException(e.getMessage(), null, sql, null);
+			throw (IllegalQoQException) (new IllegalQoQException("QoQ: error executing sql statement on query, " + e.getMessage(), null, sql, null).initCause(e));
+			//throw new DatabaseException(e.getMessage(), e, sql, null);
 		}
 
 	}

core/src/main/java/lucee/runtime/db/SQLImpl.java

@@ -106,7 +106,7 @@ public String toString() {
 		int index = 0;
 		for (int i = 0; i < sqlLen; i++) {
 			c = strSQL.charAt(i);
-			if (!inQuotes && sqlLen + 1 > i) {
+			if (!inQuotes && i < (sqlLen - 1)) {
 				// read multi line
 				if (c == '/' && strSQL.charAt(i + 1) == '*') {
 					int end = strSQL.indexOf("*/", i + 2);
@@ -118,14 +118,16 @@ public String toString() {
 				}
 
 				// read single line
-				if (c == '-' && strSQL.charAt(i + 1) == '-') {
+				if (c == '-' && i < (sqlLen - 1) && strSQL.charAt(i + 1) == '-') {
 					int end = strSQL.indexOf('\n', i + 1);
-					if (end != -1) {
-						i = end + 1;
-						if (i == sqlLen) break;
-						c = strSQL.charAt(i);
-					}
-					else break;
+					if (end == -1) {
+						i = sqlLen; // end of sql string
+					} else {
+						i = end;
+					} 
+					if (i == sqlLen) break;
+					//c = strSQL.charAt(i);
+					continue;
 				}
 			}
 
@@ -142,7 +144,8 @@ public String toString() {
 				sb.append(c);
 			}
 			else if (!inQuotes && c == '?') {
-				if ((index + 1) > items.length) throw new RuntimeException("there are more question marks in the SQL than params defined, in the SQL String: [" + strSQL + "]");
+				if ((index + 1) > items.length) throw new RuntimeException("There are more question marks [" + (index+1)
+					+ "] in the SQL than params defined [" + items.length + "], in the SQL String: [" + strSQL + "]");
 				if (items[index].isNulls()) sb.append("null");
 				else sb.append(SQLCaster.toString(items[index]));
 				index++;

core/src/main/java/lucee/runtime/db/SQLPrettyfier.java

@@ -39,7 +39,7 @@ public static String prettyfie(String sql) {
 
 	public static String prettyfie(String sql, boolean validZql) {
 
-		ParserString ps = new ParserString(sql.trim());
+		ParserString ps = new ParserString(sql.trim(),true);
 		boolean insideString = false;
 		// short insideKlammer=0;
 		StringBuilder sb = new StringBuilder(sql.length());

core/src/main/java/lucee/runtime/sql/SelectParser.java

@@ -62,7 +62,7 @@ public class SelectParser {
 	// select <select-statement> from <tables> where <where-statement>
 	public Selects parse(String sql) throws SQLParserException {
 		columnIndex = 0;
-		ParserString raw = new ParserString(sql.trim());
+		ParserString raw = new ParserString(sql.trim(),true);
 		Selects selects = new Selects();
 		Select select = new Select();
 
@@ -160,7 +160,7 @@ public Selects parse(String sql) throws SQLParserException {
 
 		if (raw.forwardIfCurrent(';')) raw.removeSpace();
 
-		if (!raw.isAfterLast()) throw new SQLParserException("can not read the full sql statement (stop at:" + raw.getCurrent() + ")");
+		if (!raw.isAfterLast()) throw new SQLParserException("Error parsing SQL statement (stop at char:" + raw.getCurrent() + ", pos: " + raw.getPos() + "), sql: [" + raw.toString() + "]");
 		return selects;
 	}
 

core/src/main/java/lucee/runtime/tag/util/QueryParamConverter.java

@@ -146,26 +146,27 @@ private static SQL convert(String sql, List<SQLItems<SQLItem>> items, List<SQLIt
 
 		for (int i = 0; i < sqlLen; i++) {
 			c = sql.charAt(i);
-			if (!inQuotes && sqlLen + 1 > i) {
+			if (!inQuotes && i < (sqlLen - 1)) {
 				// read multi line
 				if (c == '/' && sql.charAt(i + 1) == '*') {
 					int end = sql.indexOf("*/", i + 2);
 					if (end != -1) {
+						sb.append(sql.substring(i, end+2));
 						i = end + 2;
 						if (i == sqlLen) break;
 						c = sql.charAt(i);
 					}
 				}
 
 				// read single line
-				if (c == '-' && sql.charAt(i + 1) == '-') {
+				if (c == '-' && i < (sqlLen - 1) && sql.charAt(i + 1) == '-') {
 					int end = sql.indexOf('\n', i + 1);
-					if (end != -1) {
-						i = end + 1;
-						if (i == sqlLen) break;
-						c = sql.charAt(i);
-					}
-					else break;
+					if (end == -1) {
+						end = sqlLen-1; // end of sql string
+					} 
+					sb.append(sql.substring(i, end+1));
+					i = end;
+					continue;
 				}
 			}
 
@@ -190,7 +191,7 @@ else if (!inQuotes) {
 						continue;
 					}
 
-					if (++_qm > initialParamSize) throw new ApplicationException("there are more question marks in the SQL than params defined", "SQL: " + sql + "");
+					if (++_qm > initialParamSize) throw new ApplicationException("There are more question marks ["+(qm+1)+"] in the SQL than params defined ["+initialParamSize+"], at position ["+ i +"]", "SQL: " + sql + ", ParsedSQL:" + sb.toString());
 				}
 				else if (c == ':') {
 

test/_testRunner.cfc

@@ -223,14 +223,17 @@ component {
 									}
 									*/
 								}
+								if ( !isNull( specStat.error.sql ) && !isEmpty( trim(specStat.error.sql) ) ){
+									systemOutput( TAB & "SQL: [" & specStat.error.sql & "]", true );
+								}
 								if ( !isNull( specStat.error.StackTrace ) && !isEmpty( specStat.error.StackTrace ) ){
 									systemOutput( TAB & specStat.error.type, true );
 									// printStackTrace( specStat.error.StackTrace );
 									systemOutput( TAB & specStat.error.StackTrace, true );
 									systemOutput( NL );
 								}
 
-							//	systemOutput(NL & serialize(specStat.error), true);
+								//systemOutput(NL & serializeJson(specStat.error), true);
 
 							} // if !isNull
 						}

test/tickets/LDEV1508.cfc

@@ -1,4 +1,4 @@
-component extends="org.lucee.cfml.test.LuceeTestCase" labels="mysql" {
+component extends="org.lucee.cfml.test.LuceeTestCase" labels="mysql,qoq" {
 	// skip closure
 	function isNotSupported() {
 		var mySql = getCredentials();

test/tickets/LDEV2754/test.cfm

@@ -33,9 +33,8 @@
 			params = {
 				id : { value : 2, type : "integer" }
 			},
-			queryoptions = { dbtype = "query" }
+			queryoptions = { dbtype = "query", name="usingQOQ" }
 		);
 		writeOutput(usingQOQ.name);
 	}
-
 </cfscript>

test/tickets/LDEV4866.cfc

@@ -0,0 +1,41 @@
+component extends="org.lucee.cfml.test.LuceeTestCase" labels="query" {
+
+	variables.ds = server.getDatasource( service="h2", dbFile=server._getTempDir( "LDEV4866" ) );
+	variables.params = { a:1, b:2 };
+
+	function run( testResults , testBox ) {
+		describe( title='LDEV-4866' , body=function(){
+			it( title='test query parsing, /* */-' , body=function() {
+				doQuery("#chr(13)# /* */- ");
+				doQuery("#chr(13)# /* */- ");
+				doQuery("/* */-");
+			});
+			it( title='test query parsing, just a - whitespace' , body=function() {
+				doQuery("#chr(9)# - #chr(13)# ");
+			});
+
+			it( title='test query parsing, just a -' , body=function() {
+				doQuery("-");
+			});
+
+			it( title='test query parsing, just a / whitespace' , body=function() {
+				doQuery("#chr(9)# / #chr(13)# ");
+			});
+			it( title='test query parsing, just a /' , body=function() {
+				doQuery("/");
+			});
+		});
+	}
+
+	private function doQuery(sql){
+		try {
+			query name="test" datasource="#ds#" params="#params#" {
+				echo( sql );
+			}
+		} catch (e) {
+			 if ( e.stackTrace.indexOf("lucee.runtime.exp.DatabaseException:") neq 0 )
+				rethrow;
+		}
+	}
+
+}