From commits-return-12457-archive-asf-public=cust-asf.ponee.io@olingo.apache.org Wed Apr 25 10:49:25 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id BAFFF180676 for ; Wed, 25 Apr 2018 10:49:24 +0200 (CEST) Received: (qmail 75170 invoked by uid 500); 25 Apr 2018 08:49:23 -0000 Mailing-List: contact commits-help@olingo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@olingo.apache.org Delivered-To: mailing list commits@olingo.apache.org Received: (qmail 75157 invoked by uid 99); 25 Apr 2018 08:49:23 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 25 Apr 2018 08:49:23 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 662DCE062A; Wed, 25 Apr 2018 08:49:23 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: ramyav@apache.org To: commits@olingo.apache.org Message-Id: <06c63767820c4e6ebd03d8b5cd2ef269@git.apache.org> X-Mailer: ASF-Git Admin Mailer 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 +0000 (UTC) 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 Authored: Wed Apr 25 14:19:15 2018 +0530 Committer: ramya vasanth 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 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 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 exceptionList = new ArrayList(); 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 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 acceptTypes = AcceptType.create("application/json;q=0.2,abc"); + List 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 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 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 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 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 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 getAcceptCharset(String acceptCharset) + throws ContentNegotiatorException, AcceptHeaderContentNegotiatorException { + List 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 } } }