Skip to content

ext/standard: http(s) wrapper corrupts the basic auth header on perce…#22172

Open
devnexen wants to merge 2 commits into
php:PHP-8.5from
devnexen:gh22171
Open

ext/standard: http(s) wrapper corrupts the basic auth header on perce…#22172
devnexen wants to merge 2 commits into
php:PHP-8.5from
devnexen:gh22171

Conversation

@devnexen
Copy link
Copy Markdown
Member

…nt-encoded userinfo.

php_url_decode() returns the shorter decoded length but ZSTR_LEN() is left untouched, so smart_str_append() carries the stale [decoded][NUL][undecoded tail] bytes into the base64 credentials.

Fix #22171

…nt-encoded userinfo.

php_url_decode() returns the shorter decoded length but ZSTR_LEN() is left
untouched, so smart_str_append() carries the stale [decoded][NUL][undecoded
tail] bytes into the base64 credentials.

Fix php#22171
@devnexen
Copy link
Copy Markdown
Member Author

the ftp wrapper, however, does it correctly.

@devnexen devnexen marked this pull request as ready for review May 28, 2026 18:41
@devnexen devnexen requested a review from bukka as a code owner May 28, 2026 18:41
@devnexen devnexen linked an issue May 28, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

Comment thread ext/standard/tests/http/gh22171.phpt Outdated

function probe(string $label, string $userinfo, string $expected): void
{
$responses = array(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Use [].

Comment thread ext/standard/tests/http/gh22171.phpt Outdated
$url = preg_replace('#^http://#', 'http://' . $userinfo . '@', $uri);
file_get_contents($url);

http_server_kill($pid);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if it's reliable to kill the server before seeking the content, so maybe move this down?

Comment thread ext/standard/tests/http/gh22171.phpt Outdated
fseek($output, 0, SEEK_SET);
$output = stream_get_contents($output);

if (preg_match('/^Authorization:\s*Basic\s+(\S+)/mi', $output, $m)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Inconsistent regex delimiter. I prefer (), but we should at least be consistent within the file.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid auth header generation in http(s) stream wrapper

2 participants