olingo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ram...@apache.org
Subject olingo-odata4 git commit: [OLINGO-1238]Code fixes w.r.t Accept and Accept charset headers
Date Wed, 25 Apr 2018 08:49:23 GMT
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 <ramya.vasanth@sap.com>
Authored: Wed Apr 25 14:19:15 2018 +0530
Committer: ramya vasanth <ramya.vasanth@sap.com>
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
       }
     }
   }


Mime
View raw message