You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@olingo.apache.org by mi...@apache.org on 2016/08/29 06:40:37 UTC

olingo-odata4 git commit: [OLINGO-1015] Should allow more characters in batch header names

Repository: olingo-odata4
Updated Branches:
  refs/heads/master 7a56dfa39 -> b380f97b7


[OLINGO-1015] Should allow more characters in batch header names

Change-Id: Ie99b5ad2343c1109322186989a6a7744204dc8df

Signed-off-by: Michael Bolz <mi...@sap.com>


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

Branch: refs/heads/master
Commit: b380f97b74b129b2ff39d17b6d2c77cd8f0550ee
Parents: 7a56dfa
Author: Klaus Straubinger <kl...@sap.com>
Authored: Fri Aug 26 14:46:03 2016 +0200
Committer: Michael Bolz <mi...@sap.com>
Committed: Mon Aug 29 08:13:28 2016 +0200

----------------------------------------------------------------------
 .../deserializer/batch/BatchParserCommon.java   |  65 +++++----
 .../batch/BatchParserCommonTest.java            | 133 +++++++++----------
 .../batch/HttpRequestStatusLineTest.java        |   4 +-
 3 files changed, 105 insertions(+), 97 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b380f97b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommon.java
----------------------------------------------------------------------
diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommon.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommon.java
index 055a56d..c9ae89e 100644
--- a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommon.java
+++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommon.java
@@ -24,7 +24,6 @@ import java.nio.charset.Charset;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -33,35 +32,50 @@ import org.apache.olingo.server.api.deserializer.batch.BatchDeserializerExceptio
 
 public class BatchParserCommon {
 
-  private static final String PATTERN_BOUNDARY =
-      "([a-zA-Z0-9_\\-\\.'\\+]{1,70})|"
-          + "\"([a-zA-Z0-9_\\-\\.'\\+\\s\\(\\),/:=\\?]{1,69}[a-zA-Z0-9_\\-\\.'\\+\\(\\),/:=\\?])\"";
+  // Multipart boundaries are defined in RFC 2046:
+  //     boundary      := 0*69<bchars> bcharsnospace
+  //     bchars        := bcharsnospace / " "
+  //     bcharsnospace := DIGIT / ALPHA / "'" / "(" / ")" / "+" / "_" / "," / "-" / "." / "/" / ":" / "=" / "?"
+  // The first alternative is for the case that only characters are used that don't need quoting.
+  private static final Pattern PATTERN_BOUNDARY = Pattern.compile(
+      "((?:\\w|[-.'+]){1,70})|"
+          + "\"((?:\\w|[-.'+(),/:=?]|\\s){0,69}(?:\\w|[-.'+(),/:=?]))\"");
   private static final Pattern PATTERN_LAST_CRLF = Pattern.compile("(.*)\\r\\n\\s*", Pattern.DOTALL);
-  private static final Pattern PATTERN_HEADER_LINE = Pattern.compile("([a-zA-Z\\-]+):\\s?(.*)\\s*");
+  // HTTP header fields are defined in RFC 7230:
+  //     header-field   = field-name ":" OWS field-value OWS
+  //     field-name     = token
+  //     field-value    = *( field-content / obs-fold )
+  //     field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
+  //     field-vchar    = VCHAR / obs-text
+  //     obs-fold       = CRLF 1*( SP / HTAB )
+  //     token          = 1*tchar
+  //     tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
+  //                      / DIGIT / ALPHA
+  // For the field-name the specification is followed strictly,
+  // but for the field-value the pattern currently accepts more than specified.
+  private static final Pattern PATTERN_HEADER_LINE = Pattern.compile("((?:\\w|[!#$%\\&'*+\\-.^`|~])+):\\s?(.*)\\s*");
 
   public static final String CONTENT_TRANSFER_ENCODING = "Content-Transfer-Encoding";
 
   protected static final String BOUNDARY = "boundary";
   public static final String BINARY_ENCODING = "binary";
 
-  private BatchParserCommon() { /* private ctor for helper class */}
+  private BatchParserCommon() { /* private constructor for helper class */ }
 
   public static String getBoundary(final String contentType, final int line) throws BatchDeserializerException {
     final ContentType type = parseContentType(contentType, ContentType.MULTIPART_MIXED, line);
-    final Map<String, String> parameters = type.getParameters();
-    for (final Map.Entry<String, String> entries : parameters.entrySet()) {
-      if (BOUNDARY.equalsIgnoreCase(entries.getKey())) {
-        final String boundary = entries.getValue().trim();
-        if (boundary.matches(PATTERN_BOUNDARY)) {
-          return trimQuotes(boundary);
-        } else {
-          throw new BatchDeserializerException("Invalid boundary format",
-              BatchDeserializerException.MessageKeys.INVALID_BOUNDARY, Integer.toString(line));
-        }
-      }
+    String boundary = type.getParameter(BOUNDARY);
+    if (boundary == null) {
+      throw new BatchDeserializerException("Missing boundary.",
+          BatchDeserializerException.MessageKeys.MISSING_BOUNDARY_DELIMITER, Integer.toString(line));
+    }
+    boundary = boundary.trim();
+    if (PATTERN_BOUNDARY.matcher(boundary).matches()) {
+      return trimQuotes(boundary);
+    } else {
+      throw new BatchDeserializerException("Invalid boundary format",
+          BatchDeserializerException.MessageKeys.INVALID_BOUNDARY, Integer.toString(line));
     }
-    throw new BatchDeserializerException("Missing boundary.",
-        BatchDeserializerException.MessageKeys.MISSING_BOUNDARY_DELIMITER, Integer.toString(line));
   }
 
   /**
@@ -77,17 +91,16 @@ public class BatchParserCommon {
    */
   public static ContentType parseContentType(final String contentType, final ContentType expected, final int line)
       throws BatchDeserializerException {
+    if (contentType == null) {
+      throw new BatchDeserializerException("Missing content type",
+          BatchDeserializerException.MessageKeys.MISSING_CONTENT_TYPE, Integer.toString(line));
+    }
     ContentType type;
     try {
       type = ContentType.create(contentType);
     } catch (final IllegalArgumentException e) {
-      if (contentType == null) {
-        throw new BatchDeserializerException("Missing content type", e,
-            BatchDeserializerException.MessageKeys.MISSING_CONTENT_TYPE, Integer.toString(line));
-      } else {
-        throw new BatchDeserializerException("Invalid content type.", e,
-            BatchDeserializerException.MessageKeys.INVALID_CONTENT_TYPE, Integer.toString(line));
-      }
+      throw new BatchDeserializerException("Invalid content type.", e,
+          BatchDeserializerException.MessageKeys.INVALID_CONTENT_TYPE, Integer.toString(line));
     }
     if (type.isCompatible(expected)) {
       return type;

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b380f97b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommonTest.java
----------------------------------------------------------------------
diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommonTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommonTest.java
index 276c5b2..6c27b19 100644
--- a/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommonTest.java
+++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/BatchParserCommonTest.java
@@ -19,7 +19,9 @@
 package org.apache.olingo.server.core.deserializer.batch;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -32,16 +34,12 @@ public class BatchParserCommonTest {
   private static final String CRLF = "\r\n";
 
   @Test
-  public void testMultipleHeader() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleHeaders() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Content-Id: 1" + CRLF,
         "Content-Id: 2" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> contentIdHeaders = header.getHeaders(HttpHeader.CONTENT_ID);
@@ -52,16 +50,12 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testMultipleHeaderSameValue() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleHeadersSameValue() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Content-Id: 1" + CRLF,
         "Content-Id: 1" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> contentIdHeaders = header.getHeaders(HttpHeader.CONTENT_ID);
@@ -71,16 +65,12 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testHeaderSperatedByComma() throws Exception {
-    String[] messageRaw = new String[] {
+  public void headersSeparatedByComma() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Content-Id: 1" + CRLF,
         "Upgrade: HTTP/2.0, SHTTP/1.3, IRC/6.9, RTA/x11" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> upgradeHeader = header.getHeaders("upgrade");
@@ -93,17 +83,13 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testMultipleAcceptHeader() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleAcceptHeaders() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Accept: application/atomsvc+xml;q=0.8, application/json;odata=verbose;q=0.5, */*;q=0.1" + CRLF,
         "Accept: text/plain;q=0.3" + CRLF,
         "Accept-Language:en-US,en;q=0.7,en-UK;q=0.9" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> acceptHeader = header.getHeaders(HttpHeader.ACCEPT);
@@ -112,17 +98,13 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testMultipleAcceptHeaderSameValue() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleAcceptHeadersSameValue() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Accept: application/atomsvc+xml;q=0.8, application/json;odata=verbose;q=0.5, */*;q=0.1" + CRLF,
         "Accept: application/atomsvc+xml;q=0.8" + CRLF,
         "Accept-Language:en-US,en;q=0.7,en-UK;q=0.9" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> acceptHeader = header.getHeaders(HttpHeader.ACCEPT);
@@ -131,16 +113,12 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testMultipleAccepLanguagetHeader() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleAcceptLanguageHeaders() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Accept-Language:en-US,en;q=0.7,en-UK;q=0.9" + CRLF,
         "Accept-Language: de-DE;q=0.3" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> acceptLanguageHeader = header.getHeaders(HttpHeader.ACCEPT_LANGUAGE);
@@ -149,16 +127,12 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testMultipleAccepLanguagetHeaderSameValue() throws Exception {
-    String[] messageRaw = new String[] {
+  public void multipleAcceptLanguageHeadersSameValue() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
         "Accept-Language:en-US,en;q=0.7,en-UK;q=0.9" + CRLF,
         "Accept-Language:en-US,en;q=0.7" + CRLF,
         "content-type: Application/http" + CRLF,
-        "content-transfer-encoding: Binary" + CRLF
-    };
-    List<Line> message = toLineList(messageRaw);
-
-    final Header header = BatchParserCommon.consumeHeaders(message);
+        "content-transfer-encoding: Binary" + CRLF));
     assertNotNull(header);
 
     final List<String> acceptLanguageHeader = header.getHeaders(HttpHeader.ACCEPT_LANGUAGE);
@@ -167,54 +141,75 @@ public class BatchParserCommonTest {
   }
 
   @Test
-  public void testRemoveEndingCRLF() {
-    String line = "Test\r\n";
+  public void headersWithSpecialNames() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList(
+        "Test0123456789: 42" + CRLF,
+        "a_b: c/d" + CRLF,
+        "!#$%&'*+-.^_`|~: weird" + CRLF));
+    assertNotNull(header);
+    assertTrue(header.exists("Test0123456789"));
+    assertTrue(header.exists("a_b"));
+    assertTrue(header.exists("!#$%&'*+-.^_`|~"));
+    assertEquals("weird", header.getHeader("!#$%&'*+-.^_`|~"));
+  }
+
+  @Test
+  public void headerWithWrongName() throws Exception {
+    final Header header = BatchParserCommon.consumeHeaders(toLineList("a,b: c/d" + CRLF));
+    assertNotNull(header);
+    assertFalse(header.iterator().hasNext());
+  }
+
+  @Test
+  public void removeEndingCRLF() {
+    String line = "Test" + CRLF;
     assertEquals("Test", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveLastEndingCRLF() {
-    String line = "Test\r\n\r\n";
-    assertEquals("Test\r\n", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
+  public void removeLastEndingCRLF() {
+    String line = "Test" + CRLF + CRLF;
+    assertEquals("Test" + CRLF, BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveEndingCRLFWithWS() {
-    String line = "Test\r\n            ";
+  public void removeEndingCRLFWithWS() {
+    String line = "Test" + CRLF + "            ";
     assertEquals("Test", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveEndingCRLFNothingToRemove() {
-    String line = "Hallo\r\nBla";
-    assertEquals("Hallo\r\nBla", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
+  public void removeEndingCRLFNothingToRemove() {
+    String line = "Hallo" + CRLF + "Bla";
+    assertEquals(line, BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveEndingCRLFAll() {
-    String line = "\r\n";
+  public void removeEndingCRLFAll() {
+    String line = CRLF;
     assertEquals("", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveEndingCRLFSpace() {
-    String line = "\r\n                      ";
+  public void removeEndingCRLFSpace() {
+    String line = CRLF + "                      ";
     assertEquals("", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveLastEndingCRLFWithWS() {
-    String line = "Test            \r\n";
+  public void removeLastEndingCRLFWithWS() {
+    String line = "Test            " + CRLF;
     assertEquals("Test            ", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
   @Test
-  public void testRemoveLastEndingCRLFWithWSLong() {
-    String line = "Test            \r\nTest2    \r\n";
-    assertEquals("Test            \r\nTest2    ", BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
+  public void removeLastEndingCRLFWithWSLong() {
+    String line = "Test            " + CRLF + "Test2    " + CRLF;
+    assertEquals("Test            " + CRLF + "Test2    ",
+        BatchParserCommon.removeEndingCRLF(new Line(line, 1)).toString());
   }
 
-  private List<Line> toLineList(final String[] messageRaw) {
+  private List<Line> toLineList(final String... messageRaw) {
     final List<Line> lineList = new ArrayList<Line>();
     int counter = 1;
 

http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b380f97b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/HttpRequestStatusLineTest.java
----------------------------------------------------------------------
diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/HttpRequestStatusLineTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/HttpRequestStatusLineTest.java
index eb87baa..86000ce 100644
--- a/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/HttpRequestStatusLineTest.java
+++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/batch/HttpRequestStatusLineTest.java
@@ -94,14 +94,14 @@ public class HttpRequestStatusLineTest {
   }
 
   HttpRequestStatusLine parse(final String uri) throws BatchDeserializerException {
-    Line statusline = new Line(HttpMethod.GET.toString().toUpperCase() + SPACE + uri + SPACE + HTTP_VERSION, 0);
+    Line statusline = new Line(HttpMethod.GET.name() + SPACE + uri + SPACE + HTTP_VERSION, 0);
     return new HttpRequestStatusLine(statusline, baseUri, serviceResolutionUri);
   }
 
   void parseFail(final String uri, final MessageKeys messageKey) {
     try {
       parse(uri);
-      fail("Expceted exception");
+      fail("Expected exception");
     } catch (BatchDeserializerException e) {
       assertEquals(messageKey, e.getMessageKey());
     }