pmlopes / yoke

Yoke is a middleware framework for Vert.x
http://pmlopes.github.io/yoke/
Apache License 2.0
157 stars 44 forks source link

Compress middleware is not working correctly #138

Closed dop251 closed 9 years ago

dop251 commented 10 years ago

Hello,

I'm trying to make use of the Compress middleware and found out that although it sets the encoding header correctly the actual content comes uncompressed. I believe this is due to a bug in the following method in AbstractWriterFilter:

public Buffer end(@NotNull final Buffer buffer) {
    end(buffer.getBytes());
    return buffer;
}

Surely this should be "return this.buffer"?

I also suspect that even after this bug is fixed it still won't work for files because the filtering is not enabled in YokeResponse.sendFile().

Is there any plan to make this functionality work?

There is an alternative solution -- server.setCompressionSupported(true), but that turns compression for everything, including images, archives, etc. Also it does not set "vary: accept-encoding" which breaks public proxies.

Many thanks.

pmlopes commented 10 years ago

Hi,

this middleware was implemented before the setcompression was available. It has the limitation that files and streams were not compressed and compression happens using blocking api...

Sure it needs more testing and i think a rewrite to use the setcompressons instead of the filters.

-----Original Message----- From: dop251 Sent: 06/10/2014, 18:28 To: pmlopes/yoke Subject: [yoke] Compress middleware is not working correctly (#138)

Hello,

I'm trying to make use of the Compress middleware and found out that although it sets the encoding header correctly the actual content comes uncompressed. I believe this is due to a bug in the following method in AbstractWriterFilter:

public Buffer end(@NotNull final Buffer buffer) {
    end(buffer.getBytes());
    return buffer;
}

Surely this should be "return this.buffer"?

I also suspect that even after this bug is fixed it still won't work for files because the filtering is not enabled in YokeResponse.sendFile().

Is there any plan to make this functionality work?

There is an alternative solution -- server.setCompressionSupported(true), but that turns compression for everything, including images, archives, etc. Also it does not set "vary: accept-encoding" which breaks public proxies.

Many thanks.


Reply to this email directly or view it on GitHub: https://github.com/pmlopes/yoke/issues/138