3

The PSGI specification defines the HTTP response as consisting of three parts, the third of which may be either an array reference or a filehandle. The filehandle may be:

An IO::Handle-like object or a built-in filehandle.

And the spec goes on to say:

Servers MAY check if the body is a real filehandle using fileno and Scalar::Util::reftype and if it's a real filehandle that has a file descriptor, it MAY optimize the file serving using techniques like sendfile(2).

Now I've cobbled together a command-line example using plackup (Plack version 0.9978), and it appears that checking if the body is a real filehandle results in a fatal error:

Can't locate object method "FILENO" via package "IO::Scalar" at /usr/lib/perl5/5.10/i686-cygwin/IO/Handle.pm line 390

Here's the command-line example:

plackup -MData::Dumper -MIO::Scalar -e \
'sub { $env=shift; return [200, [], IO::Scalar->new(\Dumper $env) ] }'

Of course I could just not use a filehandle:

plackup --port 9999 -MData::Dumper -e \
'sub { $env=shift; return [200, [], [Dumper $env] ] }'

But I'm interested in what works and what doesn't. So shouldn't Plack exercise more caution when calling FILENO on the handle so it wouldn't run into an exception?

And to add another one:

plackup --port 9999 -MData::Dumper -e \
'sub{$env=shift; $s=Dumper $env; open $fh,q(<),\$s or die; return [200,[],$fh ]}'

Looks like the filehandle isn't recognized as such. The error message is:

body should be an array ref or filehandle at /usr/lib/perl5/site_perl/5.10/Plack/Middleware/StackTrace.pm line 35

Update:

As ysth stated in his answer, the following will work (at least on 5.10.1 on Cygwin):

plackup -p 9999 -MData::Dumper -MIO::String -e \
'sub { return [200, [], IO::String->new(\Dumper shift) ] }'

But clearly, there is an issue someplace as can be seen from the failing examples, and it will be reported once I've made up my mind what it actually is.

Lumi
  • 14,775
  • 8
  • 59
  • 92

3 Answers3

9

These are not bugs - It's actually easier to call this a bug in Plack and fix it to handle them as a valid response. But that would make things worse, since now Plack handles things that are not (clearly) defined as a proper response in the PSGI specification. (PSGI != Plack, likewise HTTP != Apache)

The point of PSGI specification is that it is a common interface between web servers and applications. If a server/application needs to add extra 2-3 lines of code to conform to the specification, that is a good compromise. It's much better to have 2-3 lines of code in each N applications and M servers, than having N * 2-3 lines of extra code to handle corner cases in servers and vice versa in applications.

The spec defines the response body should be either a "built-in filehandle" or "IO::Handle-like object that implements getline". Handling something that works similar to this is easy in Plack, but we shouldn't blindly do it - remember, Plack is not the only PSGI implementation. The Lint middleware warning you about that incompatibility is a right thing.

That said:

a) IO::Scalar is an object that implements getline() method, so it should be accepted. It's unfortunate that the Lint dying on it because of the bug of the module, as pointed out by others. It could easily be worked around with the monkey patching, and it's also easy to patch Plack::Util::is_real_fh to trap errors in the ->fileno call, but again, I need to think about it if it is a right thing to do.

b) PerlIO in-memory filehandle is a tricky thing. The spec only says "built-in filehandle" and the in-memory filehandle could be argued as a built-in thing as well. Actually, if you disable the Lint middleware (with -E production option for plackup, for example), the filehandle would just work. But again, the Lint middleware gives you a message because it's not guaranteed to work elsewhere.

At last but not least, this should probably be addressed in the FAQ. Feel free to open a case in the psgi-specs repository.

miyagawa
  • 1,329
  • 7
  • 9
  • It seems pretty clear to me from the spec that in memory filehandles are ok. They implement `getline` and `close` and return false from `fileno` to prevent `sendfile` optimizations. And if an object implementing `getline` and `close`, which could be doing anything, is ok it would seem in-memory filehandles should be too. What are these scenarios where they won't work? – Schwern May 16 '11 at 04:26
  • 1
    It *should* be okay as long as the server doesn't check the response type to figure out what to do - at least we have ARRAY ref and an object or a filehandle, so the *straightforward* implementation would be `if (ref $res->[2] eq 'ARRAY') { respond_from_array() } else { # do something with $res->[2]->getline }`. Actually that is what PSGI documentation suggests - so you're right that it should mostly work. It is a bit tricky for Lint to make sure whether the filehandle implements getline() without special casing GLOB ref. It may probably have to do so, though. – miyagawa May 16 '11 at 05:17
  • 1
    `open my $io, "<", \"Hello"; $io->can("getline"); # false / $io->getline() # actually works` this is why it is tricky. See [Yuval's post](http://blog.woobling.org/2009/10/are-filehandles-objects.html) for more details. (and that's why blessing into IO::Handle is a workaround) – miyagawa May 16 '11 at 05:22
  • @miyagawa, thanks for your answer and for your work on the PSGI/Plack development. I'm aware of PSGI != Plack and what's a specification and what's an implementation, but it appears to me, too, that the spec makes it clear plain old filehandles are okay. If you're stipulating that `$io->can("getline")` should evaluate to true, than the bug would, it seems, have to be raised against the spec, which should unambiguously state the requirements an implementation will have to fulfil. – Lumi May 16 '11 at 07:03
  • I would suggest the check in validate_res be modified to add an additional || condition: `(ref $res->[2] eq 'GLOB' && *{$res->[2]}{IO} && *{$res->[2]}{IO}->can('getline'))` – ysth May 16 '11 at 07:20
  • ysth, that looks like an insane code but sounds sane to do in the Lint :) I'll take a shot. – miyagawa May 16 '11 at 17:16
  • @michael-ludwig it is more of a bug in perl (see yuval's post) and making a workaround in the PSGI spec sounds like a wrong thing to me. That said, I updated the Lint to pass through such filehandles on git master. Again, I won't be surprised if some PSGI web servers (outside Plack, for instance, uwsgi, mod_psgi, evpsgi) do not like the in-memory file handles, in the same way Lint didn't like it. – miyagawa May 16 '11 at 17:21
  • @miyagawa Given that PSGI is the *Perl* Server Gateway Interface, it should make some allowances for very long standing design bugs in Perl. In particular ones where there's no consequences if it gets fixed (ie. things aren't going to break if `$fh->can("getline")` starts working) But does lint need to verify that a filehandle has getline? Is that guaranteed once IO::Handle is loaded? Plack::Util doesn't check it for filehandles which have a fileno. – Schwern May 16 '11 at 18:31
  • In general, I disagree that it should make allowances for *all* Perl bugs - I admit there's some aspect of it in PSGI already, but we designed it in 2009, and there was no PSGI implementation at that time - There was no reason to make the specification as unnecessarily complicated to support Perl bugs. But again, that's a general statement, and while we allow a file-based bare filehandles, disallowing in-memory based perlIO filehandles do not make much sense. So it's now fixed. – miyagawa May 16 '11 at 18:40
  • "But does lint need to verify that a filehandle has getline?" Web servers usually shouldn't, but it's a good idea for Lint to make sure, since Lint is to make sure you code is sane. "Is that guaranteed once IO::Handle is loaded?" per my discussion with p5p it's not *guaranteed*, but it was once broken in some devel releases during 5.11.*, and the Plack unit tests caught that issue. "Plack::Util doesn't check it for filehandles which have a fileno. " Yes - it's not fair to prefer the filehandles that has IO *with* fileno while the ones *without* fileno. Now fixed. – miyagawa May 16 '11 at 18:42
  • See [this thread](http://www.nntp.perl.org/group/perl.perl5.porters/2010/01/msg156143.html) for the breakage of IO::Handle and seek/getline methods in 5.11. – miyagawa May 16 '11 at 18:44
  • @miyagawa, I've read Yuval's post, which is interesting. But there's nothing in it that makes me think there is a bug in Perl. Kludge or design bug, maybe, to make things work; real bug, no. So I feel that if PSGI requires "built-in filehandle" (as it does), all implementations should deliver "built-in filehandle". So thanks for fixing Plack in that respect to now allow memory filehandles! :-) - As for `IO::Scalar` not implementing `fileno`, that seems to be a shortcoming of that module. (AFAICS, there's no spec for what it must implement, so you can't call it a bug.) – Lumi May 16 '11 at 20:48
  • It's fine if you won't call it a bug - there are some certain areas that filehandles work in a weird way (i.e. `$fh->can('getline')` is false but `$fh->getline` works) - and in PSGI, the new spec we built in 2009, it's better we make some compromise in either side (in the application or the server) so that we can ignore these kludge / design bugs. – miyagawa May 16 '11 at 21:04
  • @miyagawa Thanks for all the patches! FWIW, IMO checks against bugs in old dev versions of Perl in production code... that way madness lies. – Schwern May 18 '11 at 00:47
8

This appears to be a bug in Plack. It tries to figure out if it has a real filehandle, via fileno, and if not it will only accept objects with a getline method. This misses out on both tied filehandles without FILENO defined (valid, if impolite) and in memory filehandles which do not have a valid fileno nor are they blessed objects. You can see it in the logic in Plack::Middleware::Lint->validate_res and Plack::Util->is_real_fh.

I'd report it to Plack as a bug.

Meanwhile, you can work around the problem in IO::Scalar by defining IO::Scalar::FILENO to return undef.

sub IO::Scalar::FILENO { return }

This would be an improvement to IO::Scalar, but it hasn't been updated in six years so I wouldn't hold my breath.

To allow in memory filehandles, you can trick Plack by blessing the filehandle. Sometime between opening it and handing it off, do this:

bless $fh, "IO::Handle";

That's harmless as any filehandle will respond to IO::Handle methods anyway. But also please do report it as a bug.

Schwern
  • 153,029
  • 25
  • 195
  • 336
  • Normal in memory filehandles don't have a problem (if you've used IO::Handle); it's just IO::Scalar not living up to its promise of acting like an IO::Handle. – ysth May 16 '11 at 04:36
  • @ysth, could you clarify how "in memory filehandles don't have a problem"? In my example they clearly do, with or without `IO::Handle`: `plackup -MData::Dumper -MIO::Handle -e 'sub { open $fh, "<", \Dumper shift; return [200, [], $fh ] }'` (Perl 5.10.1 on Cygwin) – Lumi May 16 '11 at 06:53
  • @Schwern, thanks. I will report any bugs once it is clear to me what exactly they are. (At the moment, I'd say they're in Plack, which appears to fail to live up to PSGI when it comes to the statement: "An IO::Handle-like object or a built-in filehandle.") – Lumi May 16 '11 at 06:56
  • Oh, sorry, they don't have a problem with calling fileno, but don't pass the later blessed and can->('getline') checks; your suggested blessing works for that. – ysth May 16 '11 at 07:02
  • As does using an ioref instead of a globref: `open $fh, "<", \$buffer; $fh=*$fh{IO}` or `$fh = Symbol::geniosym; open $fh, "<", \$buffer` – ysth May 16 '11 at 07:10
  • @Schwern, now that @miyagawa fixed what was arguably a bug against the PSGI spec ("built-in filehandle") we don't have to report it any more. Do you still think Plack's failure to deal with `IO::Scalar` should be reported as a bug? You said "tied filehandles without FILENO defined (valid, if impolite)", but to me, doing `perl -MIO::Scalar -lwe '$fh=IO::Scalar->new; print $fh->fileno'` (which dies with `IO::Handle` blaming `IO::Scalar`) suggests that `IO::Scalar` is indeed to blame here. (Suggests, because there is no spec, only an `IO::Handle` implementation.) – Lumi May 16 '11 at 21:08
  • I've reported the bug in `IO::Scalar` and the solution (`sub IO::Scalar::FILENO { return }`) to the email address listed as the maintainer of that module. We'll see. – Lumi May 16 '11 at 22:30
  • @Lumi By 'valid' I meant according to the docs in perltie, but its asking for trouble to not have all the methods defined. So yes, IO::Scalar should be patched to define FILENO. It will prevent a lot of headaches. – Schwern May 18 '11 at 00:44
  • @Schwern - Good news: The maintainer of `IO::Scalar` said he's going to update the module. :-) And I'm going to read `perltie` one of these days. – Lumi May 18 '11 at 07:37
0

Looks like a bug in IO::Scalar. Report it, and insteaad use IO::String or the built-in in memory file support added in 5.8.

ysth
  • 96,171
  • 6
  • 121
  • 214
  • thanks. I've updated my question to reflect the fact that IO::String does work. Could you clarify how you see "built-in memory file support" working? I'm assuming you mean opening a filehandle to a scalar holding a string in memory. That fails in the example I gave in my question, regardless of whether or not `IO::Handle` is loaded or not. At least in Perl 5.10.1 on Cygwin. – Lumi May 16 '11 at 06:47
  • I just tested fileno, not the other things plack is doing; see my comments elsewhere. – ysth May 16 '11 at 07:21