From 6235f3a4434545d6db1026f2b40698b5296f1944 Mon Sep 17 00:00:00 2001 From: Michael Bolz Date: Tue, 17 Nov 2015 15:29:01 +0100 Subject: [PATCH] [OLINGO-568] Minor code and character validation improvements --- .../olingo/server/core/uri/parser/Parser.java | 2 +- .../core/uri/parser/search/SearchParser.java | 13 +- .../uri/parser/search/SearchQueryToken.java | 2 +- .../uri/parser/search/SearchTokenizer.java | 119 +++++++++++++++--- .../search/SearchParserAndTokenizerTest.java | 3 +- .../uri/parser/search/SearchParserTest.java | 4 +- .../parser/search/SearchTokenizerTest.java | 34 +++++ 7 files changed, 147 insertions(+), 30 deletions(-) diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java index 82094cf18..d6cb557e2 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java @@ -217,7 +217,7 @@ public class Parser { systemOption = (OrderByOptionImpl) uriParseTreeVisitor.visitOrderByEOF(ctxOrderByExpression); } else if (option.name.equals(SystemQueryOptionKind.SEARCH.toString())) { SearchParser searchParser = new SearchParser(); - systemOption = searchParser.parse(path, option.value); + systemOption = searchParser.parse(option.value); } else if (option.name.equals(SystemQueryOptionKind.SELECT.toString())) { SelectEOFContext ctxSelectEOF = (SelectEOFContext) parseRule(option.value, ParserEntryRules.Select); diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java index 804ca67e2..a9fe332dd 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java @@ -33,11 +33,11 @@ public class SearchParser { private Iterator tokens; private SearchQueryToken token; - public SearchOption parse(String path, String value) throws SearchParserException, SearchTokenizerException { + public SearchOption parse(String searchQuery) throws SearchParserException, SearchTokenizerException { SearchTokenizer tokenizer = new SearchTokenizer(); SearchExpression searchExpression; try { - searchExpression = parseInternal(tokenizer.tokenize(value)); + searchExpression = parse(tokenizer.tokenize(searchQuery)); } catch (SearchTokenizerException e) { return null; } @@ -46,7 +46,7 @@ public class SearchParser { return searchOption; } - protected SearchExpression parseInternal(List tokens) throws SearchParserException { + protected SearchExpression parse(List tokens) throws SearchParserException { this.tokens = tokens.iterator(); nextToken(); if (token == null) { @@ -101,10 +101,7 @@ public class SearchParser { } private boolean isToken(SearchQueryToken.Token toCheckToken) { - if (token == null) { - return false; - } - return token.getToken() == toCheckToken; + return token != null && token.getToken() == toCheckToken; } private void validateToken(SearchQueryToken.Token toValidateToken) throws SearchParserException { @@ -194,6 +191,6 @@ public class SearchParser { private SearchTerm processPhrase() { String literal = token.getLiteral(); nextToken(); - return new SearchTermImpl(literal); + return new SearchTermImpl(literal.substring(1,literal.length()-1)); } } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchQueryToken.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchQueryToken.java index eb1a0095a..3fb66f1ac 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchQueryToken.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchQueryToken.java @@ -19,7 +19,7 @@ package org.apache.olingo.server.core.uri.parser.search; public interface SearchQueryToken { - enum Token {OPEN, TERM, NOT, AND, OR, WORD, PHRASE, CLOSE} + enum Token {OPEN, NOT, AND, OR, WORD, PHRASE, CLOSE} Token getToken(); String getLiteral(); diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java index 1e3b2efc3..fb0ad9452 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java @@ -98,16 +98,22 @@ public class SearchTokenizer { } /** - * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" - * other-delims = "!" / "(" / ")" / "*" / "+" / "," / ";" + * searchPhrase = quotation-mark 1*qchar-no-AMP-DQUOTE quotation-mark + * + * qchar-no-AMP-DQUOTE = qchar-unescaped / escape ( escape / quotation-mark ) + * * qchar-unescaped = unreserved / pct-encoded-unescaped / other-delims / ":" / "@" / "/" / "?" / "$" / "'" / "=" + * + * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" + * + * escape = "\" / "%5C" ; reverse solidus U+005C + * * pct-encoded-unescaped = "%" ( "0" / "1" / "3" / "4" / "6" / "7" / "8" / "9" / A-to-F ) HEXDIG * / "%" "2" ( "0" / "1" / "3" / "4" / "5" / "6" / "7" / "8" / "9" / A-to-F ) * / "%" "5" ( DIGIT / "A" / "B" / "D" / "E" / "F" ) * - * qchar-no-AMP-DQUOTE = qchar-unescaped / escape ( escape / quotation-mark ) + * other-delims = "!" / "(" / ")" / "*" / "+" / "," / ";" * - * escape = "\" / "%5C" ; reverse solidus U+005C * quotation-mark = DQUOTE / "%22" * * ALPHA = %x41-5A / %x61-7A @@ -119,19 +125,100 @@ public class SearchTokenizer { */ static boolean isAllowedPhrase(final char character) { // FIXME mibo: check missing - return isAlphaOrDigit(character) - || character == '-' - || character == '.' - || character == '_' - || character == '~' - || character == ':' - || character == '@' - || character == '/' - || character == '$' - || character == '\'' - || character == '='; + return isQCharUnescaped(character) || isEscaped(character); } + /** + * escape = "\" / "%5C" ; reverse solidus U+005C + * @param character which is checked + * @return true if character is allowed + */ + private static boolean isEscaped(char character) { + // TODO: mibo(151117): check how to implement + return false; + } + + /** + * qchar-unescaped = unreserved / pct-encoded-unescaped / other-delims / ":" / "@" / "/" / "?" / "$" / "'" / "=" + * @param character which is checked + * @return true if character is allowed + */ + private static boolean isQCharUnescaped(char character) { + return isUnreserved(character) + || isPctEncodedUnescaped(character) + || isOtherDelims(character) + || character == ':' + || character == '@' + || character == '/' + || character == '$' + || character == '\'' + || character == '='; + } + + /** + * other-delims = "!" / "(" / ")" / "*" / "+" / "," / ";" + * @param character which is checked + * @return true if character is allowed + */ + private static boolean isOtherDelims(char character) { + return character == '!' + || character == '(' + || character == ')' + || character == '*' + || character == '+' + || character == ',' + || character == ';'; + } + + /** + * pct-encoded-unescaped = "%" ( "0" / "1" / "3" / "4" / "6" / "7" / "8" / "9" / A-to-F ) HEXDIG + * / "%" "2" ( "0" / "1" / "3" / "4" / "5" / "6" / "7" / "8" / "9" / A-to-F ) + * / "%" "5" ( DIGIT / "A" / "B" / "D" / "E" / "F" ) + * + * HEXDIG = DIGIT / A-to-F + * + * @param character which is checked + * @return true if character is allowed + */ + private static boolean isPctEncodedUnescaped(char character) { + String hex = Integer.toHexString((int) character); + char aschar[] = hex.toCharArray(); + if(aschar[0] == '%') { + if(aschar[1] == '2') { + return aschar[2] != '2' && isHexDigit(aschar[2]); + } else if(aschar[1] == '5') { + return aschar[2] != 'C' && isHexDigit(aschar[2]); + } else if(isHexDigit(aschar[1])) { + return isHexDigit(aschar[2]); + } + } + return false; + } + + private static boolean isHexDigit(char character) { + return 'A' <= character && character <= 'F' // case A..F + || '0' <= character && character <= '9'; // case 0..9 + } + + /** + * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" + * @param character which is checked + * @return true if character is allowed + */ + private static boolean isUnreserved(char character) { + return isAlphaOrDigit(character) + || character == '-' + || character == '.' + || character == '_' + || character == '~'; + } + + /** + * ALPHA = %x41-5A / %x61-7A + * DIGIT = %x30-39 + * @param character which is checked + * @return true if character is allowed + */ private static boolean isAlphaOrDigit(char character) { return 'A' <= character && character <= 'Z' // case A..Z || 'a' <= character && character <= 'z' // case a..z @@ -220,7 +307,7 @@ public class SearchTokenizer { private class SearchTermState extends LiteralState { public SearchTermState() { - super(Token.TERM); + super(null); } @Override diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java index dd5ab70d1..23cac8e3d 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java @@ -185,7 +185,7 @@ public class SearchParserAndTokenizerTest { private SearchExpression getSearchExpression() throws SearchParserException, SearchTokenizerException { SearchParser tokenizer = new SearchParser(); - SearchOption result = tokenizer.parse(null, searchQuery); + SearchOption result = tokenizer.parse(searchQuery); Assert.assertNotNull(result); final SearchExpression searchExpression = result.getSearchExpression(); Assert.assertNotNull(searchExpression); @@ -195,5 +195,4 @@ public class SearchParserAndTokenizerTest { return searchExpression; } } - } diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java index 97e941c03..ee10e1a22 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java @@ -220,7 +220,7 @@ public class SearchParserTest extends SearchParser { private SearchExpression run(SearchQueryToken.Token... tokenArray) throws SearchParserException { List tokenList = prepareTokens(tokenArray); - SearchExpression se = parseInternal(tokenList); + SearchExpression se = parse(tokenList); assertNotNull(se); return se; } @@ -236,7 +236,7 @@ public class SearchParserTest extends SearchParser { when(token.getLiteral()).thenReturn("word" + wordNumber); wordNumber++; } else if (aTokenArray == Token.PHRASE) { - when(token.getLiteral()).thenReturn("phrase" + phraseNumber); + when(token.getLiteral()).thenReturn("\"phrase" + phraseNumber + "\""); phraseNumber++; } when(token.toString()).thenReturn("" + aTokenArray); diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java index ce4c3caf4..8408e93e1 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java @@ -19,6 +19,7 @@ package org.apache.olingo.server.core.uri.parser.search; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import java.util.ArrayList; @@ -109,7 +110,40 @@ public class SearchTokenizerTest { SearchValidator.init("abc or \"xyz\"").addExpected(WORD, WORD, PHRASE).validate(); } + /** + * https://tools.oasis-open.org/version-control/browse/wsvn/odata/trunk/spec/ABNF/odata-abnf-testcases.xml + * @throws Exception + */ @Test + @Ignore("Test must be moved to SearchParserTest and SearchParserAndTokenizerTest") + public void parsePhraseAbnfTestcases() throws Exception { + // + SearchValidator.init("\"blue%20green\"").validate(); + // + SearchValidator.init("\"blue%20green%22").validate(); + // + // $search="blue\"green" + SearchValidator.init("\"blue\\\"green\"").validate(); + + // + // $search="blue\\green" + SearchValidator.init("\"blue\\\\green\"").validate(); + + // + SearchValidator.init("\"blue\"green\"").validate(); + + // + SearchValidator.init("\"blue%22green\"").validate(); + +// +// $search=blue green +// SearchValidator.init("\"blue%20green\"").validate(); + // +// SearchValidator.init("blue%20green").validate(); + } + + + @Test public void parseNot() throws Exception { SearchTokenizer tokenizer = new SearchTokenizer(); List result;