Skip to content

Commit

Permalink
fix: makes escape processing strict according to JDBC spec
Browse files Browse the repository at this point in the history
before, escape code parsing was very lenient and
invalid escape code formats were accidentally processed.

closes #657
  • Loading branch information
seut authored and vlsi committed Oct 8, 2016
1 parent 7270435 commit 00a8478
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 37 deletions.
115 changes: 79 additions & 36 deletions pgjdbc/src/main/java/org/postgresql/core/Parser.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@


/** /**
* Basic query parser infrastructure. * Basic query parser infrastructure.
* Note: This class should not be considered as pgjdbc public API.
* *
* @author Michael Paesold (mpaesold@gmx.at) * @author Michael Paesold (mpaesold@gmx.at)
* @author Christopher Deckers (chrriis@gmail.com) * @author Christopher Deckers (chrriis@gmail.com)
Expand Down Expand Up @@ -961,7 +962,7 @@ public static JdbcCallParseInfo modifyJdbcCall(String jdbcSql, boolean stdString
* @param standardConformingStrings whether standard_conforming_strings is on * @param standardConformingStrings whether standard_conforming_strings is on
* @return PostgreSQL-compatible SQL * @return PostgreSQL-compatible SQL
*/ */
static String replaceProcessing(String p_sql, boolean replaceProcessingEnabled, public static String replaceProcessing(String p_sql, boolean replaceProcessingEnabled,
boolean standardConformingStrings) throws SQLException { boolean standardConformingStrings) throws SQLException {
if (replaceProcessingEnabled) { if (replaceProcessingEnabled) {
// Since escape codes can only appear in SQL CODE, we keep track // Since escape codes can only appear in SQL CODE, we keep track
Expand Down Expand Up @@ -1012,6 +1013,8 @@ private static int parseSql(char[] p_sql, int i, StringBuilder newsql, boolean s
i--; i--;
while (!endOfNested && ++i < len) { while (!endOfNested && ++i < len) {
char c = p_sql[i]; char c = p_sql[i];

state_switch:
switch (state) { switch (state) {
case IN_SQLCODE: case IN_SQLCODE:
if (c == '$') { if (c == '$') {
Expand Down Expand Up @@ -1054,37 +1057,20 @@ private static int parseSql(char[] p_sql, int i, StringBuilder newsql, boolean s
break; break;
} else if (c == '{') { // start of an escape code? } else if (c == '{') { // start of an escape code?
if (i + 1 < len) { if (i + 1 < len) {
char next = p_sql[i + 1]; SqlParseState[] availableStates = SqlParseState.values();
char nextnext = (i + 2 < len) ? p_sql[i + 2] : '\0'; // skip first state, it's not a escape code state
if (next == 'd' || next == 'D') { for (int j = 1; j < availableStates.length; j++) {
state = SqlParseState.ESC_TIMEDATE; SqlParseState availableState = availableStates[j];
i++; int matchedPosition = availableState.getMatchedPosition(p_sql, i + 1);
newsql.append("DATE "); if (matchedPosition == 0) {
break; continue;
} else if (next == 't' || next == 'T') { }
state = SqlParseState.ESC_TIMEDATE; i += matchedPosition;
if (nextnext == 's' || nextnext == 'S') { if (availableState.replacementKeyword != null) {
// timestamp constant newsql.append(availableState.replacementKeyword);
i += 2;
newsql.append("TIMESTAMP ");
} else {
// time constant
i++;
newsql.append("TIME ");
} }
break; state = availableState;
} else if (next == 'f' || next == 'F') { break state_switch;
state = SqlParseState.ESC_FUNCTION;
i += (nextnext == 'n' || nextnext == 'N') ? 2 : 1;
break;
} else if (next == 'o' || next == 'O') {
state = SqlParseState.ESC_OUTERJOIN;
i += (nextnext == 'j' || nextnext == 'J') ? 2 : 1;
break;
} else if (next == 'e' || next == 'E') {
// we assume that escape is the only escape sequence beginning with e
state = SqlParseState.ESC_ESCAPECHAR;
break;
} }
} }
} }
Expand Down Expand Up @@ -1114,7 +1100,9 @@ private static int parseSql(char[] p_sql, int i, StringBuilder newsql, boolean s
} }
state = SqlParseState.IN_SQLCODE; // end of escaped function (or query) state = SqlParseState.IN_SQLCODE; // end of escaped function (or query)
break; break;
case ESC_TIMEDATE: case ESC_DATE:
case ESC_TIME:
case ESC_TIMESTAMP:
case ESC_OUTERJOIN: case ESC_OUTERJOIN:
case ESC_ESCAPECHAR: case ESC_ESCAPECHAR:
if (c == '}') { if (c == '}') {
Expand Down Expand Up @@ -1178,12 +1166,67 @@ private static String escapeFunction(String functionName, String args, boolean s
} }
} }


private final static char[] QUOTE_OR_ALPHABETIC_MARKER = new char[]{'\"', '0'};
private final static char[] SINGLE_QUOTE = new char[]{'\''};

// Static variables for parsing SQL when replaceProcessing is true. // Static variables for parsing SQL when replaceProcessing is true.
private enum SqlParseState { private enum SqlParseState {
IN_SQLCODE, IN_SQLCODE,
ESC_TIMEDATE, ESC_DATE("d", SINGLE_QUOTE, "DATE "),
ESC_FUNCTION, ESC_TIME("t", SINGLE_QUOTE, "TIME "),
ESC_OUTERJOIN,
ESC_ESCAPECHAR; ESC_TIMESTAMP("ts", SINGLE_QUOTE, "TIMESTAMP "),
ESC_FUNCTION("fn", QUOTE_OR_ALPHABETIC_MARKER, null),
ESC_OUTERJOIN("oj", QUOTE_OR_ALPHABETIC_MARKER, null),
ESC_ESCAPECHAR("escape", SINGLE_QUOTE, "ESCAPE ");

private final char[] escapeKeyword;
private final char[] allowedValues;
private final String replacementKeyword;

SqlParseState() {
this("", new char[0], null);
}

SqlParseState(String escapeKeyword, char[] allowedValues, String replacementKeyword) {
this.escapeKeyword = escapeKeyword.toCharArray();
this.allowedValues = allowedValues;
this.replacementKeyword = replacementKeyword;
}

private int getMatchedPosition(char[] p_sql, int pos) {
int newPos = pos;

// check for the keyword
for (char c : escapeKeyword) {
if (newPos >= p_sql.length) {
return 0;
}
char curr = p_sql[newPos++];
if (curr != c && curr != Character.toUpperCase(c)) {
return 0;
}
}
if (newPos >= p_sql.length) {
return 0;
}

// check for the beginning of the value
char curr = p_sql[newPos];
// ignore any in-between whitespace
while (curr == ' ') {
newPos++;
if (newPos >= p_sql.length) {
return 0;
}
curr = p_sql[newPos];
}
for (char c : allowedValues) {
if (curr == c || (c == '0' && Character.isLetter(curr))) {
return newPos - pos;
}
}
return 0;
}
} }
} }
25 changes: 25 additions & 0 deletions pgjdbc/src/test/java/org/postgresql/core/ParserTest.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -103,4 +103,29 @@ public void testSelectCommandParsing() {
"select".getChars(0, 6, command, 0); "select".getChars(0, 6, command, 0);
Assert.assertTrue("Failed to correctly parse lower case command.", Parser.parseSelectKeyword(command, 0)); Assert.assertTrue("Failed to correctly parse lower case command.", Parser.parseSelectKeyword(command, 0));
} }

public void testEscapeProcessing() throws Exception {
Assert.assertEquals("DATE '1999-01-09'", Parser.replaceProcessing("{d '1999-01-09'}", true, false));
Assert.assertEquals("DATE '1999-01-09'", Parser.replaceProcessing("{D '1999-01-09'}", true, false));
Assert.assertEquals("TIME '20:00:03'", Parser.replaceProcessing("{t '20:00:03'}", true, false));
Assert.assertEquals("TIME '20:00:03'", Parser.replaceProcessing("{T '20:00:03'}", true, false));
Assert.assertEquals("TIMESTAMP '1999-01-09 20:11:11.123455'", Parser.replaceProcessing("{ts '1999-01-09 20:11:11.123455'}", true, false));
Assert.assertEquals("TIMESTAMP '1999-01-09 20:11:11.123455'", Parser.replaceProcessing("{Ts '1999-01-09 20:11:11.123455'}", true, false));

Assert.assertEquals("user", Parser.replaceProcessing("{fn user()}", true, false));
Assert.assertEquals("cos(1)", Parser.replaceProcessing("{fn cos(1)}", true, false));
Assert.assertEquals("extract(week from DATE '2005-01-24')", Parser.replaceProcessing("{fn week({d '2005-01-24'})}", true, false));

Assert.assertEquals("\"T1\" LEFT OUTER JOIN t2 ON \"T1\".id = t2.id",
Parser.replaceProcessing("{oj \"T1\" LEFT OUTER JOIN t2 ON \"T1\".id = t2.id}", true, false));

Assert.assertEquals("ESCAPE '_'", Parser.replaceProcessing("{escape '_'}", true, false));

// nothing should be changed in that case, no valid escape code
Assert.assertEquals("{obj : 1}", Parser.replaceProcessing("{obj : 1}", true, false));
}

public void testUnterminatedEscape() throws Exception {
Assert.assertEquals("{oj ", Parser.replaceProcessing("{oj ", true, false));
}
} }
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void testPrepareCall() {
public void testNativeSQL() throws Exception { public void testNativeSQL() throws Exception {
// test a simple escape // test a simple escape
con = TestUtil.openDB(); con = TestUtil.openDB();
assertEquals("DATE '2005-01-24'", con.nativeSQL("{d '2005-01-24'}")); assertEquals("DATE '2005-01-24'", con.nativeSQL("{d '2005-01-24'}"));
} }


/* /*
Expand Down
Original file line number Original file line Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.postgresql.benchmark.escaping;

import org.postgresql.core.Parser;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Threads;
import org.openjdk.jmh.annotations.Warmup;

import java.util.concurrent.TimeUnit;

@Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
@State(Scope.Thread)
@Threads(1)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class EscapeProcessing {

private String fnEscapeSQL = "{fn week({d '2005-01-24'})}";
private boolean replaceProcessingEnabled = true;
private boolean standardConformingStrings = false;

@Benchmark
public String escapeFunctionWithDate() throws Exception {
return Parser.replaceProcessing(fnEscapeSQL, replaceProcessingEnabled, standardConformingStrings);
}
}

0 comments on commit 00a8478

Please sign in to comment.