Repository navigation
Conversation
…chunk sizes Exif uses php_stream_read() to read data from the stream and process it. In this case, it's a userspace stream. Userspace streams can either by local or url streams. In 5060fc2 and subsequently 0a45e8f, the behaviour of the check was changed twice in total. In the latter commit the description talks about exceptions for streams that are local to the process. It looks like the exception for local userspace streams was forgotten. This patch updates the check such that local userspace streams also read greedily, while keeping url userspace streams unchanged. This also updates the existing test to test both local and url userspace streams.
|
Thanks to you both for the review. I fixed the comments. |
|
I'm not sure that this is a good idea as it might break some user space wrappers that expect to not be greedily read. I don't exactly see the complete correlation between url wrapper and greedy reading. For sure if it's URL one it should not read greedily but why non url ones needs to be read greedily? I think the fix should really happen in exif to handle non greedily read streams which would be also functionality improvement as you could just provide image URL and it would work (assuming that it is not currently supported when there's an issue with non greedy reading). |
|
Hmm I see. |
|
As a side note I think it would make sense to have that information (whether it uses greedy and non greedy read) available in the stream struct so it can be checked elsewhere (e.g. in exif). That could be even set during the stream creation so we don't have that ugly ops check in the read that lists all greedy stream ops (it would be just a simple field check). |
Exif uses php_stream_read() to read data from the stream and process it. In this case, it's a userspace stream. Userspace streams can either by local or url streams.
In 5060fc2 and subsequently 0a45e8f, the behaviour of the check was changed twice in total. In the latter commit the description talks about exceptions for streams that are local to the process. It looks like the exception for local userspace streams was forgotten. This patch updates the check such that local userspace streams also read greedily, while keeping url userspace streams unchanged.
This also updates the existing test to test both local and url userspace streams.
Targeting master because I think this could be a breaking change.