From eacc63c672b36dc73d73a49785307bb72de43fd5 Mon Sep 17 00:00:00 2001 From: Klaus Straubinger Date: Thu, 2 Oct 2014 14:35:29 +0200 Subject: [PATCH] more checks that media types are correctly formatted Change-Id: Ia98148846fb48aad0830e069944beade5c719b47 Signed-off-by: Christian Amend --- .../commons/api/format/ContentType.java | 15 ++++---- .../olingo/commons/api/format/TypeUtil.java | 34 +++++++++++++------ .../commons/api/format/AcceptTypeTest.java | 29 ++++++++++++++++ .../commons/api/format/ContentTypeTest.java | 30 ++++++++++++++++ 4 files changed, 89 insertions(+), 19 deletions(-) diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java index 20620fcc1..60a1ce68b 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java +++ b/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 ) * * * Once created a {@link ContentType} is IMMUTABLE. @@ -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(); } diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java index 8dcf0ce74..004eaac81 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java +++ b/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 parameterMap) { + protected static void parseParameters(final String parameters, Map 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; + } } diff --git a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java index 242c81450..72c830d65 100644 --- a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java +++ b/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;"); + } } diff --git a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java index b1e4547e7..feae0f721 100644 --- a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java +++ 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"));