more checks that media types are correctly formatted

Change-Id: Ia98148846fb48aad0830e069944beade5c719b47

Signed-off-by: Christian Amend <chrisam@apache.org>

lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java

@@ -39,6 +39,7 @@ import java.util.TreeMap;
  * type = token
  * subtype = token
  * OWS = *( SP / HTAB ) ; optional whitespace
+ * parameter = token "=" ( token / quoted-string )
  * </pre>
  *
  * Once created a {@link ContentType} is <b>IMMUTABLE</b>.
@@ -111,11 +112,8 @@ public final class ContentType {
     if (type == null || type.isEmpty() || "*".equals(type)) {
       throw new IllegalArgumentException("Illegal type '" + type + "'.");
     }
-    int len = type.length();
-    for (int i = 0; i < len; i++) {
-      if (type.charAt(i) == TypeUtil.WHITESPACE_CHAR) {
-        throw new IllegalArgumentException("Illegal whitespace found for type '" + type + "'.");
-      }
+    if (type.indexOf(TypeUtil.WHITESPACE_CHAR) >= 0) {
+      throw new IllegalArgumentException("Illegal whitespace found for type '" + type + "'.");
     }
     return type;
   }
@@ -145,7 +143,7 @@ public final class ContentType {
     ContentType ct = parse(format);
 
     for (String p : parameters) {
-      final String[] keyvalue = p.split("=");
+      final String[] keyvalue = TypeUtil.parseParameter(p);
       ct.parameters.put(keyvalue[0], keyvalue[1]);
     }
 
@@ -163,7 +161,7 @@ public final class ContentType {
     ContentType ct = new ContentType(contentType.type, contentType.subtype, contentType.parameters);
 
     for (String p : parameters) {
-      String[] keyvalue = p.split("=");
+      final String[] keyvalue = TypeUtil.parseParameter(p);
       ct.parameters.put(keyvalue[0], keyvalue[1]);
     }
 
@@ -380,7 +378,8 @@ public final class ContentType {
     sb.append(type).append(TypeUtil.TYPE_SUBTYPE_SEPARATOR).append(subtype);
 
     for (String key : parameters.keySet()) {
-      sb.append(";").append(key).append("=").append(parameters.get(key));
+      sb.append(TypeUtil.PARAMETER_SEPARATOR).append(key)
+          .append(TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR).append(parameters.get(key));
     }
     return sb.toString();
   }

lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java

@@ -47,20 +47,32 @@ class TypeUtil {
    * @param parameters
    * @param parameterMap
    */
-  static void parseParameters(final String parameters, final Map<String, String> parameterMap) {
+  protected static void parseParameters(final String parameters, Map<String, String> parameterMap) {
     if (parameters != null) {
-      String[] splittedParameters = parameters.split(TypeUtil.PARAMETER_SEPARATOR);
-      for (String parameter : splittedParameters) {
-        String[] keyValue = parameter.split(TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR);
-        String key = keyValue[0].trim().toLowerCase(Locale.ENGLISH);
-        String value = keyValue.length > 1 ? keyValue[1] : null;
-        if (value != null && Character.isWhitespace(value.charAt(0))) {
-          throw new IllegalArgumentException(
-              "Value of parameter '" + key + "' starts with whitespace ('" + parameters + "').");
-        }
-        parameterMap.put(key, value);
+      for (String parameter : parameters.split(TypeUtil.PARAMETER_SEPARATOR)) {
+        final String[] keyValue = parseParameter(parameter);
+        parameterMap.put(keyValue[0], keyValue[1]);
       }
     }
   }
 
+  protected static String[] parseParameter(final String parameter) {
+    if (parameter.isEmpty()) {
+      throw new IllegalArgumentException("An empty parameter is not allowed.");
+    }
+    String[] keyValue = parameter.trim().split(TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR);
+    if (keyValue.length != 2 || keyValue[0].isEmpty()) {
+      throw new IllegalArgumentException(
+          "Parameter '" + parameter + "' must have exactly one '" + TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR +
+          "' that separates the name and the value.");
+    }
+    keyValue[0] = keyValue[0].toLowerCase(Locale.ENGLISH);
+    if (keyValue[0].indexOf(WHITESPACE_CHAR) >= 0) {
+      throw new IllegalArgumentException("Parameter name '" + keyValue[0] + "' contains whitespace.");
+    }
+    if (Character.isWhitespace(keyValue[1].charAt(0))) {
+      throw new IllegalArgumentException("Value of parameter '" + keyValue[0] + "' starts with whitespace.");
+    }
+    return keyValue;
+  }
 }

lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java

@@ -120,4 +120,33 @@ public class AcceptTypeTest {
     AcceptType.create(" a/a;q=z ");
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void incompleteParameter() {
+    AcceptType.create("a/b;parameter");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void missingParameterValue() {
+    AcceptType.create("a/b;parameter=");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void parameterValueStartingWithWhitespace() {
+    AcceptType.create("a/b;name= value");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void missingParameterName() {
+    AcceptType.create("a/b;=value");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void parameterNameWithWhitespace() {
+    AcceptType.create("a/b;the name=value");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void trailingSemicolon() {
+    AcceptType.create("a/b;");
+  }
 }

lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java

@@ -46,6 +46,36 @@ public class ContentTypeTest {
     ContentType.create(" a / b ");
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail3() {
+    ContentType.create("a/b;");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail4() {
+    ContentType.create("a/b;parameter");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail5() {
+    ContentType.create("a/b;parameter=");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail6() {
+    ContentType.create("a/b;=value");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail7() {
+    ContentType.create("a/b;the name=value");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testCreateFail8() {
+    ContentType.create("a/b;name= value");
+  }
+
   @Test
   public void testCreateWithParameter() {
     assertEquals(ContentType.create("a/b;c=d"), ContentType.create("a/b", "c=d"));