[OLINGO-568] Minor code and character validation improvements

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);

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<SearchQueryToken> 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<SearchQueryToken> tokens) throws SearchParserException {
+  protected SearchExpression parse(List<SearchQueryToken> 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 token != null && token.getToken() == toCheckToken;
-      return false;
-    }
-    return 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));
   }
 }

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();

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 / "-" / "." / "_" / "~"
+     * searchPhrase = quotation-mark 1*qchar-no-AMP-DQUOTE quotation-mark
-     * other-delims = "!" / "(" / ")" / "*" / "+" / "," / ";"
+     *
+     * 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)
+      return isQCharUnescaped(character) || isEscaped(character);
-          || character == '-'
-          || character == '.'
-          || character == '_'
-          || character == '~'
-          || character == ':'
-          || character == '@'
-          || character == '/'
-          || character == '$'
-          || character == '\''
-          || 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

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;
     }
   }
-
 }

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<SearchQueryToken> 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);

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 {
+    //    <TestCase Name="5.1.7 Search - simple phrase" Rule="queryOptions">
+    SearchValidator.init("\"blue%20green\"").validate();
+    //    <TestCase Name="5.1.7 Search - simple phrase" Rule="queryOptions">
+    SearchValidator.init("\"blue%20green%22").validate();
+    //    <TestCase Name="5.1.7 Search - phrase with escaped double-quote" Rule="queryOptions">
+    //    <Input>$search="blue\"green"</Input>
+    SearchValidator.init("\"blue\\\"green\"").validate();
+
+    //    <TestCase Name="5.1.7 Search - phrase with escaped backslash" Rule="queryOptions">
+    //    <Input>$search="blue\\green"</Input>
+    SearchValidator.init("\"blue\\\\green\"").validate();
+
+    //    <TestCase Name="5.1.7 Search - phrase with unescaped double-quote" Rule="queryOptions" FailAt="14">
+    SearchValidator.init("\"blue\"green\"").validate();
+
+    //    <TestCase Name="5.1.7 Search - phrase with unescaped double-quote" Rule="queryOptions" FailAt="16">
+    SearchValidator.init("\"blue%22green\"").validate();
+
+//    <TestCase Name="5.1.7 Search - implicit AND" Rule="queryOptions">
+//    <Input>$search=blue green</Input>
+//    SearchValidator.init("\"blue%20green\"").validate();
+    //    <TestCase Name="5.1.7 Search - implicit AND, encoced" Rule="queryOptions">
+//    SearchValidator.init("blue%20green").validate();
+  }
+
+
+    @Test
   public void parseNot() throws Exception {
     SearchTokenizer tokenizer = new SearchTokenizer();
     List<SearchQueryToken> result;