diff --git a/core/src/main/java/lucee/commons/lang/ParserString.java b/core/src/main/java/lucee/commons/lang/ParserString.java index fdec8097c5..85cf0a4035 100644 --- a/core/src/main/java/lucee/commons/lang/ParserString.java +++ b/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()) { diff --git a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java b/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java index e422d06161..3938e70d23 100644 --- a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java +++ b/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); } } diff --git a/core/src/main/java/lucee/runtime/db/SQLImpl.java b/core/src/main/java/lucee/runtime/db/SQLImpl.java index 173e55648d..c59178c99a 100644 --- a/core/src/main/java/lucee/runtime/db/SQLImpl.java +++ b/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++; diff --git a/core/src/main/java/lucee/runtime/db/SQLPrettyfier.java b/core/src/main/java/lucee/runtime/db/SQLPrettyfier.java index 4ccc49c9cf..cb317ce5d5 100644 --- a/core/src/main/java/lucee/runtime/db/SQLPrettyfier.java +++ b/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()); diff --git a/core/src/main/java/lucee/runtime/sql/SelectParser.java b/core/src/main/java/lucee/runtime/sql/SelectParser.java index 1e04e5be2f..6902427bd1 100644 --- a/core/src/main/java/lucee/runtime/sql/SelectParser.java +++ b/core/src/main/java/lucee/runtime/sql/SelectParser.java @@ -62,7 +62,7 @@ public class SelectParser { // select from where 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; } diff --git a/core/src/main/java/lucee/runtime/tag/util/QueryParamConverter.java b/core/src/main/java/lucee/runtime/tag/util/QueryParamConverter.java index 045e953813..54a082e609 100644 --- a/core/src/main/java/lucee/runtime/tag/util/QueryParamConverter.java +++ b/core/src/main/java/lucee/runtime/tag/util/QueryParamConverter.java @@ -146,11 +146,12 @@ private static SQL convert(String sql, List> items, List 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); @@ -158,14 +159,14 @@ private static SQL convert(String sql, List> items, List 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 == ':') { diff --git a/test/_testRunner.cfc b/test/_testRunner.cfc index c6db5d3e2e..2b420b8ee7 100644 --- a/test/_testRunner.cfc +++ b/test/_testRunner.cfc @@ -223,6 +223,9 @@ 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 ); @@ -230,7 +233,7 @@ component { systemOutput( NL ); } - // systemOutput(NL & serialize(specStat.error), true); + //systemOutput(NL & serializeJson(specStat.error), true); } // if !isNull } diff --git a/test/tickets/LDEV1508.cfc b/test/tickets/LDEV1508.cfc index fb824e8f44..e8a862d62d 100644 --- a/test/tickets/LDEV1508.cfc +++ b/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(); diff --git a/test/tickets/LDEV2754/test.cfm b/test/tickets/LDEV2754/test.cfm index 61c5f3efab..3f10a5ed7f 100644 --- a/test/tickets/LDEV2754/test.cfm +++ b/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); } - \ No newline at end of file diff --git a/test/tickets/LDEV4866.cfc b/test/tickets/LDEV4866.cfc new file mode 100644 index 0000000000..a34b4b6ae0 --- /dev/null +++ b/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; + } + } + +} \ No newline at end of file diff --git a/test/tickets/LDEV4867.cfc b/test/tickets/LDEV4867.cfc new file mode 100644 index 0000000000..80432c8532 --- /dev/null +++ b/test/tickets/LDEV4867.cfc @@ -0,0 +1,176 @@ +component extends="org.lucee.cfml.test.LuceeTestCase" labels="query" { + + function beforeAll(){ + variables.ds = server.getDatasource( service="h2", dbFile=server._getTempDir( "LDEV4867" ) ); + variables.params = { + id: { + value: 1, sqltype="numeric" + } + }; + + variables.LF = chr( 10 ); + variables.qry = QueryNew( "engine,id", "varchar,numeric", [ + [ "lucee", 1 ], + [ "ralio" , 2 ] + ]); + _setupDatabase(); + }; + + function run( testResults , testBox ) { + describe( title='LDEV-4867', body=function(){ + + it( title='test query parsing, removing comments', body=function() { + doTest( ["-- foo", "/* bar */", "SELECT engine from qry where id = :id "] + ,[ "-- foo" , "/* bar */" ] + ); + }); + + it( title='test query parsing, mixed nested comments', body=function() { + doTest( ["/* bar -- foo */", "SELECT engine from qry where id = :id ", "/* bar -- foo */"] + ,[ "/* bar -- foo */" ] + ); + doTest( ["--foo /* bar */", "SELECT engine from qry where id = :id ", "--foo /* bar */"] + ,[ "--foo /* bar */" ] + ); + }); + + it( title='test query parsing, nested comment blocks', skip=true, body=function() { + // Nested comments aren't generally supported anyway.... + doTest( ["/* bar /* #LF# -- foo #LF# */ */", "SELECT engine from qry where id = :id "] + ,[ "/* bar /* #LF# -- foo #LF# */ */" ] + ); + }); + + it( title='test query parsing, with a ? in a comment', body=function() { + doTest( [ "-- foo", "/* bar? */", "SELECT engine"," from qry", "where id = :id" ] + ,[ "-- foo", "/* bar? */" ] + ); + }); + + it( title='test query parsing, with a ? in a /* */ comment', body=function() { + doTest( [ "-- foo", "/* bar? */", "SELECT engine"," from qry", "where id = :id" ], + [ "-- foo", "/* bar? */" ] ); + }); + + it( title='test query parsing, with a ? and : in a comment', body=function() { + doTest( [ "-- foo ? :do", "/* bar? :*/", "SELECT engine"," from qry", "where id = :id" ], + [ "-- foo ? :do", "/* bar? :*/" ] + ); + }); + + it( title='test query parsing, with a ? in a comment', body=function() { + doTest( [ "-- foo ? :do", "/* bar? :*/", "SELECT engine"," from qry", "where id = :id" ], + [ "-- foo ? :do", "/* bar? :*/" ] + ); + }); + + it( title='test query parsing, with a ? in a trailing line comment', body=function() { + doTest( [ "SELECT engine"," from qry", "where id = :id", "-- foo ? :do" ], + [ "-- foo ? :do" ] + ); + }); + + it( title='test query parsing, with a ? in a trailing comment block', body=function() { + doTest( [ "SELECT engine"," from qry", "where id = :id", "/* foo ? :do */" ], + [ "/* foo ? :do */" ] + ); + }); + }); + } + + private function doTest( array sql, array comments ){ + var newlines = ArrayToList( arguments.sql, chr( 10 ) ); + executeTest( newlines, comments, "[LF]" ); + + var paddedNewlines = ArrayToList( arguments.sql, " " & chr( 10 ) & " " ); + executeTest( paddedNewlines, comments, "[ LF ]" ); + + var leading = ArrayToList( arguments.sql, " " & chr( 10 ) ); + executeTest( leading, comments, "[ LF]" ); + + var trailing = ArrayToList( arguments.sql, chr( 10 ) & " " ); + executeTest( trailing, comments, "[LF ]" ); + + var emptySqlComment = ArrayToList( arguments.sql, "--" & chr( 10 ) & " " ); + executeTest( emptySqlComment, comments, "[--LF ]" ); + + var emptySqlCommentParam = ArrayToList( arguments.sql, "--?" & chr( 10 ) & " " ); + executeTest( emptySqlCommentParam, comments, "[--?LF ]" ); + + var emptySqlCommentParam2 = ArrayToList( arguments.sql, "--:foo" & chr( 10 ) & " " ); + executeTest( emptySqlCommentParam2, comments, "[--:fooLF ]" ); + } + + private function executeTest( string sql, array comments, string seperatorWhitespace ){ + _executeTest( arguments.sql, arguments.comments, + arguments.seperatorWhitespace & ", no extra" ); + _executeTest( " " & chr( 10 ) & arguments.sql, arguments.comments, + arguments.seperatorWhitespace & ", leading SPACE LF" ); + _executeTest( " " & chr( 10 ) & arguments.sql & " " & chr( 10 ), arguments.comments, + arguments.seperatorWhitespace & ", leading and trailing SPACE LF" ); + _executeTest( arguments.sql & " " & chr( 10 ), arguments.comments, + arguments.seperatorWhitespace & ", trailing SPACE LF" ); + _executeTest( chr( 10 ) & arguments.sql, arguments.comments, + arguments.seperatorWhitespace & ", leading LF"); + _executeTest( chr( 10 ) & arguments.sql & chr( 10 ), arguments.comments, + arguments.seperatorWhitespace & ", leading and trailing LF" ); + _executeTest( arguments.sql & chr( 10 ), arguments.comments, + arguments.seperatorWhitespace & ", trailing LF" ); + } + + private function _executeTest( string sql, array comments, string whitespaceDesc ){ + + // real database, h2 + var db = doQuery( arguments.sql, arguments.whitespaceDesc, "" ); + for ( var comment in arguments.comments ){ + expect ( db.result.sql ).toInclude( comment ); + } + expect( db.rs.engine ).toBe( "lucee" ); + + // QoQ + var qoq = doQuery( arguments.sql, arguments.whitespaceDesc, "query" ); + for ( var comment in arguments.comments ){ + expect ( qoq.result.sql ).toInclude( comment ); + } + expect( qoq.rs.engine ).toBe( "lucee" ); + // systemOutput( out.result, true ); + } + + private function doQuery( string sql, string whitespaceDesc, string dbtype ){ + try { + query name="local.rs" datasource="#ds#" params="#params#" dbtype="#arguments.dbtype#" result="local.result" { + echo( sql ); + } + } catch (e) { + systemOutput( "WHITESPACE: " & arguments.whitespaceDesc, true ); + systemOutput("Source: "& sql, true); + // if ( e.stackTrace.indexOf("lucee.runtime.exp.DatabaseException:") neq 0 ) + rethrow; + //systemOutput(e.stackTrace, true); + } + /* + systemOutput("", true); + systemOutput("Parsed: " & result.sql, true); + systemOutput("Source: "& sql, true); + */ + return { + result: result, + rs: rs + } + } + + private function _setupDatabase() { + query datasource=ds { + echo("CREATE TABLE qry ( id int NOT NULL, engine varchar(255) ) "); + } + var delim = ""; + query datasource=ds { + echo("insert into qry ( id, engine ) VALUES "); + loop query="qry" { + echo(" #delim# ( #qry.id#, '#qry.engine#' ) "); + delim = ","; + } + } + } + +} \ No newline at end of file