Skip to content

Commit

Permalink
Fix for Bug#33637993, Loss of backslashes in data after modify api is…
Browse files Browse the repository at this point in the history
… used.

Change-Id: Ibd64876e597bd76223b3fa40f672a817c4946a35
  • Loading branch information
fjssilva committed Sep 3, 2022
1 parent cb57466 commit 6334292
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 37 deletions.
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

Version 8.0.31

- Fix for Bug#33637993, Loss of backslashes in data after modify api is used.

- Fix for Bug#34529014, JSonParser not accepting valid JSON string when converting to DbDoc.

- Fix for Bug#67828 (Bug#15967406), Crashing applets due to reading file.encoding system property.
Expand Down
71 changes: 55 additions & 16 deletions src/main/user-impl/java/com/mysql/cj/xdevapi/ExprParser.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates.
* Copyright (c) 2015, 2022, Oracle and/or its affiliates.
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, version 2.0, as published by the
Expand Down Expand Up @@ -54,7 +54,7 @@

// Grammar includes precedence & associativity of binary operators:
// (^ refers to the preceding production)
// (c.f. https://dev.mysql.com/doc/refman/5.7/en/operator-precedence.html)
// (c.f. https://dev.mysql.com/doc/refman/8.0/en/operator-precedence.html)
//
// AtomicExpr: [Unary]OpExpr | Identifier | FunctionCall | '(' Expr ')'
//
Expand All @@ -80,6 +80,20 @@
* Expression parser for X protocol.
*/
public class ExprParser {
private static HashMap<Character, Character> escapeChars = new HashMap<>();
static { // Replicated from JsonParser.EscapeChar
escapeChars.put('"', '"');
escapeChars.put('\'', '\'');
escapeChars.put('`', '`');
escapeChars.put('\\', '\\');
escapeChars.put('/', '/');
escapeChars.put('b', '\b');
escapeChars.put('f', '\f');
escapeChars.put('n', '\n');
escapeChars.put('r', '\r');
escapeChars.put('t', '\t');
}

/** String being parsed. */
String string;
/** Token stream produced by lexer. */
Expand Down Expand Up @@ -124,7 +138,7 @@ public ExprParser(String s, boolean allowRelationalColumns) {
/**
* Token types used by the lexer.
*/
private static enum TokenType {
private enum TokenType {
NOT, AND, ANDAND, OR, OROR, XOR, IS, LPAREN, RPAREN, LSQBRACKET, RSQBRACKET, BETWEEN, TRUE, NULL, FALSE, IN, LIKE, INTERVAL, REGEXP, ESCAPE, IDENT,
LSTRING, LNUM_INT, LNUM_DOUBLE, DOT, DOLLAR, COMMA, EQ, NE, GT, GE, LT, LE, BITAND, BITOR, BITXOR, LSHIFT, RSHIFT, PLUS, MINUS, STAR, SLASH, HEX, BIN,
NEG, BANG, EROTEME, MICROSECOND, SECOND, MINUTE, HOUR, DAY, WEEK, MONTH, QUARTER, YEAR, SECOND_MICROSECOND, MINUTE_MICROSECOND, MINUTE_SECOND,
Expand Down Expand Up @@ -406,20 +420,45 @@ void lex() {
char quoteChar = c;
StringBuilder val = new StringBuilder();
try {
for (c = this.string.charAt(++i); c != quoteChar
boolean escapeNextChar = false;
for (c = this.string.charAt(++i); c != quoteChar || escapeNextChar
|| (i + 1 < this.string.length() && this.string.charAt(i + 1) == quoteChar); c = this.string.charAt(++i)) {
if (c == '\\' || c == quoteChar) {
++i;
if (escapeNextChar) {
if (escapeChars.containsKey(c)) {
val.append(escapeChars.get(c));
} else if (c == 'u') {
// \\u[4 hex digits] represents a unicode code point (ISO/IEC 10646)
char[] buf = new char[4];
this.string.getChars(++i, i + 4, buf, 0);
String hexCodePoint = String.valueOf(buf);
try {
val.append((char) Integer.parseInt(hexCodePoint, 16));
} catch (NumberFormatException e) {
throw new WrongArgumentException("Invalid Unicode code point '" + hexCodePoint + "'");
}
i += 3;
} else {
val.append('\\').append(c);
}
escapeNextChar = false;
} else {
if (c == '\\' || c == quoteChar) { // Escape sequence or two consecutive quotes
escapeNextChar = true;
} else {
val.append(c);
}
}
val.append(this.string.charAt(i));
}
if (escapeNextChar) {
throw new WrongArgumentException("Unterminated escape sequence at " + i);
}
} catch (StringIndexOutOfBoundsException ex) {
throw new WrongArgumentException("Unterminated string starting at " + start);
}
this.tokens.add(new Token(quoteChar == '`' ? TokenType.IDENT : TokenType.LSTRING, val.toString()));
break;
default:
throw new WrongArgumentException("Can't parse at pos: " + i);
throw new WrongArgumentException("Can't parse at position " + i);
}
} else {
// otherwise, it's an identifier
Expand Down Expand Up @@ -458,10 +497,10 @@ void lex() {
*/
void assertTokenAt(int pos, TokenType type) {
if (this.tokens.size() <= pos) {
throw new WrongArgumentException("No more tokens when expecting " + type + " at token pos " + pos);
throw new WrongArgumentException("No more tokens when expecting " + type + " at token position " + pos);
}
if (this.tokens.get(pos).type != type) {
throw new WrongArgumentException("Expected token type " + type + " at token pos " + pos);
throw new WrongArgumentException("Expected token type " + type + " at token position " + pos);
}
}

Expand Down Expand Up @@ -590,7 +629,7 @@ DocumentPathItem docPathMember() {
consumeToken(TokenType.LSTRING);
memberName = t.value;
} else {
throw new WrongArgumentException("Expected token type IDENT or LSTRING in JSON path at token pos " + this.tokenPos);
throw new WrongArgumentException("Expected token type IDENT or LSTRING in JSON path at token position " + this.tokenPos);
}
DocumentPathItem.Builder item = DocumentPathItem.newBuilder();
item.setType(DocumentPathItem.Type.MEMBER);
Expand Down Expand Up @@ -619,7 +658,7 @@ DocumentPathItem docPathArrayLoc() {
consumeToken(TokenType.RSQBRACKET);
return builder.setType(DocumentPathItem.Type.ARRAY_INDEX).setIndex(v).build();
} else {
throw new WrongArgumentException("Expected token type STAR or LNUM_INT in JSON path array index at token pos " + this.tokenPos);
throw new WrongArgumentException("Expected token type STAR or LNUM_INT in JSON path array index at token position " + this.tokenPos);
}
}

Expand Down Expand Up @@ -737,7 +776,7 @@ Expr buildUnaryOp(String name, Expr param) {
*/
Expr atomicExpr() { // constant, identifier, variable, function call, etc
if (this.tokenPos >= this.tokens.size()) {
throw new WrongArgumentException("No more tokens when expecting one at token pos " + this.tokenPos);
throw new WrongArgumentException("No more tokens when expecting one at token position " + this.tokenPos);
}
Token t = this.tokens.get(this.tokenPos);
this.tokenPos++; // consume
Expand All @@ -754,7 +793,7 @@ Expr atomicExpr() { // constant, identifier, variable, function call, etc
} else if (t.type == TokenType.EROTEME) {
placeholderName = String.valueOf(this.positionalPlaceholderCount);
} else {
throw new WrongArgumentException("Invalid placeholder name at token pos " + this.tokenPos);
throw new WrongArgumentException("Invalid placeholder name at token position " + this.tokenPos);
}
Expr.Builder placeholder = Expr.newBuilder().setType(Expr.Type.PLACEHOLDER);
if (this.placeholderNameToPosition.containsKey(placeholderName)) {
Expand Down Expand Up @@ -880,7 +919,7 @@ && posTokenTypeEquals(this.tokenPos + 2, TokenType.IDENT) && posTokenTypeEquals(
default:
break;
}
throw new WrongArgumentException("Cannot find atomic expression at token pos: " + (this.tokenPos - 1));
throw new WrongArgumentException("Cannot find atomic expression at token position " + (this.tokenPos - 1));
}

/**
Expand Down Expand Up @@ -1030,7 +1069,7 @@ Expr ilriExpr() {
params.add(compExpr());
break;
default:
throw new WrongArgumentException("Unknown token after NOT at pos: " + this.tokenPos);
throw new WrongArgumentException("Unknown token after NOT at position " + this.tokenPos);
}
if (isNot) {
opName = "not_" + opName;
Expand Down
8 changes: 6 additions & 2 deletions src/test/java/com/mysql/cj/xdevapi/ExprParserTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2021, Oracle and/or its affiliates.
* Copyright (c) 2015, 2022, Oracle and/or its affiliates.
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, version 2.0, as published by the
Expand Down Expand Up @@ -107,7 +107,7 @@ public void testUnparseables() {
}

/**
* Check that a string parses and is reconstituted as a string that we expect. Futher we parse the canonical version to make sure it doesn't change.
* Check that a string parses and is reconstituted as a string that we expect. Further we parse the canonical version to make sure it doesn't change.
*
* @param input
* @param expected
Expand Down Expand Up @@ -220,6 +220,10 @@ public void testRoundTrips() {
// star function
checkParseRoundTrip("*", "*");
checkParseRoundTrip("count(*) + 1", "(count(*) + 1)");
checkParseRoundTrip("foo\u003Dbar", "(foo == bar)");
checkParseRoundTrip("\"foo\"", "\"foo\"");
checkParseRoundTrip("\"foo\\\"bar\"", "\"foo\"\"bar\"");
checkParseRoundTrip("\"foo\nbar\"", "\"foo\nbar\""); // TODO: Could it be that the unparsed \n should be escaped?
}

/**
Expand Down
42 changes: 23 additions & 19 deletions src/test/java/testsuite/x/devapi/CollectionFindTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2021, Oracle and/or its affiliates.
* Copyright (c) 2015, 2022, Oracle and/or its affiliates.
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, version 2.0, as published by the
Expand Down Expand Up @@ -1236,13 +1236,15 @@ public void testOverlaps() {
assertTrue(((JsonLiteral) doc.get("overlaps")).equals(JsonLiteral.FALSE));

// TSFR4
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0", () -> this.collection.find("overlaps $.list").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 4", () -> this.collection.find("$.list OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0", () -> this.collection.find("overlaps").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1", () -> this.collection.find("NOT overlaps").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 5",
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0",
() -> this.collection.find("overlaps $.list").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 4",
() -> this.collection.find("$.list OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0", () -> this.collection.find("overlaps").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1", () -> this.collection.find("NOT overlaps").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 5",
() -> this.collection.find("$.list NOT OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1", () -> this.collection.find("not overlaps").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1", () -> this.collection.find("not overlaps").execute());

// TSFR5
res = this.collection.find("[1, 2, 3] OVERLAPS $.age").execute();
Expand Down Expand Up @@ -2268,29 +2270,31 @@ public void testCollectionFindOverlaps() throws Exception {
assertEquals(maxrec, docs.count());

/* When the right number of operands are not provided - error should be thrown */
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0",
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0",
() -> this.collection.find("overlaps $.ARR1").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 4",
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 4",
() -> this.collection.find("$.ARR1 OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0", () -> this.collection.find("OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1",
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0", () -> this.collection.find("OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1",
() -> this.collection.find("not overlaps $.ARR1").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 5",
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 5",
() -> this.collection.find("$.ARR1 NOT OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1", () -> this.collection.find("not OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1",
() -> this.collection.find("not OVERLAPS").execute());

final Table table1 = this.schema.getCollectionAsTable(this.collectionName);

assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0",
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0",
() -> table1.select().where("overlaps $.ARR1").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 4",
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 4",
() -> table1.select().where("$.ARR1 OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 0", () -> table1.select().where("OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1",
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 0", () -> table1.select().where("OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1",
() -> table1.select().where("not overlaps $.ARR1").execute());
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token pos 5",
assertThrows(WrongArgumentException.class, "No more tokens when expecting one at token position 5",
() -> table1.select().where("$.ARR1 NOT OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token pos: 1", () -> table1.select().where("not OVERLAPS").execute());
assertThrows(WrongArgumentException.class, "Cannot find atomic expression at token position 1",
() -> table1.select().where("not OVERLAPS").execute());

/* invalid criteria, e.g. .find("[1, 2, 3] OVERLAPS $.age") . where $.age is atomic value */
dropCollection("coll2");
Expand Down
Loading

0 comments on commit 6334292

Please sign in to comment.