6
\$\begingroup\$

I've created the following function that creates a cookie string for passing to cURL. It can take a nested array or object of key-value pairs. It can either create nested cookie values as JSON, or using square brackets in cookie names.

Please could someone kindly:

  1. Assess whether it's functionally correct or not, does it generate good cookies? I don't know if it's necessary for it to repeat the expires/domain/path/secure/httponly for each cookie.
  2. Help reduce the bloat. I don't like how: (a) it calls itself 3 times. (b) it has the following code twice: $keyPrefix?$keyPrefix."[".$key."]":$key (c) in the middle, it checks: if (is_object($val) || is_array($val)) - it seems a bit redundant.

The function:

function http_build_cookie($data, $useJson=true, $expires = 0, $path = '', $domain = '', $secure = false, $httpOnly = false, $keyPrefix = '') {
    if (is_object($data)) {
        $data = (array) $data;
    }
    if (is_array($data)) {
        $cookie_parts = array();
        foreach ($data as $key => $val) {
            if ($useJson)
            {
                $cookie_parts[] = http_build_cookie(
                    rawurlencode($key)."="
                    .rawurlencode(json_encode($val)),
                    $useJson, $expires, $path, $domain, $secure, $httpOnly
                );
            }
            else
            {
                if (is_object($val) || is_array($val)) {
                    $cookie_parts[] = http_build_cookie(
                        $val, $useJson, $expires, $path, $domain,
                        $secure, $httpOnly, $keyPrefix?$keyPrefix."[".$key."]":$key
                    );
                }
                else
                {
                    $cookie_parts[] = http_build_cookie(
                        rawurlencode($keyPrefix?$keyPrefix."[".$key."]":$key)."="
                        .rawurlencode($val),
                        $useJson, $expires, $path, $domain, $secure, $httpOnly
                    );
                }
            }
        }
        return implode('; ', $cookie_parts);
    }
    if (is_scalar($data)) {
        $cookie = $data;
        if ($expires) {
            $cookie .= '; expires=' . gmdate('D, d-M-Y H:i:s T', $expires);
        }
        if ($path) {
            $cookie .= '; path=' . $path;
        }
        if ($domain) {
            $cookie .= '; domain=' . $domain;
        }
        if ($secure) {
            $cookie .= '; secure';
        }
        if ($httpOnly) {
            $cookie .= '; HttpOnly';
        }
        return $cookie;
    }
    return '';
}

Any advice would be massively appreciated!

\$\endgroup\$
10
  • 2
    \$\begingroup\$ Write some tests! \$\endgroup\$
    – slepic
    Apr 20, 2023 at 12:50
  • \$\begingroup\$ @slepic and test what exactly? And how? And why? I've tested that the function works as I intend, but I don't know if what I intended is good for cookie specifications and use cases \$\endgroup\$
    – Jodes
    Apr 20, 2023 at 12:53
  • \$\begingroup\$ Is there any reason why you're trying to send such complicated cookies instead of sending the data you want in the URL (as query parameters) or in the body (for POST requests)? \$\endgroup\$ Apr 20, 2023 at 22:11
  • \$\begingroup\$ @CL22 This is definitely the sort of function where I'd write a set of test cases first, and only then do the implementation. Working out the test cases is basically working out the specification of the function and lets you more easily observe that you've not missed anything. Then, during implementation and debugging they make testing go a lot faster, of course. You also get the huge speed increase again should you ever need to modify the function (to fix a bug, add a feature, or just clean up the code). \$\endgroup\$
    – cjs
    Apr 21, 2023 at 1:47
  • \$\begingroup\$ Beyond this, when you show the function to others this also clearly documents the specification for others and allows us to see if there are test cases missing. Right now you have a somewhat vague spec and we have no idea what tests you've run against it, what cases you expect to work, and what cases you expect to fail. \$\endgroup\$
    – cjs
    Apr 21, 2023 at 1:47

3 Answers 3

1
\$\begingroup\$

I had a go at refactoring your code for you to make it more simple, otherwise the functionality seemed to work from what I could see.

    $cookie_parts = array();

    // Check if data is a scalar value
    if (is_scalar($data)) {
        $cookie_parts[] = rawurlencode($keyPrefix) . '=' . rawurlencode($data);
    }
    // Check if data is an array or object
    elseif (is_array($data) || is_object($data)) {
        // Loop through array/object keys and values
        foreach ($data as $key => $val) {
            // Generate the key name based on whether a prefix is set or not
            $cookie_key = $keyPrefix ? $keyPrefix . '[' . $key . ']' : $key;
            
            // If value is an array/object, recursively call this function
            if (is_array($val) || is_object($val)) {
                $cookie_parts[] = http_build_cookie(
                    $val, $useJson, $expires, $path, $domain, $secure, $httpOnly, $cookie_key
                );
            }
            // If value is a scalar, generate the cookie string
            else {
                $cookie_value = $useJson ? json_encode($val) : $val;
                $cookie_parts[] = rawurlencode($cookie_key) . '=' . rawurlencode($cookie_value);
            }
        }
    }

    // Append the common cookie attributes to the final cookie string
    $cookie = implode('; ', $cookie_parts);
    if ($expires) {
        $cookie .= '; expires=' . gmdate('D, d-M-Y H:i:s T', $expires);
    }
    if ($path) {
        $cookie .= '; path=' . $path;
    }
    if ($domain) {
        $cookie .= '; domain=' . $domain;
    }
    if ($secure) {
        $cookie .= '; secure';
    }
    if ($httpOnly) {
        $cookie .= '; HttpOnly';
    }

    return $cookie;
}
\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much! \$\endgroup\$
    – Jodes
    Apr 21, 2023 at 20:45
4
\$\begingroup\$

Cookies are typically used only for simple values

Cookies typically are used to store simple data like a session id - not complex data. Refer to the introduction section on the MDN documentation Using HTTP Cookies - especially the last paragraph:

An HTTP cookie (web cookie, browser cookie) is a small piece of data that a server sends to a user's web browser. The browser may store the cookie and send it back to the same server with later requests. Typically, an HTTP cookie is used to tell if two requests come from the same browser—keeping a user logged in, for example. It remembers stateful information for the stateless HTTP protocol.

Cookies are mainly used for three purposes:

Session management

Logins, shopping carts, game scores, or anything else the server should remember

Personalization

User preferences, themes, and other settings

Tracking

Recording and analyzing user behavior

Cookies were once used for general client-side storage. While this made sense when they were the only way to store data on the client, modern storage APIs are now recommended. Cookies are sent with every request, so they can worsen performance (especially for mobile data connections). Modern APIs for client storage are the Web Storage API (localStorage and sessionStorage) and IndexedDB.

Format code per PSR-12 style guide

PSR-12 is intended to "to reduce cognitive friction when scanning code from different authors" .... "by enumerating a shared set of rules and expectations about how to format PHP code" 1. This will greatly decrease the number of lines and spacing.

Below is the output from the analysis via WEBCodeSniffer.net:

----------------------------------------------------------------------
FOUND 52 ERRORS AND 1 WARNING AFFECTING 16 LINES
----------------------------------------------------------------------
  1 | ERROR   | [x] End of line character is invalid; expected "\n"
    |         |     but found "\r\n"
  2 | ERROR   | [x] Incorrect spacing between argument "$useJson" and
    |         |     equals sign; expected 1 but found 0
  2 | ERROR   | [x] Incorrect spacing between default value and
    |         |     equals sign for argument "$useJson"; expected 1
    |         |     but found 0
  2 | WARNING | [ ] Line exceeds 120 characters; contains 143
    |         |     characters
  2 | ERROR   | [x] Opening brace should be on a new line
  9 | ERROR   | [x] Expected 1 space(s) after closing parenthesis;
    |         |     found newline
 12 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 12 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 13 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 14 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 14 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 14 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 14 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 14 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 16 | ERROR   | [x] Expected 1 space after closing brace; newline
    |         |     found
 17 | ERROR   | [x] Expected 1 space(s) after ELSE keyword; newline
    |         |     found
 21 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 21 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 21 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 21 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 22 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 22 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 22 | ERROR   | [x] Expected at least 1 space before "?"; 0 found
 22 | ERROR   | [x] Expected at least 1 space after "?"; 0 found
 22 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 22 | ERROR   | [x] Expected at least 1 space before ":"; 0 found
 22 | ERROR   | [x] Expected at least 1 space after ":"; 0 found
 24 | ERROR   | [x] Expected 1 space after closing brace; newline
    |         |     found
 25 | ERROR   | [x] Expected 1 space(s) after ELSE keyword; newline
    |         |     found
 28 | ERROR   | [x] Expected at least 1 space before "?"; 0 found
 28 | ERROR   | [x] Expected at least 1 space after "?"; 0 found
 28 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space before ":"; 0 found
 28 | ERROR   | [x] Expected at least 1 space after ":"; 0 found
 28 | ERROR   | [x] Expected at least 1 space before "."; 0 found
 28 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 29 | ERROR   | [x] Expected at least 1 space after "."; 0 found
 30 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 30 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 30 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 30 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 30 | ERROR   | [x] Only one argument is allowed per line in a
    |         |     multi-line function call
 57 | ERROR   | [x] Expected 1 newline at end of file; 0 found

The opening braces for the blocks of the first two if statements exist on the same line ✅, though the next if statement has the brace starting on the following line:

 if ($useJson)
 {

And the three lines with the else in-between

        }
        else
        {

can be simplified to a single line

} else {

Use Variable parsing (String interpolation) in double quoted strings

There are places in the code like:

 $keyPrefix?$keyPrefix."[".$key."]":$key

The concatenation can be removed using Variable parsing:

$keyPrefix ? "{$keyPrefix}[$key]" : "$key"

The complex syntax using { and } around $keyPrefix are required so the square braces won't be interpreted as array access.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thank you! I didn't realise there were standards that could be followed for code formatting in PHP! \$\endgroup\$
    – Jodes
    Apr 21, 2023 at 20:46
  • \$\begingroup\$ I would argue that you should always use the "{$var}" syntax over the "$var" syntax. The former is a lot easier to identify when reading a string, even in an editor that doesn't have syntax highlight or when working without colors. \$\endgroup\$ Apr 22, 2023 at 1:47
3
\$\begingroup\$

The curl way of managing cookies is using the curls options.

curl_setopt( $curl_handle, CURLOPT_COOKIESESSION, true );
curl_setopt( $curl_handle, CURLOPT_COOKIEJAR, $uniquefilename );
curl_setopt( $curl_handle, CURLOPT_COOKIEFILE, $uniquefilename );

The above code is taken from this SO thread: https://stackoverflow.com/questions/12885538/php-curl-and-cookies

I am a bit suspicious that you are worried about the problems with storing the cookies in a file as described in that thread.

However, you are probably confused about how cookies work.

Server may send Set-Cookie headers, each containing a key-value pair with all the cookie options.

Client stores the (server) cookies received from server in a "cookie jar".

Client may also add or remove any (client) cookies in the cookie jar.

In consecutive requests the client queries the cookie jar with the request url and the cookie jar returns the keys and values of cookies that match that url according to the cookies' options.

Client then sends those key-value pairs in Cookie header to the requested url.

To implement own cookie management over curl you will need

  • function to build Cookie header from key-value pairs
  • function to parse Set-Cookie header
  • class to represent a parsed response cookie (key value pair with options)
  • class to represent a cookie jar with methods to
    • add cookie (with options)
    • get cookies for given url

You have actually confused a lot of people here, including me.

Your function looks like it attempts to build a Set-Cokkie header. While the intent probably was to have it build a Cookie header. And in fact it builds a strange mix of both.

Anyway, don't be shy to use modern php features.

final readonly class Cookie
{
    public function __construct(
        public string $key,
        public string $value,
        public string $domain,
        public string $path = '',
        // ...
    )
    {
    }
}

interface CookieHeaderFactory
{
    /**
     * @param array<string, string> $cookies
     */
    public function createCookieHeaderValue(array $cookies): string;
}

interface SetCookieHeaderParser
{
    public function parseSetCookieHeaderValue(string $value): Cookie;
}

interface CookieJar
{
  public function addCookie(Cookie $cookie): void;

  /**
   * @return array<string, string>
   */
  public function getRequestCookies(string $url): array;
}

EDIT:

According to

Cookie: <cookie-list>

<cookie-list> A list of name-value pairs in the form of =. Pairs in the list are separated by a semicolon and a space ('; ').

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cookie

All you really need for the request Cookie header is this:

/**
 * @param array<string, string> $cookies
 */
function http_build_cookie_header_value(array $cookies): string
{
    $pairs = [];
    foreach ($cookies as $name => $value) {
        if (str_contains_forbidden_cookie_chars($name)) {
            throw new \InvalidArgumentException('Invalid cookie name: '. $name);
        }
        if (str_contains_forbidden_cookie_chars($value)) {
            throw new \InvalidArgumentException('Invalid cookie value: '. $value);
        }
        $pairs[] = $name . '=' . $value;
    }
    return \implode('; ', $pairs);
}

$cookies = ['PHPSESSID' => 'aaaaaaaaaa'];
$headerLine = 'Cookie: ' . http_build_cookie_header_value($cookies);

Encoding: Many implementations perform URL encoding on cookie values. However, this is not required by the RFC specification. The URL encoding does help to satisfy the requirements of the characters allowed for .

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

This is for Set-Cookie header but it clearly applies for Cookie as well.

So in fact, you don't need to encode the names and values. Instead you should make sure they dont contain any forbidden characters.

As for the part of updating cookies from Set-Cookie headers received from server. If you are only concerned about a single domain, you can omit the complexity of the cookie jar and all the cookie options. Basically just parse the <name>=<value> part of the response header and ignore everything else. Except maybe the Expires part which servers may use to delete a cookie by setting its expiration to the past. If a cookie value is empty string you may also consider the cookie to be deleted.


function http_update_cookies(array $previousCookies, array $responseHeaders): array
{
  // look up Set-Cookie headers
  // parse them for name=value
  // combine with $previousCookies
  // and return new set of cookies
}

Example of how you would combine it together

$cookies = ['PHPSESSID' => 'aaaaaaaaaa'];

$response1 = call_api($url1, ['Cookie' => http_build_cookie_header_value($cookies)]);

$cookies = http_update_cookies($cookies, $response['headers']);

$response2 = call_api($url2, ['Cookie' => http_build_cookie_header_value($cookies)]);

$cookies = http_update_cookies($cookies, $response['headers']);

// ...

```
\$\endgroup\$
10
  • \$\begingroup\$ Thanks. Now I'm really confused! I obviously have a lot of understanding I need to work on. Thanks for your time! I'll take a good look at what you've written and see if I can get my head round it. I didn't realise there was PHP functionality for this either. \$\endgroup\$
    – Jodes
    Apr 21, 2023 at 20:40
  • \$\begingroup\$ When you say don't be afraid to use modern PHP features, what were you referring to? And the interfaces you provided, are you suggesting that as something for me to implement? Thanks again \$\endgroup\$
    – Jodes
    Apr 21, 2023 at 20:57
  • \$\begingroup\$ You're right, I was trying to create a Cookie header. In what way was I not doing it right, and making something for SetCookie instead? From what I gather, while it caters for SetCookie, it can also be used just for Cookie? \$\endgroup\$
    – Jodes
    Apr 21, 2023 at 21:09
  • \$\begingroup\$ @CL22 i was referring mostly to typed function parameters and return type. Also the use of long array syntax. Makes it look like you learn php from a 90's tutorial... \$\endgroup\$
    – slepic
    Apr 22, 2023 at 5:15
  • \$\begingroup\$ No you don't have to implement those interfaces if you're not familiar. You can stick to functions. I just wanted to show the signatures of such functions. See where I write down the list of things you will need for custom cookie management over curl. The interfaces are just the same list again, but written in php. \$\endgroup\$
    – slepic
    Apr 22, 2023 at 5:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.