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

Cookies and headers are sent twice #15334

Closed
DestinyMKas opened this issue Mar 17, 2021 · 8 comments · Fixed by #15429
Closed

Cookies and headers are sent twice #15334

DestinyMKas opened this issue Mar 17, 2021 · 8 comments · Fixed by #15429
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium

Comments

@DestinyMKas
Copy link
Contributor

$application->handle() by default sends cookies and headers
https://github.com/phalcon/cphalcon/blob/master/phalcon/Mvc/Application.zep#L393-L405

I use following logic in my code:

$response = $application->handle($uri);
$response->send();

In this case cookies and headers are sent twice.
My suggestions would be:

  1. application->handle() by default should not send headers & cookies;
  2. prevent sending same cookie & header twice.
@bin20060407
Copy link

I have encountered the same situation and will send cookies twice

@Jeckerson Jeckerson added status: unverified Unverified bug A bug report 5.0 The issues we want to solve in the 5.0 release labels Mar 19, 2021
@niden
Copy link
Sponsor Member

niden commented Apr 21, 2021

I am going to use the old cliche this is not a bug it is a feature

The majority of applications I have seen and developed, I was expecting the call to handle() to send the relevant headers back, so that I don't have to worry about it.

It appears your workflow is different and you are in need of sending headers and cookies at a later time. To change the default behavior you can do this:

<?php

$response = $application
    ->sendCookiesOnHandleRequest(false)
    ->sendHeadersOnHandleRequest(true)
    ->handle()
;

$response->send();

@niden niden added not a bug Reported issue is not a bug and removed status: unverified Unverified labels Apr 21, 2021
@DestinyMKas
Copy link
Contributor Author

If you expect and design the system to send headers & cookies on handle(), then you should not send headers & cookies on $response->send(). At least not by default.

From my experience "send" means "send" and "handle" doesn't mean "send" :) Method names should be self explained.

@niden
Copy link
Sponsor Member

niden commented Apr 21, 2021

How things are and how they should be in terms of verbiage is a long discussion. The code in question was written back in 2013 and hardly touched since. I cannot remember a reference of this or something similar honestly.

However digging through the code I see the following:

Response -> send (https://github.com/phalcon/cphalcon/blob/5.0.x/phalcon/Http/Response.zep#L334)

Application - handle

The code seems OK to be honest. The check is if headers are sent and then it will send cookies/headers back. I am not sure why you get duplicates.

Could you offer an example so that at least we can figure out why this is happening?

@DestinyMKas
Copy link
Contributor Author

I see that you use headers_sent which becomes true only when actual content is sent.
Simple example:

<?php

header('Something: something');
var_dump(headers_list(), headers_sent());
?>

Result for me is:

array(2) { [0]=> string(24) "X-Powered-By: PHP/7.4.16" [1]=> string(20) "Something: something" } bool(false) 

As you see headers_sent() returns false even I try to send header before. In your case it doesnt to prevent to send headers, cookies twice because no actual content is sent.

@niden
Copy link
Sponsor Member

niden commented Apr 22, 2021

Honestly this is the first time I see this. I was under the impression that the headers_sent does what I thought it was i.e. that the headers were already sent and thus no duplicate sending.

What is really going on here - again from what I understand seeing your example, is that somewhere in the code some headers/cookies are being set. The output has not started yet

Then at some point the response->send() is called and the same headers/cookies are set again

Finally output starts and then you have the duplicate headers.

I really don't know how I can get this to not send stuff twice. handle is what collates all the operations including headers sending and then output is sent back.

Looking at the cookies, we could check the $_COOKIE superglobal if the cookie has been set already and could use the has() method in the Cookies object which does just that. That of course assumes that when you sending the cookies over and the cookie does not exist in the $_COOKIE superglobal it will not be sent twice. Making that change will not allow people to overwrite existing cookies, which from experience I have seen many applications do.

For the headers it might be a bit easier since we can query the headers_list

Thoughts?

@niden
Copy link
Sponsor Member

niden commented Apr 22, 2021

Discussed the issue offline and a solution was found. PR to follow

@niden niden mentioned this issue Apr 22, 2021
5 tasks
@niden niden linked a pull request Apr 22, 2021 that will close this issue
5 tasks
@niden niden added status: medium Medium and removed not a bug Reported issue is not a bug labels Apr 22, 2021
@niden
Copy link
Sponsor Member

niden commented Apr 22, 2021

Resolved in #15429

Thank you @DestinyMKas and @bin20060407

@niden niden closed this as completed Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants