Skip to content

Implement GH-19249: http context - allow content to be a stream/resource #19267

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
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nielsdos
Copy link
Member

This may be useful for sending large chunks of data.

…source

This may be useful for sending large chunks of data.
*str_out = Z_STR_P(content);
return true;
} else if (Z_TYPE_P(content) == IS_RESOURCE) {
if ((php_stream_from_zval_no_verify(*stream_out, content))) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ((php_stream_from_zval_no_verify(*stream_out, content))) {
if (php_stream_from_zval_no_verify(*stream_out, content)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is used to silence a compiler warning because php_stream_from_zval_no_verify expands to an assignment

Comment on lines 40 to 41
file_get_contents(): supplied resource is not a valid stream resource
file_get_contents(): supplied resource is not a valid stream resource
Copy link
Member

Choose a reason for hiding this comment

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

This is a confusing error message, as it is in the stream context the problem occurs really, not in the file_get_contents

Comment on lines 378 to 386
zend_off_t current_position = php_stream_tell(content_stream);
if (php_stream_seek(content_stream, 0, SEEK_END) < 0) {
return false;
}
zend_off_t end_position = php_stream_tell(content_stream);
if (php_stream_seek(content_stream, current_position, SEEK_SET) < 0) {
return false;
}
smart_str_append_unsigned(req_buf, end_position - current_position);
Copy link
Member

Choose a reason for hiding this comment

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

What about the non seekable streams?

I'm thinking that it might be better to use chunked encoding for such cases

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively you will need to fetch them to memory so it won't give much memory saving in this case but I guess files are bigger use case for this so it's probably better than nothing (considering that addition of chunked encoding support might be quite involved).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll go with the "read to memory" solution as I believe that non-seekable streams are pretty rare in this context.

@nielsdos nielsdos marked this pull request as ready for review July 27, 2025 16:41
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.

http context - allow content to be a stream/resource
3 participants