-
Notifications
You must be signed in to change notification settings - Fork 573
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
Comments
Not sure which solution is best here. 🤔 |
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. |
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 |
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. |
Steps to reproduce the behavior
Example code:
./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:
$uncompressed
first$output
is a Mojo::ByteStream, and either stringify it or use$output->gzip
directly, which would workrender()
should not be fed Mojo::ByteStream (would violate principle of least astonishment)The text was updated successfully, but these errors were encountered: