[OLINGO-396] Added stricter check for $format in uri

lib/server-core/src/main/antlr4/org/apache/olingo/server/core/uri/antlr/UriParser.g4

@@ -146,7 +146,7 @@ orderByItem         : vC=commonExpr ( WSP ( vA=ASC | vD=DESC ) )?;
 
 skip                : SKIP EQ INT;
 top                 : TOP EQ INT;
-//format              : FORMAT EQ ( ATOM | JSON | XML | PCHARS ( SLASH PCHARS)?);
+//format              : FORMAT EQ ( ATOM | JSON | XML | PCHARS SLASH PCHARS);
 
 inlinecount         : COUNT EQ booleanNonCase;
 

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java

@@ -27,10 +27,12 @@ import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.atn.PredictionMode;
 import org.antlr.v4.runtime.misc.ParseCancellationException;
 import org.apache.olingo.commons.api.edm.Edm;
+import org.apache.olingo.commons.api.format.ODataFormat;
 import org.apache.olingo.server.api.uri.UriInfo;
 import org.apache.olingo.server.api.uri.UriInfoKind;
 import org.apache.olingo.server.api.uri.UriResource;
 import org.apache.olingo.server.api.uri.UriResourcePartTyped;
+import org.apache.olingo.server.api.uri.queryoption.SystemQueryOptionKind;
 import org.apache.olingo.server.core.uri.UriInfoImpl;
 import org.apache.olingo.server.core.uri.antlr.UriLexer;
 import org.apache.olingo.server.core.uri.antlr.UriParserParser;
@@ -165,7 +167,7 @@ public class Parser {
             customOption.setName(option.name);
             customOption.setText(option.value);
             context.contextUriInfo.addCustomQueryOption(customOption);
-          } else if (option.name.equals("$filter")) {
+          } else if (option.name.equals(SystemQueryOptionKind.FILTER.toString())) {
             FilterExpressionEOFContext ctxFilterExpression =
                 (FilterExpressionEOFContext) parseRule(option.value, ParserEntryRules.FilterExpression);
 
@@ -174,14 +176,22 @@ public class Parser {
 
             context.contextUriInfo.setSystemQueryOption(filterOption);
 
-          } else if (option.name.equals("$format")) {
+          } else if (option.name.equals(SystemQueryOptionKind.FORMAT.toString())) {
             FormatOptionImpl formatOption = new FormatOptionImpl();
             formatOption.setName(option.name);
             formatOption.setText(option.value);
+            if (option.value.equalsIgnoreCase(ODataFormat.JSON.name())
+                || option.value.equalsIgnoreCase(ODataFormat.XML.name())
+                || option.value.equalsIgnoreCase(ODataFormat.ATOM.name())
+                || isFormatSyntaxValid(option)) {
               formatOption.setFormat(option.value);
+            } else {
+              throw new UriParserSemanticException("Illegal value of $format option!",
+                  UriParserSemanticException.MessageKeys.TEST);
+            }
             context.contextUriInfo.setSystemQueryOption(formatOption);
 
-          } else if (option.name.equals("$expand")) {
+          } else if (option.name.equals(SystemQueryOptionKind.EXPAND.toString())) {
             ExpandItemsEOFContext ctxExpandItems =
                 (ExpandItemsEOFContext) parseRule(option.value, ParserEntryRules.ExpandItems);
 
@@ -190,60 +200,62 @@ public class Parser {
 
             context.contextUriInfo.setSystemQueryOption(expandOption);
 
-          } else if (option.name.equals("$id")) {
+          } else if (option.name.equals(SystemQueryOptionKind.ID.toString())) {
             IdOptionImpl idOption = new IdOptionImpl();
             idOption.setName(option.name);
             idOption.setText(option.value);
             idOption.setValue(option.value);
             context.contextUriInfo.setSystemQueryOption(idOption);
-          } else if (option.name.equals("$orderby")) {
+          } else if (option.name.equals(SystemQueryOptionKind.LEVELS.toString())) {
+            throw new UriParserSyntaxException("System query option '$levels' is allowed only inside '$expand'!",
+                UriParserSyntaxException.MessageKeys.TEST);
+          } else if (option.name.equals(SystemQueryOptionKind.ORDERBY.toString())) {
             OrderByEOFContext ctxFilterExpression =
                 (OrderByEOFContext) parseRule(option.value, ParserEntryRules.Orderby);
 
-            OrderByOptionImpl filterOption =
+            OrderByOptionImpl orderByOption =
                 (OrderByOptionImpl) uriParseTreeVisitor.visitOrderByEOF(ctxFilterExpression);
 
-            context.contextUriInfo.setSystemQueryOption(filterOption);
+            context.contextUriInfo.setSystemQueryOption(orderByOption);
-          } else if (option.name.equals("$search")) {
+          } else if (option.name.equals(SystemQueryOptionKind.SEARCH.toString())) {
             throw new RuntimeException("System query option '$search' not implemented!");
-          } else if (option.name.equals("$select")) {
+          } else if (option.name.equals(SystemQueryOptionKind.SELECT.toString())) {
             SelectEOFContext ctxSelectEOF =
                 (SelectEOFContext) parseRule(option.value, ParserEntryRules.Select);
 
-            SelectOptionImpl expandOption =
+            SelectOptionImpl selectOption =
                 (SelectOptionImpl) uriParseTreeVisitor.visitSelectEOF(ctxSelectEOF);
 
-            context.contextUriInfo.setSystemQueryOption(expandOption);
+            context.contextUriInfo.setSystemQueryOption(selectOption);
-          } else if (option.name.equals("$skip")) {
+          } else if (option.name.equals(SystemQueryOptionKind.SKIP.toString())) {
-            SkipOptionImpl inlineCountOption = new SkipOptionImpl();
+            SkipOptionImpl skipOption = new SkipOptionImpl();
-            inlineCountOption.setName(option.name);
+            skipOption.setName(option.name);
-            inlineCountOption.setText(option.value);
+            skipOption.setText(option.value);
             try {
-              inlineCountOption.setValue(Integer.parseInt(option.value));
+              skipOption.setValue(Integer.parseInt(option.value));
             } catch (final NumberFormatException e) {
               throw new UriParserSemanticException("Illegal value of $skip option!", e,
                   UriParserSemanticException.MessageKeys.TEST);
             }
-            context.contextUriInfo.setSystemQueryOption(inlineCountOption);
+            context.contextUriInfo.setSystemQueryOption(skipOption);
-          } else if (option.name.equals("$skiptoken")) {
+          } else if (option.name.equals(SystemQueryOptionKind.SKIPTOKEN.toString())) {
-            SkipTokenOptionImpl inlineCountOption = new SkipTokenOptionImpl();
+            SkipTokenOptionImpl skipTokenOption = new SkipTokenOptionImpl();
-            inlineCountOption.setName(option.name);
+            skipTokenOption.setName(option.name);
-            inlineCountOption.setText(option.value);
+            skipTokenOption.setText(option.value);
-            inlineCountOption.setValue(option.value);
+            skipTokenOption.setValue(option.value);
-            context.contextUriInfo.setSystemQueryOption(inlineCountOption);
+            context.contextUriInfo.setSystemQueryOption(skipTokenOption);
-          } else if (option.name.equals("$top")) {
+          } else if (option.name.equals(SystemQueryOptionKind.TOP.toString())) {
-            TopOptionImpl inlineCountOption = new TopOptionImpl();
+            TopOptionImpl topOption = new TopOptionImpl();
-            inlineCountOption.setName(option.name);
+            topOption.setName(option.name);
-            inlineCountOption.setText(option.value);
+            topOption.setText(option.value);
             try {
-              inlineCountOption.setValue(Integer.parseInt(option.value));
+              topOption.setValue(Integer.parseInt(option.value));
             } catch (final NumberFormatException e) {
               throw new UriParserSemanticException("Illegal value of $top option!", e,
                   UriParserSemanticException.MessageKeys.TEST);
             }
-            context.contextUriInfo.setSystemQueryOption(inlineCountOption);
+            context.contextUriInfo.setSystemQueryOption(topOption);
-          } else if (option.name.equals("$count")) {
+          } else if (option.name.equals(SystemQueryOptionKind.COUNT.toString())) {
-            // todo create CountOption
             CountOptionImpl inlineCountOption = new CountOptionImpl();
             inlineCountOption.setName(option.name);
             inlineCountOption.setText(option.value);
@@ -254,6 +266,9 @@ public class Parser {
                   UriParserSemanticException.MessageKeys.TEST);
             }
             context.contextUriInfo.setSystemQueryOption(inlineCountOption);
+          } else {
+            throw new UriParserSyntaxException("Unknown system query option!",
+                UriParserSyntaxException.MessageKeys.TEST);
           }
         }
       }
@@ -272,6 +287,11 @@ public class Parser {
     return null;
   }
 
+  private boolean isFormatSyntaxValid(RawUri.QueryOption option) {
+    final int index = option.value.indexOf('/');
+    return index > 0 && index < option.value.length() - 1 && index == option.value.lastIndexOf('/');
+  }
+
   private ParserRuleContext parseRule(final String input, final ParserEntryRules entryPoint)
       throws UriParserSyntaxException {
     UriParserParser parser = null;

lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java

@@ -19,6 +19,7 @@
 package org.apache.olingo.server.core.uri.antlr;
 
 import org.apache.olingo.commons.api.edm.Edm;
+import org.apache.olingo.commons.api.http.HttpContentType;
 import org.apache.olingo.commons.core.Encoder;
 import org.apache.olingo.server.api.ODataApplicationException;
 import org.apache.olingo.server.api.uri.UriInfoKind;
@@ -1106,13 +1107,13 @@ public class TestFullResourcePath {
   public void runEsNameKeyCast() throws Exception {
     /*
      * testUri.runEx("ESTwoPrim(1)/com.sap.odata.test1.ETBase(1)")
-     * .isExSemantic(0);
+     * .isExSemantic(UriParserSemanticException.MessageKeys.TEST);
      * 
      * testUri.runEx("ESTwoPrim/com.sap.odata.test1.ETBase(1)/com.sap.odata.test1.ETTwoBase(1)")
-     * .isExSemantic(0);
+     * .isExSemantic(UriParserSemanticException.MessageKeys.TEST);
      * 
      * testUri.runEx("ESBase/com.sap.odata.test1.ETTwoPrim(1)")
-     * .isExSemantic(0);
+     * .isExSemantic(UriParserSemanticException.MessageKeys.TEST);
      */
 
     testUri.run("ESTwoPrim(1)/com.sap.odata.test1.ETBase")
@@ -2513,6 +2514,9 @@ public class TestFullResourcePath {
         .isKind(UriInfoKind.resource).goPath()
         .isEntitySet("ESKeyNav")
         .isTopText("-3");
+
+    testUri.runEx("ESKeyNav?$top=undefined").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESKeyNav?$top=").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
   }
 
   @Test
@@ -2533,6 +2537,14 @@ public class TestFullResourcePath {
     testUri.run("ESKeyNav(1)?$format=Test_all_valid_signsSpecified_for_format_signs%26-._~$@%27/Aa123%26-._~$@%27")
         .isKind(UriInfoKind.resource).goPath()
         .isFormatText("Test_all_valid_signsSpecified_for_format_signs&-._~$@'/Aa123&-._~$@'");
+    testUri.run("ESKeyNav(1)?$format=" + HttpContentType.APPLICATION_ATOM_XML_ENTRY_UTF8)
+        .isKind(UriInfoKind.resource).goPath()
+        .isFormatText(HttpContentType.APPLICATION_ATOM_XML_ENTRY_UTF8);
+    testUri.runEx("ESKeyNav(1)?$format=noSlash").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESKeyNav(1)?$format=slashAtEnd/").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESKeyNav(1)?$format=/startsWithSlash").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESKeyNav(1)?$format=two/Slashes/tooMuch").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESKeyNav(1)?$format=").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
   }
 
   @Test
@@ -2544,6 +2556,8 @@ public class TestFullResourcePath {
     testUri.run("ESAllPrim?$count=false")
         .isKind(UriInfoKind.resource).goPath()
         .isInlineCountText("false");
+    testUri.runEx("ESAllPrim?$count=undefined").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESAllPrim?$count=").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
   }
 
   @Test
@@ -2558,16 +2572,22 @@ public class TestFullResourcePath {
     testUri.run("ESAllPrim?$skip=-3")
         .isKind(UriInfoKind.resource).goPath()
         .isSkipText("-3");
+    testUri.runEx("ESAllPrim?$skip=F").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
+    testUri.runEx("ESAllPrim?$skip=").isExSemantic(UriParserSemanticException.MessageKeys.TEST);
   }
 
   @Test
   public void skiptoken() throws Exception {
-
     testUri.run("ESAllPrim?$skiptoken=foo")
         .isKind(UriInfoKind.resource).goPath()
         .isSkipTokenText("foo");
   }
 
+  @Test
+  public void notExistingSystemQueryOption() throws Exception {
+    testUri.runEx("ESAllPrim?$wrong=error").isExSyntax(UriParserSyntaxException.MessageKeys.TEST);
+  }
+
   @Test
   public void misc() throws Exception {
 

lib/server-test/src/test/java/org/apache/olingo/server/core/uri/validator/UriValidatorTest.java

@@ -62,7 +62,7 @@ public class UriValidatorTest {
   private static final String URI_NAV_ENTITY_SET = "/ESKeyNav/NavPropertyETKeyNavMany";
 
   private static final String QO_FILTER = "$filter='1' eq '1'";
-  private static final String QO_FORMAT = "$format=bla";
+  private static final String QO_FORMAT = "$format=bla/bla";
   private static final String QO_EXPAND = "$expand=*";
   private static final String QO_ID = "$id=Products(0)";
   private static final String QO_COUNT = "$count=true";
@@ -381,7 +381,7 @@ public class UriValidatorTest {
       }
       uris.add(uri);
     }
-    return uris.toArray(new String[0]);
+    return uris.toArray(new String[uris.size()]);
   }
 
   private void parseAndValidate(final String uri, final HttpMethod method)