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

CachingSteam returns incorrect stream size (call to getSize) #365

Closed
psdesse opened this issue Dec 15, 2020 · 4 comments
Closed

CachingSteam returns incorrect stream size (call to getSize) #365

psdesse opened this issue Dec 15, 2020 · 4 comments

Comments

@psdesse
Copy link

psdesse commented Dec 15, 2020

PHP version: 7.3.19

Description
getSize on CachingSteam does not handle cases when the remote steam does not know its size (ie, returns null on getSize). In this case, the size returned by the CachingStream is incorrect. It does a max between null and the cached size, which will always return the cached size even if the full stream was not cached.

How to reproduce
Create a CachingStream from a stream that does not know its size (returning null when calling getSize).

Possible Solution
As a first idea, I am wondering why the getSize on the CachingStream does not only returns the size advertised by the remote stream. However, we could improve this by knowing if the full stream has been cached. In this case, the size is known and it could returns.

Additional context
In the seek function, the case were the remote stream returns null on getSize is correctly handled.

@Nyholm
Copy link
Member

Nyholm commented Mar 21, 2021

I am wondering why the getSize on the CachingStream does not only returns the size advertised by the remote stream.

I think this makes sense too.

I agree with your suggestion.

@stale
Copy link

stale bot commented Jul 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@GrahamCampbell
Copy link
Member

Not stale.

@Nyholm
Copy link
Member

Nyholm commented Oct 4, 2021

Closed by #438

@Nyholm Nyholm closed this as completed Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants