Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tobez opened this issue Jan 25, 2024 · 4 comments
Open

Comments

@tobez
Copy link

tobez commented Jan 25, 2024

  • 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
@kraih
Copy link
Member

kraih commented Jan 25, 2024

Not sure which solution is best here. 🤔

@jberger
Copy link
Member

jberger commented Jan 25, 2024

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
Copy link
Member

jberger commented Jan 25, 2024

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
Copy link
Member

jberger commented Jan 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants