[PSR-17] createStreamFromResource MUST create a readable Stream out of a non-readable resource?

91 views
Skip to first unread message

Martijn van der Ven

unread,
Sep 2, 2018, 3:04:52 PM9/2/18
to PHP Framework Interoperability Group
I have been spending some time this weekend with the HTTP Interop unit tests for HTTP Factories (PSR-17) and ended up puzzling about createStreamFromResource’s strict (RFC 2119 “MUST”) requirement on the output Stream being readable:

/**
 * Create a new stream from an existing resource.
 *
 * The stream MUST be readable and may be writable.
 *
 * @param resource $resource The PHP resource to use as the basis for the stream.
 */

public function createStreamFromResource($resource): StreamInterface;

For argument’s sake, say we have the following code:

$resource = fopen(tempnam(sys_get_temp_dir(), 'psr17'), 'w');
fwrite
($resource, 'Foobar');
$stream
= $factory->createFromResource($resource);

What functionality should exist on the returned Stream? The interface is telling me it must be readable, but we don’t have a clean way to read from the existing resource at all and can probably never access the “Foobar” string that already exists in the file.

I would have expected the interface to define a \RuntimeException in case the provided $resource is unreadable. Like how createStreamFromFile defines it for files it couldn’t open. Or to just not have put this requirement on the output at all, createStreamFromFile allows write-only streams to be created.

I couldn’t find any sort of discussion about this in the meta document or on the (now deprecated) proposal GitHub repo, so maybe someone here can tell me why this method is the way it is. (Sadly I didn’t spot this back when I was working with it during its proposal stage.)

Regards,

Martijn van der Ven
https://vanderven.se/martijn/

Woody Gilk

unread,
Sep 4, 2018, 9:27:59 AM9/4/18
to PHP Framework Interoperability Group
Thanks for bringing this up, Martijn. I agree that bit is rather
awkward and should be clarified and tested better.

I think the docblock was intended to read:

> The resource MUST be readable and may be writable.

However, even if that was the case, how would it be enforced? I don't
know of a way to check if a resource is readable.

This may require an errata to PSR-17.
--
Woody Gilk
https://shadowhand.me
> --
> 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 php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/1ad6929e-ab43-4a38-8b29-cf5bd2799062%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Martijn van der Ven

unread,
Sep 4, 2018, 10:26:12 AM9/4/18
to PHP Framework Interoperability Group
On Tuesday, 4 September 2018 15:27:59 UTC+2, Woody Gilk wrote:
Thanks for bringing this up, Martijn. I agree that bit is rather
awkward and should be clarified and tested better.

I think the docblock was intended to read:

> The resource MUST be readable and may be writable.

However, even if that was the case, how would it be enforced? I don't
know of a way to check if a resource is readable.

If the word “stream” in the docblock referred to a stream-type resource (cf. resource types) you should be able to use stream_get_meta_data() and check whether the mode allows reading. The fact that stream may have two different meanings there is confusing in and of itself. Though even then figuring out exactly when to error out is frustrating:

if (false === is_resource($resource) ||
   
'stream' !== get_resource_type($resource) ||
   
true === array_key_exists('stream_type', $metadata = stream_get_meta_data($resource)) && $metadata['stream_type'] === 'dir' ||
   
false === array_key_exists('mode', $metadata) ||
    in_array
($metadata['mode'], ['r', 'r+', 'w+', 'a+', 'x+', 'c+'])
) {
   
throw new \InvalidArgumentException('resource must be readable and of type stream.');
}

(N.b. the in_array() check displayed there isn’t completely accurate. E.g. mode may contain t or b flags. Doing a proper mode check is left as an exercise to the reader ;-) )

This may require an errata to PSR-17.

I agree. If only to standardise on an exception (RuntimeException? InvalidArgumentException?) to throw when the implementation cannot use the provided resource.

But preferably the errata can also clarify what sort of resources are expected to be handled for interop reasons. As a consumer of the interface I would like to know that I can always feed it a resource from fopen(). Whether an implementation also supports zlib type resources (from gzopen()) is up to them and not guaranteed by the interface. Or is setting a minimum implementation requirement outside of the scope for the PSR?

I would love to hear if there is anything I can do to help this take shape. I am willing to put some time into getting this clarified for myself and everyone else.

Stefano Torresi

unread,
Sep 4, 2018, 10:37:04 AM9/4/18
to php...@googlegroups.com
Hey Woody,

I think the docblock was intended to read:

> The resource MUST be readable and may be writable.

Yep; it appears to be so by reading the original docblock, before splitting the polymorphic method:
 
However, even if that was the case, how would it be enforced? I don't
know of a way to check if a resource is readable.

I guess with stream_get_meta_data ? That's an implementation detail, though.
 
This may require an errata to PSR-17.

Yup.
Reply all
Reply to author
Forward
0 new messages