Repository: olingo-odata4
Updated Branches:
  refs/heads/master edee782ea -> c81833d50


[OLINGO-1238]Code fixes w.r.t Accept and Accept charset headers


Project: http://git-wip-us.apache.org/repos/asf/olingo-odata4/repo
Commit: http://git-wip-us.apache.org/repos/asf/olingo-odata4/commit/c81833d5
Tree: http://git-wip-us.apache.org/repos/asf/olingo-odata4/tree/c81833d5
Diff: http://git-wip-us.apache.org/repos/asf/olingo-odata4/diff/c81833d5

Branch: refs/heads/master
Commit: c81833d50e9ab34a8b939584fc04f33eec8ba68b
Parents: edee782
Author: ramya vasanth <[email protected]>
Authored: Wed Apr 25 14:19:15 2018 +0530
Committer: ramya vasanth <[email protected]>
Committed: Wed Apr 25 14:19:15 2018 +0530

----------------------------------------------------------------------
 .../AcceptHeaderAcceptCharsetHeaderITCase.java  | 212 ++++++++++++++++++-
 .../commons/api/format/AcceptCharset.java       |  36 ++--
 .../olingo/commons/api/format/AcceptType.java   |  14 +-
 .../commons/api/format/AcceptCharsetTest.java   |  24 ++-
 .../commons/api/format/AcceptTypeTest.java      |   8 +-
 .../AcceptHeaderContentNegotiatorException.java |  14 +-
 .../olingo/server/core/ContentNegotiator.java   | 114 +++++++---
 .../server-core-exceptions-i18n.properties      |   3 +-
 .../server/core/ContentNegotiatorTest.java      |  40 +++-
 9 files changed, 391 insertions(+), 74 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/fit/src/test/java/org/apache/olingo/fit/tecsvc/http/AcceptHeaderAcceptCharsetHeaderITCase.java
----------------------------------------------------------------------
diff --git 
a/fit/src/test/java/org/apache/olingo/fit/tecsvc/http/AcceptHeaderAcceptCharsetHeaderITCase.java
 
b/fit/src/test/java/org/apache/olingo/fit/tecsvc/http/AcceptHeaderAcceptCharsetHeaderITCase.java
index b60ebb0..e7b971e 100644
--- 
a/fit/src/test/java/org/apache/olingo/fit/tecsvc/http/AcceptHeaderAcceptCharsetHeaderITCase.java
+++ 
b/fit/src/test/java/org/apache/olingo/fit/tecsvc/http/AcceptHeaderAcceptCharsetHeaderITCase.java
@@ -66,8 +66,8 @@ public class AcceptHeaderAcceptCharsetHeaderITCase extends 
AbstractBaseTestITCas
     assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
 
     final String content = IOUtils.toString(connection.getErrorStream());
-    assertTrue(content.contains("The content-type range 'abc' is not supported 
"
-        + "as value of the Accept header."));
+    assertTrue(content.contains("The content-type range ' abc' is not 
supported as "
+        + "value of the Accept header."));
   }
   
   @Test
@@ -135,7 +135,7 @@ public class AcceptHeaderAcceptCharsetHeaderITCase extends 
AbstractBaseTestITCas
     connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "abc");
     connection.connect();
 
-    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+    assertEquals(HttpStatusCode.NOT_ACCEPTABLE.getStatusCode(), 
connection.getResponseCode());
     
     final String content = IOUtils.toString(connection.getErrorStream());
     assertTrue(content.contains("The charset specified in Accept charset 
header 'abc' is not supported."));
@@ -230,7 +230,7 @@ public class AcceptHeaderAcceptCharsetHeaderITCase extends 
AbstractBaseTestITCas
     connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "abc");
     connection.connect();
 
-    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+    assertEquals(HttpStatusCode.NOT_ACCEPTABLE.getStatusCode(), 
connection.getResponseCode());
 
     final String content = IOUtils.toString(connection.getErrorStream());
     assertTrue(content.contains("The charset specified in Accept charset "
@@ -307,7 +307,7 @@ public class AcceptHeaderAcceptCharsetHeaderITCase extends 
AbstractBaseTestITCas
   }
   
   @Test
-  public void multipleValuesInAcceptHeader2() throws Exception {
+  public void multipleValuesInAcceptHeaderWithOneIncorrectValue() throws 
Exception {
     URL url = new URL(SERVICE_URI + "ESAllPrim");
 
     HttpURLConnection connection = (HttpURLConnection) url.openConnection();
@@ -316,13 +316,213 @@ public class AcceptHeaderAcceptCharsetHeaderITCase 
extends AbstractBaseTestITCas
         + "application/json;q=0.1,application/json;q=0.8,abc");
     connection.connect();
 
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The content-type range ' abc' is not 
supported as value of the Accept header."));
+  }
+  
+  @Test
+  public void multipleValuesInAcceptHeaderWithIncorrectParam() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT, "application/json,"
+        + "application/json;q=0.1,application/json;q=<1");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The content-type range 
'application/json;q=<1' is not "
+        + "supported as value of the Accept header."));
+  }
+  
+  @Test
+  public void multipleValuesInAcceptHeaderWithIncorrectCharset() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT, "application/json,"
+        + "application/json;q=0.1,application/json;charset=utf<8");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The charset specified in Accept header "
+        + "'application/json;charset=utf<8' is not supported."));
+  }
+  
+  @Test
+  public void acceptHeaderWithIncorrectParams() throws Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT, 
"application/json;abc=xyz");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.NOT_ACCEPTABLE.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The content-type range 
'application/json;abc=xyz' is not "
+        + "supported as value of the Accept header."));
+  }
+  
+  @Test
+  public void formatWithIllegalCharset1() throws Exception {
+    URL url = new URL(SERVICE_URI + 
"ESAllPrim?$format=application/json;charset=abc");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.connect();
+
+    assertEquals(HttpStatusCode.NOT_ACCEPTABLE.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The $format option 
'application/json;charset=abc' is not supported."));
+  }
+  
+  @Test
+  public void formatWithWrongParams() throws Exception {
+    URL url = new URL(SERVICE_URI + 
"ESAllPrim?$format=application/json;abc=xyz");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.connect();
+
+    assertEquals(HttpStatusCode.NOT_ACCEPTABLE.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The $format option 'application/json;abc=xyz' 
is not supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectParamsInAcceptCharsetHeader() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "utf-8;abc=xyz");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf-8;abc=xyz' is 
not supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectAcceptCharsetHeader() throws Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "utf<8");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf<8' is not 
supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectMultipleAcceptCharsetHeader1() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, 
"utf<8,utf-8;q=0.8");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf<8' is not 
supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectMultipleAcceptCharsetHeader2() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, 
"utf-8;q=0.8,utf-8;q=<");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf-8;q=<' is not 
supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectQParamInAcceptCharsetHeader() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "utf-8;q=<");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf-8;q=<' is not 
supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectParamInAcceptCharsetHeader() throws 
Exception {
+    URL url = new URL(SERVICE_URI + "ESAllPrim?$format=json");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "utf-8;abc=xyz");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The Accept charset header 'utf-8;abc=xyz' is 
not supported."));
+  }
+  
+  @Test
+  public void validFormatWithIncorrectCharset() throws Exception {
+    URL url = new URL(SERVICE_URI + 
"ESAllPrim?$format=application/json;charset=utf<8");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT, 
"application/json;charset=utf-8");
+    connection.connect();
+
+    assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), 
connection.getResponseCode());
+
+    final String content = IOUtils.toString(connection.getErrorStream());
+    assertTrue(content.contains("The $format option 
'application/json;charset=utf<8' is not supported."));
+  }
+  
+  @Test
+  public void formatWithAcceptCharset() throws Exception {
+    URL url = new URL(SERVICE_URI + 
"ESAllPrim?$format=application/json;charset=utf-8");
+
+    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
+    connection.setRequestMethod(HttpMethod.GET.name());
+    connection.setRequestProperty(HttpHeader.ACCEPT_CHARSET, "abc");
+    connection.connect();
+
     assertEquals(HttpStatusCode.OK.getStatusCode(), 
connection.getResponseCode());
+
     assertNotNull(connection.getHeaderField(HttpHeader.CONTENT_TYPE));
     ContentType contentType = 
ContentType.parse(connection.getHeaderField(HttpHeader.CONTENT_TYPE));
     assertEquals("application", contentType.getType());
     assertEquals("json", contentType.getSubtype());
-    assertEquals(1, contentType.getParameters().size());
+    assertEquals(2, contentType.getParameters().size());
     assertEquals("minimal", contentType.getParameter("odata.metadata"));
+    assertEquals("utf-8", contentType.getParameter("charset"));
 
     final String content = IOUtils.toString(connection.getInputStream());
     assertNotNull(content);

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptCharset.java
----------------------------------------------------------------------
diff --git 
a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptCharset.java
 
b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptCharset.java
index 7d19245..8e5a266 100644
--- 
a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptCharset.java
+++ 
b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptCharset.java
@@ -18,7 +18,6 @@
  */
 package org.apache.olingo.commons.api.format;
 
-import java.nio.charset.Charset;
 import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -26,11 +25,13 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.regex.Pattern;
 
 public class AcceptCharset {
 
   private static final Pattern Q_PATTERN = 
Pattern.compile("\\A(?:0(?:\\.\\d{0,3})?)|(?:1(?:\\.0{0,3})?)\\Z");
+  private static final Pattern CHARSET_PATTERN = 
Pattern.compile("([^,][\\w!#$%&'*+-._`|~;^]*)");
   private final String charset;
   private final Map<String, String> parameters;
   private final Float quality;
@@ -41,17 +42,17 @@ public class AcceptCharset {
     parameters = TypeUtil.createParameterMap();
     this.charset = parse(charset, parameters);
     
-    if (TypeUtil.MEDIA_TYPE_WILDCARD.equals(this.charset)) {
-      throw new IllegalArgumentException("Unsupported charset in accept 
charset header:" + this.charset);
-    } else {
-      try {
-        Charset.forName(this.charset);
-      } catch (UnsupportedCharsetException e) {
-        throw new UnsupportedCharsetException("Illegal charset in accept 
charset header:" + this.charset);
+    if (!(UTF8_CHARSET.equalsIgnoreCase(this.charset)) && 
+        !(UTF8_CHARSET1.equalsIgnoreCase(this.charset)) && 
!(TypeUtil.MEDIA_TYPE_WILDCARD.equals(this.charset))) {
+      if (CHARSET_PATTERN.matcher(this.charset).matches()) {
+        throw new UnsupportedCharsetException("Unsupported charset in accept 
charset header:" + charset);
+      } else {
+        throw new IllegalArgumentException("Illegal charset in accept charset 
header:" + charset);
       }
-      if (!(UTF8_CHARSET.equalsIgnoreCase(this.charset)) && 
-          !(UTF8_CHARSET1.equalsIgnoreCase(this.charset))) {
-        throw new IllegalArgumentException("Unsupported charset in accept 
charset header:" + this.charset);
+    }
+    for (Entry<String, String> param : parameters.entrySet()) {
+      if (!param.getKey().equals(TypeUtil.PARAMETER_Q)) {
+        throw new IllegalArgumentException("Illegal parameters in accept 
charset header:" + charset);
       }
     }
     final String q = parameters.get(TypeUtil.PARAMETER_Q);
@@ -60,7 +61,7 @@ public class AcceptCharset {
     } else if (Q_PATTERN.matcher(q).matches()) {
         quality = Float.valueOf(q);
     } else {
-      throw new IllegalArgumentException("Illegal quality parameter '" + q + 
"'.");
+      throw new IllegalArgumentException("Illegal quality parameter '" + q + 
"' in accept charset header:" + charset);
     }
   }
 
@@ -87,6 +88,10 @@ public class AcceptCharset {
     List<Exception> exceptionList = new ArrayList<Exception>();
 
     String[] values = acceptCharsets.split(",");
+    if (values.length == 0) {
+      values = new String[1];
+      values[0] = acceptCharsets;
+    }
     for (String value : values) {
       try {
         result.add(new AcceptCharset(value.trim()));
@@ -104,6 +109,13 @@ public class AcceptCharset {
         throw new IllegalArgumentException(exceptionList.get(0).getMessage());
       }
     }
+    for (Exception ex : exceptionList) {
+      if (ex instanceof UnsupportedCharsetException) {
+        continue;
+      } else {
+        throw new IllegalArgumentException(ex.getMessage());
+      }
+    }
     sort(result);
     
     return result;

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java
----------------------------------------------------------------------
diff --git 
a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java
 
b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java
index d2a1b24..509e169 100644
--- 
a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java
+++ 
b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java
@@ -71,7 +71,8 @@ public final class AcceptType {
     subtype = typeSubtype.get(1);
 
     if (TypeUtil.MEDIA_TYPE_WILDCARD.equals(this.type) && 
!TypeUtil.MEDIA_TYPE_WILDCARD.equals(subtype)) {
-      throw new IllegalArgumentException("Illegal combination of WILDCARD type 
with NONE WILDCARD subtype.");
+      throw new IllegalArgumentException("Illegal combination of WILDCARD type 
with NONE WILDCARD "
+          + "subtype in accept header:" + type);
     }
 
     final String q = parameters.get(TypeUtil.PARAMETER_Q);
@@ -80,7 +81,7 @@ public final class AcceptType {
     } else if (Q_PATTERN.matcher(q).matches()) {
         quality = Float.valueOf(q);
     } else {
-      throw new IllegalArgumentException("Illegal quality parameter '" + q + 
"'.");
+      throw new IllegalArgumentException("Illegal quality parameter '" + q + 
"' in accept header:" + type);
     }
   }
 
@@ -94,16 +95,16 @@ public final class AcceptType {
     String[] tokens = types.split(TypeUtil.TYPE_SUBTYPE_SEPARATOR);
     if (tokens.length == 2) {
       if (tokens[0] == null || tokens[0].isEmpty()) {
-        throw new IllegalArgumentException("No type found in format '" + 
format + "'.");
+        throw new IllegalArgumentException("No type found in format: '" + 
format + "'.");
       } else if (tokens[1] == null || tokens[1].isEmpty()) {
-        throw new IllegalArgumentException("No subtype found in format '" + 
format + "'.");
+        throw new IllegalArgumentException("No subtype found in format: '" + 
format + "'.");
       } else {
         typeSubtype.add(tokens[0]);
         typeSubtype.add(tokens[1]);
       }
     } else {
       throw new IllegalArgumentException("Not exactly one '" + 
TypeUtil.TYPE_SUBTYPE_SEPARATOR +
-          "' in format '" + format + "', or it is at the beginning or at the 
end.");
+          " at the beginning or at the end in format: " + format);
     }
 
     TypeUtil.parseParameters(params, parameters);
@@ -131,9 +132,10 @@ public final class AcceptType {
       }
     }
 
-    if (result.isEmpty()) {
+    if (!exceptionList.isEmpty() || result.isEmpty()) {
       throw exceptionList.get(0);
     }
+    
     sort(result);
 
     return result;

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptCharsetTest.java
----------------------------------------------------------------------
diff --git 
a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptCharsetTest.java
 
b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptCharsetTest.java
index a963f66..8d7737a 100644
--- 
a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptCharsetTest.java
+++ 
b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptCharsetTest.java
@@ -30,7 +30,8 @@ import org.junit.Test;
 public class AcceptCharsetTest {
   @Test
   public void wildcard() {
-    expectCreateError("*");
+    List<AcceptCharset> charsets = AcceptCharset.create("*");
+    assertEquals("*", charsets.get(0).getCharset());
   }
   
   @Test
@@ -39,6 +40,27 @@ public class AcceptCharsetTest {
   }
   
   @Test
+  public void illegalCharsetWithDelimiters() {
+    expectCreateError("utf<8");
+    expectCreateError(",,,,");
+    expectCreateError(", , , ");
+    expectCreateError("utf 8");
+    expectCreateError("utf=8");
+    expectCreateError("utf-8;<");
+    expectCreateError("utf-8;q<");
+    expectCreateError("utf-8;q=<");
+    expectCreateError("utf-8;q=1<");
+    expectCreateError("utf-8;abc=xyz");
+    expectCreateError("utf-8;");
+    expectCreateError("utf-8;q='");
+    expectCreateError("utf-8;q=0.1, utf8;q=0.8, iso-8859-1, abc, xyz<");
+    expectCreateError("utf-8;q=0.1, utf8;q=0.8<, iso-8859-1, abc");
+    expectCreateError("utf-8;abc=xyz");
+    expectCreateError("utf-8;q=0.8;abc=xyz");
+    expectCreateError("utf-8;q=xyz");
+  }
+  
+  @Test
   public void unsupportedCharset() {
     expectCreateError("iso-8859-1");
   }

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java
----------------------------------------------------------------------
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 8fe0af1..2f0a299 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
@@ -139,6 +139,8 @@ public class AcceptTypeTest {
     expectCreateError(" a/a;q=z ");
     expectCreateError("a/a;q=42");
     expectCreateError("a/a;q=0.0001");
+    expectCreateError("a/a;q='");
+    expectCreateError("a/a;q=0.8,abc");
   }
 
   @Test
@@ -188,10 +190,10 @@ public class AcceptTypeTest {
   
   @Test
   public void multipleTypeswithIllegalTypes() {
-    List<AcceptType> acceptTypes = 
AcceptType.create("application/json;q=0.2,abc");
+    List<AcceptType> acceptTypes = 
AcceptType.create("application/json;q=0.2,abc/xyz");
 
-    assertEquals(1, acceptTypes.size());
-    final AcceptType acceptType = acceptTypes.get(0);
+    assertEquals(2, acceptTypes.size());
+    final AcceptType acceptType = acceptTypes.get(1);
     assertEquals("application", acceptType.getType());
     assertEquals("json", acceptType.getSubtype());
     assertEquals("0.2", acceptType.getParameters().get(TypeUtil.PARAMETER_Q));

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/server-core/src/main/java/org/apache/olingo/server/core/AcceptHeaderContentNegotiatorException.java
----------------------------------------------------------------------
diff --git 
a/lib/server-core/src/main/java/org/apache/olingo/server/core/AcceptHeaderContentNegotiatorException.java
 
b/lib/server-core/src/main/java/org/apache/olingo/server/core/AcceptHeaderContentNegotiatorException.java
index 280da32..412eb5b 100644
--- 
a/lib/server-core/src/main/java/org/apache/olingo/server/core/AcceptHeaderContentNegotiatorException.java
+++ 
b/lib/server-core/src/main/java/org/apache/olingo/server/core/AcceptHeaderContentNegotiatorException.java
@@ -18,6 +18,8 @@
  */
 package org.apache.olingo.server.core;
 
+import org.apache.olingo.server.api.ODataLibraryException.MessageKey;
+
 public class AcceptHeaderContentNegotiatorException extends 
ContentNegotiatorException {
   private static final long serialVersionUID = -8112658467394158700L;
 
@@ -27,8 +29,10 @@ public class AcceptHeaderContentNegotiatorException extends 
ContentNegotiatorExc
     /** parameter: format string */
     UNSUPPORTED_FORMAT_OPTION,
     /** parameter: accept charset header*/
-     UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS;
-
+     UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS,
+     /** parameter: charset option in accept header*/
+     UNSUPPORTED_ACCEPT_HEADER_CHARSET;
+    
     @Override
     public String getKey() {
       return name();
@@ -39,6 +43,12 @@ public class AcceptHeaderContentNegotiatorException extends 
ContentNegotiatorExc
       final String... parameters) {
     super(developmentMessage, messageKey, parameters);
   }
+  
+  public AcceptHeaderContentNegotiatorException(final String 
developmentMessage, final Throwable cause,
+      final MessageKey messageKey,
+      final String... parameters) {
+    super(developmentMessage, cause, messageKey, parameters);
+  }
 
   @Override
   protected String getBundleName() {

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java
----------------------------------------------------------------------
diff --git 
a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java
 
b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java
index a451088..a598308 100644
--- 
a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java
+++ 
b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java
@@ -20,10 +20,11 @@ package org.apache.olingo.server.core;
 
 import java.nio.charset.Charset;
 import java.nio.charset.UnsupportedCharsetException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.apache.olingo.commons.api.format.AcceptCharset;
 import org.apache.olingo.commons.api.format.AcceptType;
@@ -42,6 +43,7 @@ public final class ContentNegotiator {
   private static final String XML = "xml";
   private static final String METADATA = "METADATA";
   private static final String COLON = ":";
+  private static final Pattern CHARSET_PATTERN = 
Pattern.compile("([^,][\\w!#$%&'*+-._`|~;^]*)");
 
   private static final List<ContentType> DEFAULT_SUPPORTED_CONTENT_TYPES =
       Collections.unmodifiableList(Arrays.asList(
@@ -94,48 +96,60 @@ public final class ContentNegotiator {
     final String acceptHeaderValue = request.getHeader(HttpHeader.ACCEPT);
     String acceptCharset = request.getHeader(HttpHeader.ACCEPT_CHARSET);
     List<AcceptCharset> charsets = null;
-    if (acceptCharset != null) {
-      try {
-        charsets = AcceptCharset.create(acceptCharset); 
-      } catch (UnsupportedCharsetException e) {
-        throw new AcceptHeaderContentNegotiatorException(e.getMessage(),
-            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS,
 
-            e.getMessage().substring(e.getMessage().lastIndexOf(COLON) + 1));
-      } catch (IllegalArgumentException e) {
-        throw new ContentNegotiatorException(e.getMessage(),
-            ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_CHARSET, 
-            e.getMessage().substring(e.getMessage().lastIndexOf(COLON) + 1));
-      }
-    }
       
     ContentType result = null;
 
     if (formatOption != null && formatOption.getFormat() != null) {
       final String formatString = formatOption.getFormat().trim();
       final ContentType contentType = mapContentType(formatString, 
representationType);
-
+      boolean isCharsetInFormat = false;
+      List<AcceptType> formatTypes = null;
+      try {
+      formatTypes = AcceptType.fromContentType(contentType == null ?
+          ContentType.create(formatOption.getFormat()) : contentType);
+      } catch (final IllegalArgumentException e) {
+        throw new AcceptHeaderContentNegotiatorException(
+            "Unsupported $format=" + formatString, e,
+            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, 
formatString);
+      }
+      Map<String, String> formatParameters = 
formatTypes.get(0).getParameters();
+      if (!formatParameters.isEmpty() && null != 
formatParameters.get(ContentType.PARAMETER_CHARSET)) {
+        isCharsetInFormat = true;
+      } else {
+        isCharsetInFormat = false;
+        charsets = getAcceptCharset(acceptCharset);
+      }
       try {
-        result = getAcceptedType(
-            AcceptType.fromContentType(contentType == null ?
-                ContentType.create(formatOption.getFormat()) : contentType),
-                supportedContentTypes, charsets);
+        if (isCharsetInFormat) {
+          charsets = 
getAcceptCharset(formatParameters.get(ContentType.PARAMETER_CHARSET));
+        }
+        result = getAcceptedType(formatTypes, supportedContentTypes, charsets);
       } catch (final IllegalArgumentException e) {
         throw new AcceptHeaderContentNegotiatorException(
-            "Unsupported $format=" + formatString,
+            "Unsupported $format=" + formatString, e,
+            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, 
formatString);
+      } catch (final AcceptHeaderContentNegotiatorException e) {
+        throw new AcceptHeaderContentNegotiatorException (
+            "Unsupported $format=" + formatString, e,
             
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, 
formatString);
+      } catch (final ContentNegotiatorException e) {
+        throw new ContentNegotiatorException (
+            "Unsupported $format=" + formatString, e, 
+            ContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, 
formatString);
       }
       if (result == null) {
         throw new ContentNegotiatorException("Unsupported $format = " + 
formatString,
             ContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, 
formatString);
       }
     } else if (acceptHeaderValue != null) {
+      charsets = getAcceptCharset(acceptCharset);
       try {
         result = getAcceptedType(AcceptType.create(acceptHeaderValue), 
             supportedContentTypes, charsets);
       } catch (final IllegalArgumentException e) {
-        throw new AcceptHeaderContentNegotiatorException(
-            "Unsupported or illegal Accept header value: " + acceptHeaderValue 
+ " != " + supportedContentTypes,
-            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_TYPES, 
acceptHeaderValue);
+        throw new AcceptHeaderContentNegotiatorException(e.getMessage(), e,
+            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_TYPES, 
+            e.getMessage().substring(e.getMessage().lastIndexOf(COLON) + 1));
       } 
       if (result == null) {
         List<AcceptType> types = AcceptType.create(acceptHeaderValue);
@@ -145,6 +159,7 @@ public final class ContentNegotiator {
             ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_TYPES, 
acceptHeaderValue);
       }
     } else {
+      charsets = getAcceptCharset(acceptCharset);
       final ContentType requestedContentType = 
getDefaultSupportedContentTypes(representationType).get(0);
       result = 
getAcceptedType(AcceptType.fromContentType(requestedContentType), 
           supportedContentTypes, charsets);
@@ -158,6 +173,31 @@ public final class ContentNegotiator {
     }
     return result;
   }
+  
+  /**
+   * @param acceptCharset
+   * @return
+   * @throws ContentNegotiatorException
+   * @throws AcceptHeaderContentNegotiatorException
+   */
+  private static List<AcceptCharset> getAcceptCharset(String acceptCharset)
+      throws ContentNegotiatorException, 
AcceptHeaderContentNegotiatorException {
+    List<AcceptCharset> charsets = null;
+    if (acceptCharset != null) {
+      try {
+        charsets = AcceptCharset.create(acceptCharset); 
+      } catch (UnsupportedCharsetException e) {
+        throw new ContentNegotiatorException(e.getMessage(), e,
+            ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_CHARSET, 
+            e.getMessage().substring(e.getMessage().lastIndexOf(COLON) + 1));
+      } catch (IllegalArgumentException e) {
+        throw new AcceptHeaderContentNegotiatorException(e.getMessage(), e, 
+            
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS,
 
+            e.getMessage().substring(e.getMessage().lastIndexOf(COLON) + 1));
+      }
+    }
+    return charsets;
+  }
 
   private static ContentType mapContentType(final String formatString, 
       RepresentationType representationType) {
@@ -193,21 +233,24 @@ public final class ContentNegotiator {
         ContentType contentType = supportedContentType;
         final String charSetValue = 
acceptedType.getParameter(ContentType.PARAMETER_CHARSET);
         if (charset != null) {
-          contentType = ContentType.create(contentType, 
ContentType.PARAMETER_CHARSET, charset.toString());
-        } else if (charSetValue != null) {
-          try {
-            Charset.forName(charSetValue);
-          } catch (UnsupportedCharsetException e) {
-            throw new AcceptHeaderContentNegotiatorException(
-                "Illegal charset in Accept header: " + charSetValue,
-                
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS,
 
-                charSetValue);
+          if ("*".equals(charset.toString())) {
+            contentType = ContentType.create(contentType, 
ContentType.PARAMETER_CHARSET, "utf-8");
+          } else {
+            contentType = ContentType.create(contentType, 
ContentType.PARAMETER_CHARSET, charset.toString());
           }
+        } else if (charSetValue != null) {
           if ("utf8".equalsIgnoreCase(charSetValue) || 
"utf-8".equalsIgnoreCase(charSetValue)) {
             contentType = ContentType.create(contentType, 
ContentType.PARAMETER_CHARSET, "utf-8");
           } else {
-            throw new ContentNegotiatorException("Unsupported 
accept-header-charset = " + charSetValue,
-                
ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_HEADER_CHARSET, 
acceptedType.toString());
+            if (CHARSET_PATTERN.matcher(charSetValue).matches()) {
+              throw new ContentNegotiatorException("Unsupported 
accept-header-charset = " + charSetValue,
+                  
ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_HEADER_CHARSET, 
acceptedType.toString());
+            } else {
+              throw new AcceptHeaderContentNegotiatorException(
+                  "Illegal charset in Accept header: " + charSetValue,
+                  
AcceptHeaderContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_HEADER_CHARSET,
 
+                  acceptedType.toString());
+            }
           }
         }
 
@@ -217,7 +260,8 @@ public final class ContentNegotiator {
         } else if ("false".equalsIgnoreCase(ieee754compatibleValue)) {
           contentType = ContentType.create(contentType, 
ContentType.PARAMETER_IEEE754_COMPATIBLE, "false");
         } else if (ieee754compatibleValue != null) {
-          throw new IllegalArgumentException("Invalid IEEE754Compatible value 
" + ieee754compatibleValue);
+          throw new IllegalArgumentException("Invalid IEEE754Compatible value 
in accept header:" + 
+              acceptedType.toString());
         }
 
         if (acceptedType.matches(contentType)) {

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties
----------------------------------------------------------------------
diff --git 
a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties 
b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties
index 19ae702..f9aa128 100644
--- a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties
+++ b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties
@@ -108,7 +108,8 @@ 
ContentNegotiatorException.UNSUPPORTED_ACCEPT_HEADER_CHARSET=The charset specifi
 
 AcceptHeaderContentNegotiatorException.UNSUPPORTED_ACCEPT_TYPES=The 
content-type range '%1$s' is not supported as value of the Accept header.
 AcceptHeaderContentNegotiatorException.UNSUPPORTED_FORMAT_OPTION=The $format 
option '%1$s' is not supported.
-AcceptHeaderContentNegotiatorException.UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS=The
 charset specified in Accept charset header '%1$s' is not supported.
+AcceptHeaderContentNegotiatorException.UNSUPPORTED_ACCEPT_CHARSET_HEADER_OPTIONS=The
 Accept charset header '%1$s' is not supported.
+AcceptHeaderContentNegotiatorException.UNSUPPORTED_ACCEPT_HEADER_CHARSET=The 
charset specified in Accept header '%1$s' is not supported.
 
 SerializerException.NULL_METADATA_OR_EDM=The server does not define any 
service metadata.
 SerializerException.NOT_IMPLEMENTED=The requested serialization method has not 
been implemented yet.

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/c81833d5/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java
----------------------------------------------------------------------
diff --git 
a/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java
 
b/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java
index 46ec985..8e91cd1 100644
--- 
a/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java
+++ 
b/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java
@@ -125,7 +125,11 @@ public class ContentNegotiatorTest {
       { null,                   null,             "a/b;charset=ISO-8859-1", 
"a/b"         },
       { null,                   null,             null,                  
"text/plain"     },
          { null,                   "xxx",            null,                  
null             },
-      { null,                   null,             
ACCEPT_CASE_MIN_IEEE754_FAIL,null       }
+      { null,                   null,             
ACCEPT_CASE_MIN_IEEE754_FAIL,null       },
+      { null,                   null,             
"application/json;charset=utf<8",null   },
+      { null,                   null,             
"application/json;charset=utf-8;q=<",null},
+      { null,                   null,             
"application/json;charset=utf-8,application/json;q=1<",null},
+      { null,                   null,             
"application/json;charset=utf-8,abc",null}
   };
   
   String[][] casesAcceptCharset = {
@@ -136,16 +140,32 @@ public class ContentNegotiatorTest {
       { ACCEPT_CASE_MIN_UTF81,   null,            ACCEPT_CASE_ISO_8859_1, 
null,                    "utf-8"    },
       { ACCEPT_CASE_MIN_UTF81,   null,           
"application/json;charset=abc", null,             "utf-8"    },
       { ACCEPT_CASE_MIN_UTF8,   null,            
"application/json;charset=utf-8", null,              null    },
-      { ACCEPT_CASE_MIN_UTF8,   null,            
"application/json;charset=utf8", null,              null    }
+      { ACCEPT_CASE_MIN_UTF8,   null,            
"application/json;charset=utf8", null,              null    },
+      { ACCEPT_CASE_MIN_UTF8,   null,            
"application/json;charset=utf8;q=0.8", null,        null    }
   };
   
   String[][] casesAcceptCharsetFail = {
-      /* expected               $format           accept                 
modified content types  acceptCharset*/
-      { null,                   null,             null,                   
null,                    "ISO-8859-1"},
-      { null,                   "json",           ACCEPT_CASE_MIN_UTF8,   
null,                    "abc"       },
-      { null,                   null,             ACCEPT_CASE_ISO_8859_1, 
null,                    "*"         },
-      { null,                   null,             ACCEPT_CASE_ISO_8859_1, 
null,                     null       },
-      { null,                   null,             
"application/json;charset=abc", null,             null       }
+      /* expected               $format           accept                 
modified content types   acceptCharset*/
+      { null,                   null,             null,                   
null,                     "ISO-8859-1" },
+      { null,                   "json",           ACCEPT_CASE_MIN_UTF8,   
null,                     "abc"        },
+      { null,                   null,             ACCEPT_CASE_ISO_8859_1, 
null,                     "utf<8"      },
+      { null,                   null,             ACCEPT_CASE_MIN_UTF8, null,  
                     "utf-8;abc=xyz"},
+      { null,                   null,             ACCEPT_CASE_MIN_UTF8, null,  
                     "utf-8;q=1<"  },
+      { null,                   null,             ACCEPT_CASE_MIN_UTF8, null,  
                      "utf-8;<"   },
+      { null,                   null,             ACCEPT_CASE_ISO_8859_1, 
null,                       null       },
+      { null,                   null,             
"application/json;charset=abc", null,               null       },
+      { null,                   null,             
"application/json;charset=utf-8;q", null,           null       },
+      { null,                   null,             
"application/json;charset=utf-8;abc=xyz", null,     null       },
+      { null,                   null,             
"application/json;charset=utf-8;q<", null,          null       },
+      { null,                   null,             
"application/json;charset=utf<8", null,             null       },
+      { null,                  "json;charset=abc",ACCEPT_CASE_MIN_UTF8,        
     null,             null       },
+      { null,                  "json;charset=utf<8",ACCEPT_CASE_MIN_UTF8,      
     null,             null       },
+      { null,                  
"json;charset=utf-8;abc=xyz",ACCEPT_CASE_MIN_UTF8,   null,             null     
  },
+      { null,                  "json;charset=utf-8;q=1<",ACCEPT_CASE_MIN_UTF8, 
     null,             null       },
+      { null,                  "json;charset=utf-8;q='",ACCEPT_CASE_MIN_UTF8,  
     null,             null       },
+      { null,                  
"application/json;abc=xyz",ACCEPT_CASE_MIN_UTF8,       null,           null     
  },
+      { null,                  
"application/json;charset=utf<8",ACCEPT_CASE_MIN_UTF8, null,           null     
  },
+      { null,                  
"application/json;charset=abc",ACCEPT_CASE_MIN_UTF8,   null,           null     
  }
   };
   
   //CHECKSTYLE:ON
@@ -193,6 +213,8 @@ public class ContentNegotiatorTest {
         // Expected Exception
       } catch (final ContentNegotiatorException e) {
         // Expected Exception
+      } catch (final IllegalArgumentException e) {
+        // Expected Exception
       }
     }
   }
@@ -296,6 +318,8 @@ public class ContentNegotiatorTest {
         // Expected Exception
       } catch (final ContentNegotiatorException e) {
         // Expected Exception
+      } catch (final IllegalArgumentException e) {
+        // Expected Exception
       }
     }
   }

Reply via email to