Skip to content

Fix GH-11146: the use of SSE in the built-in webserver #11147

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Fix GH-11146: the use of SSE in the built-in webserver #11147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7aca5eb
Fix the use of SSE in the built-in webserver
Yurunsoft Apr 28, 2023
07e735c
Fix test
Yurunsoft Apr 28, 2023
3f0f74e
Fix test
Yurunsoft Apr 28, 2023
b2d234b
Fix
Yurunsoft Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion main/SAPI.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ SAPI_API size_t sapi_apply_default_charset(char **mimetype, size_t len)
charset = SG(default_charset) ? SG(default_charset) : SAPI_DEFAULT_CHARSET;

if (*mimetype != NULL) {
if (*charset && strncmp(*mimetype, "text/", 5) == 0 && strstr(*mimetype, "charset=") == NULL) {
if (*charset && strncmp(*mimetype, "text/", 5) == 0 && strstr(*mimetype, "charset=") == NULL && strcmp(*mimetype, "text/event-stream") != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This will need to be more generic so we can use it for other subtypes that could be added as well.

Copy link
Member

Choose a reason for hiding this comment

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

You can also start comparisons from subtype as text/ is already checked...

newlen = len + (sizeof(";charset=")-1) + strlen(charset);
newtype = emalloc(newlen + 1);
PHP_STRLCPY(newtype, *mimetype, newlen + 1, len);
Expand Down
45 changes: 45 additions & 0 deletions sapi/cli/tests/gh11146.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--TEST--
GH-11146: The built-in webserver will add ";charset=UTF-8" to the end of the Content-Type response header of the SSE response.
--SKIPIF--
<?php
include "skipif.inc";
?>
--FILE--
<?php
include "php_cli_server.inc";
php_cli_server_start(
<<<'CODE'
header('Content-Type: text/event-stream');
for ($i = 0; $i < 3; ++$i) {
echo 'data: ' . $i . "\n\n";
}
CODE
);

$host = PHP_CLI_SERVER_HOSTNAME;

$fp = php_cli_server_connect();
if(fwrite($fp, <<<HEADER
Copy link
Member

Choose a reason for hiding this comment

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

would be nice if you could define heredoc outside if to be more readable. Also there should be a space after if

GET / HTTP/1.1
Host: {$host}


HEADER)) {
while (!feof($fp)) {
echo fgets($fp);
}
}
?>
--EXPECTF--
HTTP/1.1 200 OK
Host: %s
Date: %s
Connection: close
X-Powered-By: PHP/%s
Content-Type: text/event-stream

data: 0

data: 1

data: 2