mojo icon indicating copy to clipboard operation
mojo copied to clipboard

Feeding render with Mojo::ByteStream is broken when gzipping is triggered

Open tobez opened this issue 1 year ago • 4 comments

  • Mojolicious version: 9.22 still works, 9.26, 9.31, 9.35 do not
  • Perl version: at least 5.34.0, 5.36.0, 5.38.0
  • Operating system: at least Ubuntu 22.4, MacOS 14.2.1

Steps to reproduce the behavior

Example code:

#! /usr/bin/env perl
use Mojolicious::Lite -signatures;
use Mojo::Collection;

get '/' => sub ($c) {
    my $col = Mojo::Collection->new;
    for (1..10000) {
        push @$col, rand;
    }
    $c->render(data => $col->join("\n"));
};

app->start;

./mojo-bug get /

Expected behavior

It should produce 10000 lines of random numbers as the output.

Actual behavior

It first says "Mojo::Reactor::EV: I/O watcher failed: A response has already been rendered", then dies with inactivity timeout.

Analysis

IO::Compress::Gzip::gzip, which Mojolicious uses internally, accepts several different things as its input parameter, and Mojo::ByteStream is not one of those things.

I am unsure what the best solution would be, hence the bug report instead of a pull request. Possibilities:

  • in Mojo::Util::gzip, stringify $uncompressed first
  • in Mojolicious::Renderer::respond, check whether $output is a Mojo::ByteStream, and either stringify it or use $output->gzip directly, which would work
  • document that render() should not be fed Mojo::ByteStream (would violate principle of least astonishment)
  • something I did not think about

tobez avatar Jan 25 '24 17:01 tobez

Not sure which solution is best here. 🤔

kraih avatar Jan 25 '24 17:01 kraih

I think I'm in the right place, but could we check if it is a blessed Mojo::ByteStream here ? https://github.com/mojolicious/mojo/blob/main/lib/Mojolicious/Renderer.pm#L137 or could we even check that if it is blessed and overloads stringify that we stringify it, because any object would fail there.

jberger avatar Jan 25 '24 19:01 jberger

Oh, and that gzip is imported from Mojo::Util, so even better, check here: https://github.com/mojolicious/mojo/blob/main/lib/Mojo/Util.pm#L164

jberger avatar Jan 25 '24 19:01 jberger

And now that I've reread the OP, both of those were suggestions ... sigh, one day I'll learn to read back. IMO, I'd check it at Mojo::Util::gzip.

jberger avatar Jan 25 '24 19:01 jberger