cxf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Beryozkin (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (CXF-3112) Further problem with cookies in Jax-RS (similar to closed issue 3035)
Date Sun, 07 Nov 2010 14:19:07 GMT

    [ https://issues.apache.org/jira/browse/CXF-3112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12929337#action_12929337
] 

Sergey Beryozkin edited comment on CXF-3112 at 11/7/10 9:18 AM:
----------------------------------------------------------------

Hi, yes, the problem with the CXF (test) server is that the server collapses two separate
Set-Cookie values into a single line, which works ok unless Expires is present. So one thing
we should fix here is to ensure, probably at the AbstractHttpDestination level, that the outbound
Set-Cookie(s) are indeed presented as separate headers. At least I see why we get a single
cookie (as opposed to expected 2 ones).   

Now, in the live environment, you're probably getting two separate Set-Cookie headers, with
only the 2nd cookie, as opposed to the 1st one, containing 'Expires'. I agree, the splitPossible
check should be done when dealing with individual header values, may be we can optimize a
bit :

boolean splitPossible = !HttpHeaders.SET_COOKIE.equalsIgnoreCase(entry.getKey());
for (String val : entry.getValue()) {

boolean splitConfirmed = splitPossible && !val.contains(HttpHeaders.EXPIRES);

String[] values = splitConfirmed ? val.split(",") : new String[]{val};
for (String s : values) {
String theValue = s.trim();
if (theValue.length() > 0) { currentResponseBuilder.header(entry.getKey(), theValue); }
}
}

Can you give me a favor and confirm this would work against the live server ? And if it does
then do a patch ?
Also, if possible, you may want to check AbstractHttpDestination.copyResponseHeaders() :

for (Iterator<?> iter = headers.keySet().iterator(); iter.hasNext();) {
                String header = (String)iter.next();
                List<?> headerList = (List<?>)headers.get(header);
                StringBuilder sb = new StringBuilder();
                for (int i = 0; i < headerList.size(); i++) {
                    sb.append(headerList.get(i));
                    if (i + 1 < headerList.size()) {
                        sb.append(',');
                    }
                }
                response.addHeader(header, sb.toString());
            }

may this code can be updated to :

for (Iterator<?> iter = headers.keySet().iterator(); iter.hasNext();) {
                String header = (String)iter.next();
                List<?> headerList = (List<?>)headers.get(header);
                StringBuilder sb = new StringBuilder();

                if (SET_COOKIE.equals(header)) {
                    for (int i = 0; i < headerList.size(); i++) {
                      response.addHeader(header, headerList.get(i));
                   }
                } else {
                   for (int i = 0; i < headerList.size(); i++) {
                      sb.append(headerList.get(i));
                      if (i + 1 < headerList.size()) {
                          sb.append(',');
                      }
                   }
                   response.addHeader(header, sb.toString()); 
                }
                
            }

This probably would not cause any issues as far as external clients of CXF servers dealing
with cookies is concerned, in fact, it probably will make CXF servers better citizens as far
as multiple cookies are concerned 


thanks, Sergey

 

      was (Author: sergey_beryozkin):
    Hi, yes, the problem with the CXF (test) server is that the server collapses two separate
Set-Cookie values into a single line, which works ok unless Expires is present. So one thing
we should fix here is to ensure, probably at the AbstractHttpDestination level, that the outbound
Set-Cookie(s) are indeed presented as separate headers. At least I see why we get a single
cookie (as opposed to expected 2 ones).   

Now, in the live environment, you're probably getting two separate Set-Cookie headers, with
only the 2nd cookie, as opposed to the 1st one, containing 'Expires'. I agree, the splitPossible
check should be done when dealing with individual header values, may be we can optimize a
bit :

boolean splitPossible = !HttpHeaders.SET_COOKIE.equalsIgnoreCase(entry.getKey());
for (String val : entry.getValue()) {

// will only be checked if it is Set-Cookie
boolean splitConfirmed = splitPossible && !val.contains(HttpHeaders.EXPIRES);

String[] values = splitConfirmed ? val.split(",") : new String[]{val};
for (String s : values) {
String theValue = s.trim();
if (theValue.length() > 0) { currentResponseBuilder.header(entry.getKey(), theValue); }
}
}

Can you give me a favor and confirm this would work against the live server ? And if it does
then do a patch ?
Also, if possible, you may want to check AbstractHttpDestination.copyResponseHeaders() :

for (Iterator<?> iter = headers.keySet().iterator(); iter.hasNext();) {
                String header = (String)iter.next();
                List<?> headerList = (List<?>)headers.get(header);
                StringBuilder sb = new StringBuilder();
                for (int i = 0; i < headerList.size(); i++) {
                    sb.append(headerList.get(i));
                    if (i + 1 < headerList.size()) {
                        sb.append(',');
                    }
                }
                response.addHeader(header, sb.toString());
            }

may this code can be updated to :

for (Iterator<?> iter = headers.keySet().iterator(); iter.hasNext();) {
                String header = (String)iter.next();
                List<?> headerList = (List<?>)headers.get(header);
                StringBuilder sb = new StringBuilder();

                if (SET_COOKIE.equals(header)) {
                    for (int i = 0; i < headerList.size(); i++) {
                      response.addHeader(header, headerList.get(i));
                   }
                } else {
                   for (int i = 0; i < headerList.size(); i++) {
                      sb.append(headerList.get(i));
                      if (i + 1 < headerList.size()) {
                          sb.append(',');
                      }
                   }
                   response.addHeader(header, sb.toString()); 
                }
                
            }

This probably would not cause any issues as far as external clients of CXF servers dealing
with cookies is concerned, in fact, it probably will make CXF servers better citizens as far
as multiple cookies are concerned 


thanks, Sergey

 
  
> Further problem with cookies in Jax-RS (similar to closed issue 3035)
> ---------------------------------------------------------------------
>
>                 Key: CXF-3112
>                 URL: https://issues.apache.org/jira/browse/CXF-3112
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.0, 2.2.11
>            Reporter: David Hagar
>
> Prior to version 2.2.11, multiple cookies were not handled properly by the method org.apache.cxf.jaxrs.client.AbstractClient#setResponseBuilder(HttpURLConnection,
Exchange) -- this was resolved for some cookies, but it still breaks for others. Specifically,
any cookie that contains an "Expires" field now gets split into two objects by the aforementioned
method. 
> For example, if the header is:
> Set-Cookie: com.wm.visitor=10789493347; Domain=.walmart.com; Expires=Thu, 01-Oct-2020
23:44:22 GMT; Path=/
> Then response.getMetadata().get("Set-Cookie"); will return an array of length 2, with
values = {" com.wm.visitor=10789493347; Domain=.walmart.com; Expires=Thu", "01-Oct-2020 23:44:22
GMT; Path=/"
> I'm pretty sure this is a conflict of the code looking for date related headers conflicting
with the code looking for cookie related headers. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message