Refer to RFC 2109 on HTTP State Management.

An origin server may include multiple Set-Cookie headers in a
response. Note that an intervening gateway could fold multiple such
headers into a single header.

Drupal folds multiple headers by using the following code in drupal_http_request():

    if (isset($result->headers[$header]) && $header == 'Set-Cookie') {
      // RFC 2109: the Set-Cookie response header comprises the token Set-
      // Cookie:, followed by a comma-separated list of one or more cookies.
      $result->headers[$header] .= ',' . trim($value);
    }

Unfortunately, Drupal fails to account for a legacy case where a cookie contains an "expires" attribute like the following:

Set-Cookie: cookie=2975; expires=Mon, 09-Nov-2009 12:34:19 GMT; path=/
Set-Cookie: cookie2=8456; expires=Mon, 09-Nov-2009 12:34:19 GMT; path=/

On this topic, the RFC notes:

Netscape's original proposal defined an Expires header that took a
date value in a fixed-length variant format in place of Max-Age:

Wdy, DD-Mon-YY HH:MM:SS GMT

Note that the Expires date format contains embedded spaces, and that
"old" cookies did not have quotes around values. Clients that
implement to this specification should be aware of "old" cookies and
Expires.

This is a legacy/quirk case, but it would not disrupt operation since the comma is not a delimiter in separate Set-Cookie headers. However, when folding the Set-Cookie handlers into a single comma-separated string, the unprotected commas cause a conflict.

Since Drupal's implementation is currently completely unaware of attribute-value pairs, it would take a lot of additional logic to separate the individual cookies into pairs, then protect values with quotes if necessary, then reassemble them. And it may not be worth it, because the case is so rare. However, I have just encountered it and will now have to code a workaround on my own, so I'm reporting the issue here.

Comments

franz’s picture

Version: 7.x-dev » 8.x-dev
Priority: Minor » Normal

Bumping. So the RFC is not fully implemented?

mikeytown2’s picture

Assigned: cburschka » Unassigned
Status: Active » Needs review
StatusFileSize
new1.4 KB
new1.42 KB
new1.44 KB

What I've done in httprl

  // Parse the response headers.
  $cookie_primary_key = 0;
  while ($line = trim(array_shift($response))) {
    list($name, $value) = explode(':', $line, 2);
    $name = strtolower($name);

    // Parse cookies before they get added to the header.
    if ($name == 'set-cookie') {
      $cookie_values = explode(';', $value);
      $first = TRUE;
      foreach ($cookie_values as $c_name_value) {
        $c_name_value = explode('=', trim($c_name_value));
        if ($first) {
          $primary_key = trim($c_name_value[0]);
          $result->cookies[$cookie_primary_key]['name'] = trim($c_name_value[0]);
          $result->cookies[$cookie_primary_key]['value'] = trim($c_name_value[1]);
          $first = FALSE;
        }
        else {
          $result->cookies[$cookie_primary_key][trim($c_name_value[0])] = trim($c_name_value[1]);
        }
      }
      $cookie_primary_key++;
    }

    // Add key value pairs to the header; including cookies.
    if (isset($result->headers[$name]) && $name == 'set-cookie') {
      // RFC 2109: the Set-Cookie response header comprises the token Set-
      // Cookie:, followed by a comma-separated list of one or more cookies.
      $result->headers[$name] .= ',' . trim($value);
    }
    else {
      $result->headers[$name] = trim($value);
    }
  }

Status: Needs review » Needs work

The last submitted patch, drupal-http_request-cookies-8.x-336367-2.patch, failed testing.

mikeytown2’s picture

I've come up with some cleaner code and added in a lot more comments.

  // Parse the response headers.
  $cookie_primary_counter = 0;
  while ($line = trim(array_shift($response))) {
    list($name, $value) = explode(':', $line, 2);
    $name = strtolower($name);

    // Parse cookies before they get added to the header.
    if ($name == 'set-cookie') {
      // Extract the key value pairs for this cookie.
      foreach (explode(';', $value) as $cookie_name_value) {
        list($cookie_key, $cookie_value) = explode('=', trim($cookie_name_value));
        // The cookie name-value pair always comes first (RFC 2109 4.2.2).
        if (!isset($result->cookies[$cookie_primary_counter])) {
          $result->cookies[$cookie_primary_counter] = array(
            'name' => trim($cookie_key),
            'value' => trim($cookie_value),
          );
        }
        // Extract the rest of the attribute-value pairs.
        else {
          $result->cookies[$cookie_primary_counter] += array(
            trim($cookie_key) => trim($cookie_value),
          );
        }
      }
      $cookie_primary_counter++;
    }

    // Add key value pairs to the header; including cookies.
    if (isset($result->headers[$name]) && $name == 'set-cookie') {
      // RFC 2109: the Set-Cookie response header comprises the token Set-
      // Cookie:, followed by a comma-separated list of one or more cookies.
      $result->headers[$name] .= ',' . trim($value);
    }
    else {
      $result->headers[$name] = trim($value);
    }
  }

Status: Needs review » Needs work

The last submitted patch, drupal-http_request-cookies-8.x-336367-3.patch, failed testing.

mikeytown2’s picture

mikeytown2’s picture

Version: 8.x-dev » 7.x-dev

D8 has guzzle. Moving this to D7.

Status: Needs review » Needs work

The last submitted patch, drupal-http_request-cookies-336367-6-D6.patch, failed testing.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.