Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: CURLRequest keeps the last value provided by headers with the same name #5041

Closed
NicolaeIotu opened this issue Aug 30, 2021 · 6 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@NicolaeIotu
Copy link
Contributor

NicolaeIotu commented Aug 30, 2021

It is not possible to get and forward all headers having multiple values of a response when performing a CURLRequest.
My experience was with 'set-cookie' header, but after investigating it was pretty obvious that the CURLRequest (and probably other involved or related classes) will only retain the last value of headers with the same name and in case of redirection trigger ErrorException: Header may not contain more than a single header, new line detected (SYSTEMPATH/HTTP/ResponseTrait.php at line 471).

The following fixed the issue for me:

System\HTTP\CURLRequest

    protected function parseOptions(array $options)
    {
        if (array_key_exists('baseURI', $options)) {
            $this->baseURI = $this->baseURI->setURI($options['baseURI']);
            unset($options['baseURI']);
        }

        if (array_key_exists('headers', $options) && is_array($options['headers'])) {
            foreach ($options['headers'] as $name => $value) {
                // my adjustment
                if($this->hasHeader($name)) {
                    $this->appendHeader($name, $value);
                } else {
                    $this->setHeader($name, $value);
                }
                // end my adjustment
            }

            unset($options['headers']);
        }
...

System\HTTP\ResponseTrait

    public function sendHeaders()
    {
        // Have the headers already been sent?
        if ($this->pretend || headers_sent()) {
            return $this;
        }

        // Per spec, MUST be sent with each request, if possible.
        // http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html
        if (! isset($this->headers['Date']) && PHP_SAPI !== 'cli-server') {
            $this->setDate(DateTime::createFromFormat('U', (string) time()));
        }

        // HTTP Status
        header(sprintf('HTTP/%s %s %s', $this->getProtocolVersion(), $this->getStatusCode(), $this->getReason()), true, $this->getStatusCode());

        // Send all of our headers
        foreach (array_keys($this->getHeaders()) as $name) {
            // my adjustment
            $headerValue = $this->getHeader($name)->getValue();
            if(is_array($headerValue)) {
                foreach($headerValue as $h_value) {
                    header($name . ': ' . $h_value, false, $this->getStatusCode());
                }
            } else {
                header($name . ': ' . $this->getHeaderLine($name), false, $this->getStatusCode());
            }
        // end my adjustment
        }

        return $this;
    }

CodeIgniter 4 version
4.1.3

Affected module(s)
System\HTTP\CURLRequest
System\HTTP\ResponseTrait
probably some of the other classes as well

Expected behavior, and steps to reproduce if appropriate

  1. Perform a CURLRequest
  2. Obtain response with multiple values for i.e. 'set-cookie' header: expected - all values, obtained - the last value
  3. Redirect to endpoint while passing all 'set-cookie' headers: expected - redirection OK, obtained - error

Context
All

@NicolaeIotu NicolaeIotu added the bug Verified issues on the current code behavior or pull requests that will fix them label Aug 30, 2021
@kenjis
Copy link
Member

kenjis commented Oct 19, 2021

@NicolaeIotu Thank you for reporting.
Why don't you send PR?
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/README.md

NicolaeIotu added a commit to NicolaeIotu/CodeIgniter4 that referenced this issue Oct 19, 2021
…rovided by headers with the same name

CURLRequest can now get and forward all headers including multiple headers with the same name
@kenjis
Copy link
Member

kenjis commented Nov 11, 2023

I confirmed the bug.

        $client   = \Config\Services::curlrequest();
        $response = $client->request('GET', 'https://github.com/', []);
        dd($response->headers());

Screenshot 2023-11-11 10 01 38

Screenshot 2023-11-11 10 03 29

@kenjis
Copy link
Member

kenjis commented Nov 11, 2023

The Set-Cookie HTTP response header is used to send a cookie from the server to the user agent, so that the user agent can send it back to the server later. To send multiple cookies, multiple Set-Cookie headers should be sent in the same response.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

@kenjis
Copy link
Member

kenjis commented Nov 11, 2023

I found this comment. But "return an array of header objects" is not yet implemented.

* Returns a single Header object. If multiple headers with the same
* name exist, then will return an array of header objects.

@kenjis
Copy link
Member

kenjis commented Nov 22, 2023

I sent PR #8240 to fix this bug.
Try if you can.

@kenjis
Copy link
Member

kenjis commented Dec 20, 2023

This will be fixed in v4.5.0. See #8240

@kenjis kenjis closed this as completed Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
2 participants