> Streams SHOULD have their internal pointer set to the end of the 
underlying resource.

I have to strongly disagree here, and would suggest:

Stream MUST have their internal pointer set of the start of the underlying 
resource.

Please consider existing implementations here.

Diactoros rewinds streams created from strings:

https://github.com/zendframework/zend-diactoros/blob/master/src/StreamFactory.php#L30

It leaves streams created from a file path with the pointer at zero:

https://github.com/zendframework/zend-diactoros/blob/master/src/StreamFactory.php#L40

https://github.com/zendframework/zend-diactoros/blob/master/src/Stream.php#L339

Emitters of existing libraries do not rewind streams:

https://github.com/narrowspark/http-emitter/blob/master/src/SapiEmitter.php#L33

https://github.com/lunephp/http-emitter/blob/master/src/ResponseEmitter.php#L41

https://github.com/zendframework/zend-httphandlerrunner/blob/master/src/Emitter/SapiEmitter.php#L40

Introducing a requirement to move the file-pointer to the end when creating 
a stream from a file path is a serious breaking change versus any existing 
emitter implementation I could find - so mandating a change at this point 
could create serious problems.

It also has a negative impact on performance in the most common scenario, 
where you're just opening a file so you can attach it as the body of a 
Response - you'll end up unnecessarily seeking to the end of the file, just 
so you can rewind it again and then read it. So this could be harmful to 
performance of the majority use-case.

Based on this, I'd suggest the following amendments to the specification of 
StreamFactoryInterface:

1. createStream() MUST return a stream with the internal file-pointer at 0.
2. createStreamFromFile() MUST return a stream with the internal 
file-pointer at 0.
3. createStreamFromResource() MUST NOT modify the internal file-pointer.

Both createStream() and createStreamFromFile() create Stream instances from 
existing content, so they should work consistently, and the majority 
use-case for opening a stream with existing content is to read/transmit 
that content, e.g. in the body of a Response.

As for createStreamFromResource(), we can't/shouldn't make any assumptions 
- if the user decides to open the stream on their own, rather than using 
one of the other two methods, we should assume they know what they're 
doing. The resource could be a streaming resources from another HTTP 
server, for example, so doing anything with the file-pointer could either 
break something (if the file stream isn't repeatable) and/or have a 
potentially critical performance impact (such as accidentally reading the 
whole stream twice.)

On Saturday, December 29, 2018 at 5:31:57 PM UTC+1, Woody Gilk wrote:
>
> Good report, Martijn!
>
> From my perspective, it would make sense that the position would be always 
> be set to the end of the stream, as if the stream had just been written, 
> mirroring the fopen(), fwrite() sequence that would normally be done.
>
> I'm not quite sure on the bylaws about level of recommendation, but an 
> errata to the following would cover this:
>
> Streams SHOULD have their internal pointer set to the end of the 
> underlying resource.
>
> FIG, would having this as an errata be acceptable?
> --
> Woody Gilk
> https://shadowhand.me
>
>
> On Fri, Dec 28, 2018 at 6:07 PM Martijn van der Ven <[email protected] 
> <javascript:>> wrote:
>
>> There seems to be some disagreement on where the file position indicator 
>> (cursor) ends up upon the creation of a Stream instance. I have done some 
>> minor testing and put the results on GitHub 
>> <https://github.com/Zegnat/php-psr17-test-cursor>. This can lead to 
>> incompatibilities when doing something like:
>>
>> echo (new StreamFactory)->createStream('Hello!')->read(6);
>>
>>
>>    - createStream() seems to be the most contested. The cursor goes at 
>>    the start or end of the string. Even more surprising (to me) is that 
>>    Diactoros will result in a different state depending on whether you use 
>> its 
>>    own factory or the tuupola one. (Again showing how compatibility may be 
>> an 
>>    issue here!)
>>    - createStreamFromResource() seems to be agreed upon by all 
>>    implementations: keep the cursor wherever it was at in the original 
>>    resource. Even without specifically being defined by the PSR this makes a 
>>    lot of sense to me, and I will propose adding a test for this to the 
>>    (official) unit tests.
>>    - createStreamFromFile() has one of the tested implementations do its 
>>    own thing: berlioz/http-message copies the contents of the file into a 
>> new 
>>    stream and puts the cursor at the end. Every other implementations puts 
>> the 
>>    cursor at the start, matching where it would be if fopen() is called on a 
>>    file.
>>    
>> Of course the (official) unit tests 
>> <https://github.com/http-interop/http-factory-tests> that go with the 
>> PSR-17 interfaces do not have tests for any of this as it isn’t covered by 
>> the PSR itself.
>>
>> The default for createStream() is a coin toss to me. I see no clear 
>> argument for either position. But I do think the PSR should define a 
>> location. This does not sound like something that can just be errata’d in, 
>> and defining behaviour now will lead to breaking changes in implementations 
>> :( Not sure what the way is to start addressing this.
>>
>> For *FromResource() there seem to be good reasons to just leave the 
>> cursor be where it is, and I believe this can be mentioned by the PSR. As 
>> all implementations do this already, maybe this can be an errata of some 
>> kind?
>>
>> For *FromFile() I don’t know what can be defined, other than saying it 
>> should follow what fopen() does by default. For files this will mean the 
>> cursor position is set to the start of the stream, I am not sure what the 
>> cursor does for other protocols/wrappers/schemes.
>>
>> Ideas?
>>
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "PHP Framework Interoperability Group" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to [email protected] <javascript:>.
>> To post to this group, send email to [email protected] 
>> <javascript:>.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/php-fig/508ab231-63e2-485c-9a9a-0ff3554815ca%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/php-fig/508ab231-63e2-485c-9a9a-0ff3554815ca%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>> For more options, visit https://groups.google.com/d/optout.
>>
>

-- 
You received this message because you are subscribed to the Google Groups "PHP 
Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/php-fig/7abc602f-d17a-4f49-8736-33ebc4fe8d81%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to