[CAMEL-12395] HttpProducer cookie handling broken

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 2.20.2
• Fix Version/s: 2.20.4, 2.21.1, 2.22.0
• Component/s: camel-http
• Estimated Complexity: Novice
• Assignee: Claus Ibsen
• Reporter: Kevin Brooks
• Created: 22/Mar/18 22:00
• Updated: 13/Apr/18 07:22
• Resolved: 13/Apr/18 07:22

Description

Assumptions from https://tools.ietf.org/html/rfc6265
When a host response contains multiple headers with the same key, each with different values, the HttpProducer overwrites the value, effectively last-in-wins, extracted problem code below

protected void populateResponse(Exchange exchange, HttpRequestBase httpRequest, HttpResponse httpResponse,
Message in, HeaderFilterStrategy strategy, int responseCode) throws IOException, ClassNotFoundException {
  ...
  // propagate HTTP response headers
  Header[] headers = httpResponse.getAllHeaders();
  Map<String, List<String>> m = new HashMap<String, List<String>>();
  for (Header header : headers) {
    String name = header.getName();
    String value = header.getValue();
    m.put(name, Collections.singletonList(value)); //<--- This is the problem
    if (name.toLowerCase().equals("content-type")) {
      name = Exchange.CONTENT_TYPE;
      exchange.setProperty(Exchange.CHARSET_NAME, IOHelper.getCharsetNameFromContentType(value));
    }
    // use http helper to extract parameter value as it may contain multiple values
    Object extracted = HttpHelper.extractHttpParameterValue(value);
    if (strategy != null && !strategy.applyFilterToExternalHeaders(name, extracted, exchange)) {
     HttpHelper.appendHeader(answer.getHeaders(), name, extracted);
    }
  }
  // handle cookies
  if (getEndpoint().getCookieHandler() != null) {
  //if host responded with multiple Set-Cookie headers, only last cookie is presented
  getEndpoint().getCookieHandler().storeCookies(exchange, httpRequest.getURI(), m);
  }
...

A simple fix ->

...
for (Header header : headers) {
  String name = header.getName();
  String value = header.getValue();
  List<String> values = m.computeIfAbsent(name, k -> new ArrayList<>()).add(value);
...

On the flip side, when the client responds, the cookies pulled from the handler are not formatted correctly, broken code snippet from HttpProducer.process()

    if (getEndpoint().getCookieHandler() != null) {
            Map<String, List<String>> cookieHeaders = getEndpoint().getCookieHandler().loadCookies(exchange, httpRequest.getURI());
            for (Map.Entry<String, List<String>> entry : cookieHeaders.entrySet()) {
                String key = entry.getKey();
                if (entry.getValue().size() > 0) {
                    // use the default toString of a ArrayList to create in the form [xxx, yyy]
                    // if multi valued, for a single value, then just output the value as is
                    String s = entry.getValue().size() > 1 ? entry.getValue().toString() : entry.getValue().get(0);//<--- This is a problem
                    httpRequest.addHeader(key, s);
                }
            }
        }

This can be fixed simply

    if (getEndpoint().getCookieHandler() != null) {
            Map<String, List<String>> cookieHeaders = getEndpoint().getCookieHandler().loadCookies(exchange, httpRequest.getURI());
            for (Map.Entry<String, List<String>> entry : cookieHeaders.entrySet()) {
                String key = entry.getKey();
                if (entry.getValue().size() > 0) {
                    httpRequest.addHeader(key, entry.getValue().stream().collect(Collectors.joining(";"))); //semi-colon, not comma...
                }
            }
        }

Additionally, the CookieHandler.loadCookies method is not properly constructing the Cookie values, as it calls HttpCookie.toString - which represents the host's Set-Cookie value, not the client's Cookie value