Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1104217 - in /cxf/trunk:
 rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/
 systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/
Date: Tue, 17 May 2011 13:01:02 -0000
To: commits@cxf.apache.org
From: sergeyb@apache.org
Message-Id: <20110517130102.32FBB2388A64@eris.apache.org>

Author: sergeyb
Date: Tue May 17 13:01:01 2011
New Revision: 1104217

URL: http://svn.apache.org/viewvc?rev=1104217&view=rev
Log:
[CXF-3518] Better processing of response headers with quoted values, patch on behalf of Ka-Loc Fung applied

Modified:
    cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
    cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
    cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java

Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java?rev=1104217&r1=1104216&r2=1104217&view=diff
==============================================================================
--- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java (original)
+++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/client/AbstractClient.java Tue May 17 13:01:01 2011
@@ -26,6 +26,7 @@ import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URL;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Date;
@@ -340,9 +341,18 @@ public class AbstractClient implements C
                     continue;                    
                 }
                 for (String val : entry.getValue()) {
-                    boolean splitPossible = !(HttpHeaders.SET_COOKIE.equalsIgnoreCase(entry.getKey())
-                        && val.toUpperCase().contains(HttpHeaders.EXPIRES.toUpperCase()));
-                    String[] values = splitPossible ? val.split(",") : new String[]{val};
+                    String[] values;
+                    if (val == null || val.length() == 0) {
+                        values = new String[]{""};
+                    } else if (val.charAt(0) == '"' && val.charAt(val.length() - 1) == '"') {
+                        // if the value starts with a quote and ends with a quote, we do a best
+                        // effort attempt to determine what the individual values are.
+                        values = parseQuotedHeaderValue(val);
+                    } else {
+                        boolean splitPossible = !(HttpHeaders.SET_COOKIE.equalsIgnoreCase(entry.getKey())
+                            && val.toUpperCase().contains(HttpHeaders.EXPIRES.toUpperCase()));
+                        values = splitPossible ? val.split(",") : new String[]{val};
+                    }
                     for (String s : values) {
                         String theValue = s.trim();
                         if (theValue.length() > 0) {
@@ -524,7 +534,58 @@ public class AbstractClient implements C
             conn.setRequestProperty(entry.getKey(), b.toString());
         }
     }
-    
+
+    protected String[] parseQuotedHeaderValue(String originalValue) {
+        // this algorithm isn't perfect; see CXF-3518 for further discussion.
+        List<String> results = new ArrayList<String>();
+        char[] chars = originalValue.toCharArray();
+
+        int lastIndex = chars.length - 1;
+
+        boolean quote = false;
+        StringBuilder sb = new StringBuilder();
+
+        for (int pos = 0; pos <= lastIndex; pos++) {
+            char c = chars[pos];
+            if (pos == lastIndex) {
+                sb.append(c);
+                results.add(sb.toString());
+            } else {
+                switch(c) {
+                case '\"':
+                    sb.append(c);
+                    quote = !quote;
+                    break;
+                case '\\':
+                    if (quote) {
+                        pos++;
+                        if (pos <= lastIndex) {
+                            c = chars[pos];
+                            sb.append(c);
+                        }
+                        if (pos == lastIndex) {
+                            results.add(sb.toString());
+                        }
+                    } else {
+                        sb.append(c);
+                    }
+                    break;
+                case ',':
+                    if (quote) {
+                        sb.append(c);
+                    } else {
+                        results.add(sb.toString());
+                        sb = new StringBuilder();
+                    }
+                    break;
+                default:
+                    sb.append(c);
+                }
+            }
+        }
+        return results.toArray(new String[results.size()]);
+    }
+
     protected ClientConfiguration getConfiguration() {
         return cfg;
     }

Modified: cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java
URL: http://svn.apache.org/viewvc/cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java?rev=1104217&r1=1104216&r2=1104217&view=diff
==============================================================================
--- cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java (original)
+++ cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookStore.java Tue May 17 13:01:01 2011
@@ -852,7 +852,50 @@ public class BookStore {
         c.setCD(cds.values());
         return c;
     }
-    
+
+    @GET
+    @Path("quotedheaders")
+    public Response getQuotedHeader() {
+        return Response.
+                ok().
+                header("SomeHeader1", "\"some text, some more text\"").
+                header("SomeHeader2", "\"some text\"").
+                header("SomeHeader2", "\"quoted,text\"").
+                header("SomeHeader2", "\"even more text\"").
+                header("SomeHeader3", "\"some text, some more text with inlined \\\"\"").
+                header("SomeHeader4", "\"\"").
+                build();
+    }
+
+    @GET
+    @Path("badlyquotedheaders")
+    public Response getBadlyQuotedHeader(@QueryParam("type")int t) {
+        Response.ResponseBuilder rb = Response.ok();
+        switch(t) {
+        case 0:
+            // problem: no trailing quote - doesn't trigger AbstractClient.parseQuotedHeaderValue
+            rb.header("SomeHeader0", "\"some text");
+            break;
+        case 1:
+            // problem: text doesn't end with " - triggers AbstractClient.parseQuotedHeaderValue
+            rb.header("SomeHeader1", "\"some text, some more text with inlined \\\"");
+            break;
+        case 2:
+            // problem: no character after \ - doesn't trigger AbstractClient.parseQuotedHeaderValue
+            rb.header("SomeHeader2", "\"some te\\");
+            break;
+        case 3:
+            // problem: mix of plain text and quoted text in same line - doesn't trigger
+            // AbstractClient.parseQuotedHeaderValue
+            rb.header("SomeHeader3", "some text").header("SomeHeader3", "\"other quoted\", text").
+                header("SomeHeader3", "blah");
+            break;
+        default:
+            throw new RuntimeException("Don't know how to handle type: " + t);
+        }
+        return rb.build();
+    }
+
     @Path("/interface")
     public BookSubresource getBookFromSubresource() {
         return new BookSubresourceImpl();

Modified: cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java
URL: http://svn.apache.org/viewvc/cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java?rev=1104217&r1=1104216&r2=1104217&view=diff
==============================================================================
--- cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java (original)
+++ cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRSClientServerBookTest.java Tue May 17 13:01:01 2011
@@ -66,7 +66,7 @@ public class JAXRSClientServerBookTest e
     @BeforeClass
     public static void startServers() throws Exception {
         assertTrue("server did not launch correctly",
-                   launchServer(BookServer.class, true));
+                   launchServer(BookServer.class));
     }
     
     @Test
@@ -164,6 +164,9 @@ public class JAXRSClientServerBookTest e
         assertTrue(headers.size() > 0);
         Object etag = headers.getFirst("ETag");
         assertNotNull(etag);
+        assertTrue(etag.toString().startsWith("\""));
+        assertTrue(etag.toString().endsWith("\""));
+
     }
     
     
@@ -1348,8 +1351,7 @@ public class JAXRSClientServerBookTest e
                                "resources/expected_get_cd.txt",
                                "application/xml", 200);
     }
-    
-    
+
     @Test
     public void testGetCDWithMultiContentTypesXML() throws Exception {
         
@@ -1417,8 +1419,67 @@ public class JAXRSClientServerBookTest e
                       expected,
                       "text/plain", "text/plain", 200);
     }
-    
-    private void getAndCompareAsStrings(String address, 
+
+    @Test
+    public void testQuotedHeaders() throws Exception {
+
+        String endpointAddress =
+            "http://localhost:" + PORT + "/bookstore/quotedheaders";
+        Response r = WebClient.create(endpointAddress).get();
+
+        List<Object> header1 = r.getMetadata().get("SomeHeader1");
+        assertEquals(1, header1.size());
+        assertEquals("\"some text, some more text\"", header1.get(0));
+
+        List<Object> header2 = r.getMetadata().get("SomeHeader2");
+        assertEquals(3, header2.size());
+        assertEquals("\"some text\"", header2.get(0));
+        assertEquals("\"quoted,text\"", header2.get(1));
+        assertEquals("\"even more text\"", header2.get(2));
+
+        List<Object> header3 = r.getMetadata().get("SomeHeader3");
+        assertEquals(1, header3.size());
+        assertEquals("\"some text, some more text with inlined \"\"", header3.get(0));
+
+        List<Object> header4 = r.getMetadata().get("SomeHeader4");
+        assertEquals(1, header4.size());
+        assertEquals("\"\"", header4.get(0));
+
+    }
+
+    @Test
+    public void testBadlyQuotedHeaders() throws Exception {
+
+        String endpointAddress =
+            "http://localhost:" + PORT + "/bookstore/badlyquotedheaders";
+
+        String[] responses = new String[] {
+            "\"some text",
+            "\"some text, some more text with inlined \"",
+            "\"some te\\",
+        };
+
+        // technically speaking, for these test cases, the client should return an error
+        // however, servers do send bad data from time to time so we try to be forgiving
+        for (int i = 0; i < 3; i++) {
+            Response r = WebClient.create(endpointAddress).query("type", Integer.toString(i)).get();
+            assertEquals(responses[i], r.getMetadata().get("SomeHeader" + i).get(0));
+        }
+
+        // this test currently returns the WRONG result per RFC2616, however it is correct
+        // per the discussion in CXF-3518
+        Response r3 = WebClient.create(endpointAddress).query("type", "3").get();
+        List<Object> r3values = r3.getMetadata().get("SomeHeader3");
+        assertEquals(4, r3values.size());
+        assertEquals("some text", r3values.get(0));
+        assertEquals("\"other quoted\"", r3values.get(1));
+        assertEquals("text", r3values.get(2));
+        assertEquals("blah", r3values.get(3));
+
+    }
+
+
+    private void getAndCompareAsStrings(String address,
                                         String resourcePath,
                                         String acceptType,
                                         int status) throws Exception {