Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add headers handler in CLI SAPI #16145

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

Open
joanhey wants to merge 8 commits into php:master
base: master
Choose a base branch
Loading
from joanhey:cli-header-handler

Conversation

Copy link
Contributor

@joanhey joanhey commented Sep 30, 2024
edited
Loading

Check issue #12304

With this change, we can use the headers from CLI SAPI. But the most important think is that we can also use setcookie() and sessions.
And also it's useful for PHP tests from CLI.

All the included tests pass.
image

(削除) Only fail the actual header() and friends [ext/standard/tests/general_functions/head.phpt] that test the old behavior.
I can change this test if this PR will be accepted. But now this test show the difference. (削除ここまで)

Changed.
https://github.com/joanhey/php-src/blob/cli-header-handler/ext/standard/tests/general_functions/head.phpt
image

BC: Nothing
That functions now do nothing from CLI.

I don't think that this feature need an RFC, but you'll say it.

Thank you

henrystivens, nelsonrojasnunez, walkor, javierlopezalvarez, argordmel, JoseMarkos, hamza221, and xepozz reacted with thumbs up emoji HenryMalas reacted with hooray emoji
To pass with the new behavior.
Copy link
Contributor Author

joanhey commented Dec 2, 2024
edited
Loading

✋ anybody could review it or start a discussion ??

PD: I can start it later in internals if necessary !!

JJCLane reacted with thumbs up emoji

Copy link
Member

bukka commented Dec 8, 2024

So I think this needs an RFC because it changes the way how CLI treats header function. I can see your use case but as can be seen in the issue, there is already one objection to it (note about throwing exception instead) so that alone shows there should be probably RFC.

Also changing behaviour is in some way a BC break as you actually needed to change one of the standard test. This would be still acceptable for minor version IMO but it should not state there is no BC break at all.

Copy link
Contributor

@xepozz xepozz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

joanhey reacted with thumbs up emoji
--SKIPIF--
<?php include "skipif.inc" ?>
--INI--
expose_php=On
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave the X-Powered-By header test separately and remove it by default in the others.

<?php
setcookie('hi', time()+3600);

setcookie('hi');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please duplicate the test with setrawcookie function

Copy link
Contributor Author

joanhey commented Aug 6, 2025
edited
Loading

What about to test these functions as well:

We are using the core header_handler that is well tested, we only add tests for CLI.

http_clear_last_response_headers() and http_get_last_response_headers() use the headers received via the HTTP wrapper, that we don't touch or use in this PR.

http_response_code() curiously is working in CLI. It's later used in the send_headers_handler that we don't use in CLI, and it's the job of the framework to manage the response code and any HTTP/ header.

<?php
ob_start();
echo PHP_SAPI . PHP_EOL;
http_response_code(400);
echo http_response_code();
/* return
cli
400
*/

session_set_cookie() and session_get_cookie() also work with CLI. And it's used by setcookie(), that it's tested in the PR.

<?php
ob_start();
echo PHP_SAPI . PHP_EOL;
session_set_cookie_params([
 'lifetime' => 600,
 'path' => '/',
 'domain' => 'example.com',
 'secure' => true,
 'httponly' => true,
 'samesite' => true
 ]);
 
 
var_dump(session_get_cookie_params());
/* return 
cli
array(6) {
 ["lifetime"]=>
 int(600)
 ["path"]=>
 string(1) "/"
 ["domain"]=>
 string(11) "example.com"
 ["secure"]=>
 bool(true)
 ["httponly"]=>
 bool(true)
 ["samesite"]=>
 string(1) "1"
}
*/

ob_end_flush() work like always, but in CLI is sent to the STDOUT without headers.
We don't want that, well we can play with the ob_start() callback to find a solution.
And here start another problem that we need to investigate, simplify data stream request and response from CLI. But that need an RFC.

Copy link
Contributor

xepozz commented Aug 6, 2025

http_response_code() curiously is working in CLI. It's later used in the send_headers_handler that we don't use in CLI, and it's the job of the framework to manage the response code and any HTTP/ header.

Wouldn't it better to create a full CLI mirror for every "http"-like function to not get back to another PR/RFC when you want to touch another aspect of the servers?
Some kind of drivers: http for web, in-memory for CLI. Like phpinfo() does, but more abstract

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bukka bukka Awaiting requested review from bukka bukka is a code owner

1 more reviewer

@xepozz xepozz xepozz left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /