Skip to content

standard: various nit and refactorings for standard stream wrappers #22875

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

Open
Girgias wants to merge 4 commits into
base: master
Choose a base branch
from
+74 −78
Open

standard: various nit and refactorings for standard stream wrappers #22875

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
e35324f
standard/fpt_fopen_wrapper: use size_t instead of int
Girgias Jul 23, 2026
2502608
standard/http_fopen_wrapper: add const qualifiers
Girgias Jul 23, 2026
05bc761
standard/http_fopen_wrapper: use zend_string API in php_stream_http_r…
Girgias Jul 23, 2026
88c225c
standard/http_fopen_wrapper: use zend_string for Location header value
Girgias Jul 23, 2026
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
11 changes: 5 additions & 6 deletions ext/standard/ftp_fopen_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char
php_uri *resource = NULL;
int result, use_ssl, use_ssl_on_data = 0;
char tmp_line[512];
char *transport;
int transport_len;

const php_uri_parser *uri_parser = php_stream_context_get_uri_parser("ftp", context);
if (uri_parser == NULL) {
Expand All @@ -150,7 +148,8 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char
if (resource->port == 0)
resource->port = 21;

transport_len = (int)spprintf(&transport, 0, "tcp://%s:" ZEND_LONG_FMT, ZSTR_VAL(resource->host), resource->port);
char *transport;
size_t transport_len = spprintf(&transport, 0, "tcp://%s:" ZEND_LONG_FMT, ZSTR_VAL(resource->host), resource->port);
stream = php_stream_xport_create(transport, transport_len, REPORT_ERRORS, STREAM_XPORT_CLIENT | STREAM_XPORT_CONNECT, NULL, NULL, context, NULL, NULL);
efree(transport);
if (stream == NULL) {
Expand Down Expand Up @@ -420,8 +419,6 @@ php_stream * php_stream_url_wrap_ftp(php_stream_wrapper *wrapper, const char *pa
zval *tmpzval;
bool allow_overwrite = false;
int8_t read_write = 0;
char *transport;
int transport_len;
zend_string *error_message = NULL;

tmp_line[0] = '\0';
Expand Down Expand Up @@ -554,7 +551,9 @@ php_stream * php_stream_url_wrap_ftp(php_stream_wrapper *wrapper, const char *pa
if (hoststart == NULL) {
hoststart = ZSTR_VAL(resource->host);
}
transport_len = (int)spprintf(&transport, 0, "tcp://%s:%d", hoststart, portno);

char *transport;
size_t transport_len = spprintf(&transport, 0, "tcp://%s:%d", hoststart, portno);
datastream = php_stream_xport_create(transport, transport_len, REPORT_ERRORS, STREAM_XPORT_CLIENT | STREAM_XPORT_CONNECT, NULL, NULL, context, &error_message, NULL);
efree(transport);
if (datastream == NULL) {
Expand Down
141 changes: 69 additions & 72 deletions ext/standard/http_fopen_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,9 @@ static zend_result php_stream_handle_proxy_authorization_header(const char *s, s
typedef struct _php_stream_http_response_header_info {
php_stream_filter *transfer_encoding;
size_t file_size;
zend_string *location;
bool error;
bool follow_location;
char *location;
size_t location_len;
} php_stream_http_response_header_info;

static void php_stream_http_response_header_info_init(
Expand Down Expand Up @@ -187,26 +186,22 @@ static bool php_stream_http_response_header_trim(char *http_header_line,
* last header line. */
static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *wrapper,
php_stream *stream, php_stream_context *context, int options,
zend_string *last_header_line_str, char *header_line, size_t *header_line_length,
zend_string *last_header_line, char *header_line, size_t *header_line_length,
int response_code, zval *response_header,
php_stream_http_response_header_info *header_info)
{
char *last_header_line = ZSTR_VAL(last_header_line_str);
size_t last_header_line_length = ZSTR_LEN(last_header_line_str);
char *last_header_line_end = ZSTR_VAL(last_header_line_str) + ZSTR_LEN(last_header_line_str) - 1;

/* Process non empty header line. */
if (header_line && (*header_line != '\n' && *header_line != '\r')) {
/* Removing trailing white spaces. */
if (php_stream_http_response_header_trim(header_line, header_line_length) &&
*header_line_length == 0) {
/* Only spaces so treat as an empty folding header. */
return last_header_line_str;
return last_header_line;
}

/* Process folding headers if starting with a space or a tab. */
if (header_line && (*header_line == ' ' || *header_line == '\t')) {
char *http_folded_header_line = header_line;
const char *http_folded_header_line = header_line;
size_t http_folded_header_line_length = *header_line_length;
/* Remove the leading white spaces. */
while (*http_folded_header_line == ' ' || *http_folded_header_line == '\t') {
Expand All @@ -218,27 +213,27 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
ZEND_ASSERT(http_folded_header_line_length > 0);
/* Concatenate last header line, space and current header line. */
zend_string *extended_header_str = zend_string_concat3(
last_header_line, last_header_line_length,
ZSTR_VAL(last_header_line), ZSTR_LEN(last_header_line),
" ", 1,
http_folded_header_line, http_folded_header_line_length);
zend_string_efree(last_header_line_str);
last_header_line_str = extended_header_str;
zend_string_efree(last_header_line);
last_header_line = extended_header_str;
/* Return new header line. */
return last_header_line_str;
return last_header_line;
}
}

/* Find header separator position. */
char *last_header_value = memchr(last_header_line, ':', last_header_line_length);
char *last_header_value = memchr(ZSTR_VAL(last_header_line), ':', ZSTR_LEN(last_header_line));
if (last_header_value) {
/* Verify there is no space in header name */
char *last_header_name = last_header_line + 1;
const char *last_header_name = ZSTR_VAL(last_header_line) + 1;
while (last_header_name < last_header_value) {
if (*last_header_name == ' ' || *last_header_name == '\t') {
header_info->error = true;
php_stream_wrapper_log_warn(wrapper, context, options, InvalidResponse,
"HTTP invalid response format (space in header name)!");
zend_string_efree(last_header_line_str);
zend_string_efree(last_header_line);
return NULL;
}
++last_header_name;
Expand All @@ -247,6 +242,7 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
last_header_value++; /* Skip ':'. */

/* Strip leading whitespace. */
const char *last_header_line_end = ZSTR_VAL(last_header_line) + ZSTR_LEN(last_header_line) - 1;
while (last_header_value < last_header_line_end
&& (*last_header_value == ' ' || *last_header_value == '\t')) {
last_header_value++;
Expand All @@ -256,14 +252,14 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
header_info->error = true;
php_stream_wrapper_log_warn(wrapper, context, options, InvalidResponse,
"HTTP invalid response format (no colon in header line)!");
zend_string_efree(last_header_line_str);
zend_string_efree(last_header_line);
return NULL;
}

bool store_header = true;
zval *tmpzval = NULL;

if (!strncasecmp(last_header_line, "Location:", sizeof("Location:")-1)) {
if (zend_string_starts_with_literal_ci(last_header_line, "Location:")) {
/* Check if the location should be followed. */
if (context && (tmpzval = php_stream_context_get_option(context, "http", "follow_location")) != NULL) {
header_info->follow_location = zend_is_true(tmpzval);
Expand All @@ -281,19 +277,17 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
php_stream_wrapper_log_warn(wrapper, context, options, InvalidResponse,
"HTTP Location header size is over the limit of %d bytes",
HTTP_HEADER_MAX_LOCATION_SIZE);
zend_string_efree(last_header_line_str);
zend_string_efree(last_header_line);
return NULL;
}
if (header_info->location_len == 0) {
header_info->location = emalloc(last_header_value_len + 1);
} else if (header_info->location_len <= last_header_value_len) {
header_info->location = erealloc(header_info->location, last_header_value_len + 1);
/* Previous location header encountered */
if (UNEXPECTED(header_info->location )) {
zend_string_release_ex(header_info->location, false);
}
header_info->location_len = last_header_value_len;
memcpy(header_info->location, last_header_value, last_header_value_len + 1);
} else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) {
header_info->location = zend_string_init(last_header_value, last_header_value_len, false);
} else if (zend_string_starts_with_literal_ci(last_header_line, "Content-Type:")) {
php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0);
} else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) {
} else if (zend_string_starts_with_literal_ci(last_header_line, "Content-Length:")) {
/* https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length */
const char *ptr = last_header_value;
/* must contain only digits, no + or - symbols */
Expand All @@ -304,11 +298,11 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
if (endptr && !*endptr) {
/* truncate for 32-bit such that no negative file sizes occur */
header_info->file_size = MIN(parsed, ZEND_LONG_MAX);
php_stream_notify_file_size(context, header_info->file_size, last_header_line, 0);
php_stream_notify_file_size(context, header_info->file_size, ZSTR_VAL(last_header_line), 0);
}
}
} else if (
!strncasecmp(last_header_line, "Transfer-Encoding:", sizeof("Transfer-Encoding:")-1)
zend_string_starts_with_literal_ci(last_header_line, "Transfer-Encoding:")
&& !strncasecmp(last_header_value, "Chunked", sizeof("Chunked")-1)
) {
/* Create filter to decode response body. */
Expand All @@ -335,10 +329,10 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w

if (store_header) {
zval http_header;
ZVAL_NEW_STR(&http_header, last_header_line_str);
ZVAL_NEW_STR(&http_header, last_header_line);
zend_hash_next_index_insert(Z_ARRVAL_P(response_header), &http_header);
} else {
zend_string_efree(last_header_line_str);
zend_string_efree(last_header_line);
}

return NULL;
Expand Down Expand Up @@ -1040,7 +1034,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
}

if (header_info.location != NULL)
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, ZSTR_VAL(header_info.location), 0);

php_stream_close(stream);
stream = NULL;
Expand All @@ -1051,63 +1045,66 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
}

if (header_info.location != NULL) {
zend_string *new_path = NULL;

/* Redirection location must be a URI with schema,
* if we don't have a schema, the location is within the existing schema and host */
if (ZSTR_LEN(header_info.location) < 8 ||
(!zend_string_starts_with_literal_ci(header_info.location, "http://") &&
!zend_string_starts_with_literal_ci(header_info.location, "https://") &&
!zend_string_starts_with_literal_ci(header_info.location, "ftp://") &&
!zend_string_starts_with_literal_ci(header_info.location, "ftps://"))
) {
zend_string *loc_path = NULL;

char *new_path = NULL;

if (strlen(header_info.location) < 8 ||
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
{
char *loc_path = NULL;
if (*header_info.location != '/') {
if (*(header_info.location+1) != '\0' && resource->path) {
/* if we don't have an absolute location we need to determine it */
if (ZSTR_VAL(header_info.location)[0] != '/') {
if (ZSTR_VAL(header_info.location)[1] != '\0' && resource->path) {
/* find last '/' to determine relative path */
char *s = strrchr(ZSTR_VAL(resource->path), '/');
if (!s) {
s = ZSTR_VAL(resource->path);
if (!ZSTR_LEN(resource->path)) {
zend_string_release_ex(resource->path, 0);
resource->path = ZSTR_INIT_LITERAL("/", 0);
s = ZSTR_VAL(resource->path);
} else {
*s = '/';
}
}
s[1] = '\0';
if (resource->path &&
ZSTR_VAL(resource->path)[0] == '/' &&
ZSTR_VAL(resource->path)[1] == '\0') {
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
/* No path segment, just prefix relative location with '/' */
loc_path = zend_string_concat2(
ZEND_STRL("/"),
ZSTR_VAL(header_info.location), ZSTR_LEN(header_info.location)
);
} else {
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
size_t offset_to_last_slash = s - ZSTR_VAL(resource->path);
ZEND_ASSERT(ZSTR_VAL(resource->path)[offset_to_last_slash] == '/');
loc_path = zend_string_concat2(
ZSTR_VAL(resource->path), offset_to_last_slash + 1,
ZSTR_VAL(header_info.location), ZSTR_LEN(header_info.location)
);
}
} else {
spprintf(&loc_path, 0, "/%s", header_info.location);
loc_path = zend_string_concat2(
ZEND_STRL("/"),
ZSTR_VAL(header_info.location), ZSTR_LEN(header_info.location)
);
}
} else {
loc_path = header_info.location;
header_info.location = NULL;
}
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
spprintf(&new_path, 0, "%s://%s:" ZEND_LONG_FMT "%s", ZSTR_VAL(resource->scheme),
ZSTR_VAL(resource->host), resource->port, loc_path);
new_path = zend_strpprintf(0, "%s://%s:" ZEND_LONG_FMT "%s", ZSTR_VAL(resource->scheme),
ZSTR_VAL(resource->host), resource->port, ZSTR_VAL(loc_path));
} else {
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
ZSTR_VAL(resource->host), loc_path);
new_path = zend_strpprintf(0, "%s://%s%s", ZSTR_VAL(resource->scheme),
ZSTR_VAL(resource->host), ZSTR_VAL(loc_path));
}
efree(loc_path);
zend_string_release_ex(loc_path, false);
} else {
new_path = header_info.location;
header_info.location = NULL;
}

php_uri_struct_free(resource);
/* check for invalid redirection URLs */
if ((resource = php_uri_parse_to_struct(uri_parser, new_path, strlen(new_path), PHP_URI_COMPONENT_READ_MODE_RAW, true)) == NULL) {
if ((resource = php_uri_parse_to_struct(uri_parser, ZSTR_VAL(new_path), ZSTR_LEN(new_path), PHP_URI_COMPONENT_READ_MODE_RAW, true)) == NULL) {
php_stream_wrapper_log_warn(wrapper, context, options, InvalidUrl,
"Invalid redirect URL! %s", new_path);
efree(new_path);
"Invalid redirect URL! %s", ZSTR_VAL(new_path));
zend_string_release_ex(new_path, false);
goto out;
}

Expand All @@ -1119,7 +1116,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
while (s < e) { \
if (iscntrl(*s)) { \
php_stream_wrapper_log_warn(wrapper, context, options, InvalidUrl, \
"Invalid redirect URL! %s", new_path); \
"Invalid redirect URL! %s", ZSTR_VAL(new_path)); \
efree(new_path); \

@devnexen devnexen Jul 28, 2026

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.

If I follow well, this needs an update.

@Girgias Girgias Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It does indeed.

goto out; \
} \
Expand All @@ -1128,7 +1125,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
} \
}
/* check for control characters in login, password & path */
if (strncasecmp(new_path, "http://", sizeof("http://") - 1) || strncasecmp(new_path, "https://", sizeof("https://") - 1)) {
if (zend_string_starts_with_literal_ci(new_path, "http://") || zend_string_starts_with_literal_ci(new_path, "https://")) {

@devnexen devnexen Jul 28, 2026

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.

it seems the old condition was always true regardless ? I would either drop the "migrated" version of it or

if (!zend_string_starts_with_literal_ci(new_path, "http://") && !zend_string_starts_with_literal_ci(new_path, "https://"))
```. wdyt ?

@Girgias Girgias Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm... not even sure what the semantics are. Reading the rest of the code it does seem to me that it expects the path to start with an HTTP protocol schema....

As we are passing new_path to php_stream_url_wrap_http_ex

CHECK_FOR_CNTRL_CHARS(resource->user);
CHECK_FOR_CNTRL_CHARS(resource->password);
CHECK_FOR_CNTRL_CHARS(resource->path);
Expand All @@ -1141,9 +1138,9 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
new_flags |= HTTP_WRAPPER_KEEP_METHOD;
}
stream = php_stream_url_wrap_http_ex(
wrapper, new_path, mode, options, opened_path, context,
wrapper, ZSTR_VAL(new_path), mode, options, opened_path, context,
--redirect_max, new_flags, response_header STREAMS_CC);
efree(new_path);
zend_string_release_ex(new_path, false);
} else {
php_stream_wrapper_log_warn(wrapper, context, options, ProtocolError,
"HTTP request failed! %s", tmp_line);
Expand All @@ -1158,7 +1155,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
}

if (header_info.location != NULL) {
efree(header_info.location);
zend_string_release_ex(header_info.location, false);
}

if (resource) {
Expand Down
Loading