Skip to content
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

PhpInputStream's getSize function is Wrong! #2

Closed
2 tasks
weierophinney opened this issue Dec 31, 2019 · 6 comments
Closed
2 tasks

PhpInputStream's getSize function is Wrong! #2

weierophinney opened this issue Dec 31, 2019 · 6 comments
Labels
Invalid This doesn't seem right

Comments

@weierophinney
Copy link
Member

reference: https://www.php.net/manual/en/wrappers.php.php
php://input is not support fstat

Supports stat() php://memory and php://temp only.
  • I was not able to find an open or closed issue matching what I'm seeing.
  • This is not a question. (Questions should be asked on chat (Signup here) or our forums.)

Provide a narrative description of what you are trying to accomplish.

Code to reproduce the issue

Expected results

Actual results


Originally posted by @zhushengwen at zendframework/zend-diactoros#380

@weierophinney
Copy link
Member Author

@zhushengwen

fstat and stat are two different PHP functions.

Can you provide failing test case, please?


Originally posted by @michalbundyra at zendframework/zend-diactoros#380 (comment)

@weierophinney
Copy link
Member Author

image


Originally posted by @zhushengwen at zendframework/zend-diactoros#380 (comment)

@weierophinney
Copy link
Member Author

@zhushengwen
Please create a pull request with an unit test which illustrates the problem.
It's very hard to reproduce the problem per screenshot. 😉

Thanks in advance!


Originally posted by @froschdesign at zendframework/zend-diactoros#380 (comment)

@Xerkus
Copy link
Member

Xerkus commented May 2, 2023

fstat() can not be used on input stream. Stream needs to be fully read into internal cache property before the size could be determined but read should not happen on getSize() invocation.

Because size can not be determined for php input stream getSize() returns null and that is a correct behavior.

However, investigating this issue I discovered another problem that I will report as a separate issue.

@Xerkus Xerkus closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2023
@boesing
Copy link
Member

boesing commented May 2, 2023

I recently had a look into guzzle and they do in fact pass the stream over to a temp stream:
https://github.com/guzzle/psr7/blob/fe18df3deb34d1e0eab8a63ef4fe2aeb2a8e8c94/src/Utils.php#L305-L310

Not sure if we should handle this as well.

@Xerkus
Copy link
Member

Xerkus commented May 2, 2023

We use property to store cached content. It should really be temp stream, but I do not think we should eagerly read it for metadata and such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants