http icon indicating copy to clipboard operation
http copied to clipboard

Interest to support Brotli?

Open hartator opened this issue 6 years ago • 4 comments

Brotli is becoming more common on servers thanks to its debatable better performance over gzip. It's accessible same way as gzip via HTTP.headers("Accept-Encoding" => "gzip, deflate, br").

Unfortunately, http.rb doesn't support br auto-inflation.

We have to support it in our application. Any interest for a PR?

hartator avatar Aug 13 '18 20:08 hartator

In general yes. Please open a PR so we will be able to start discussion on how to better implement this :D

I think we should improve use(:auto_inflate) api to accept list of "supported" and desired encodings (it's not accepting anything like that atm afaik). And it should require dependency only when given list includes brotli.

Alternatively we can check if brotli gem is available and set accept-encoding to gzip, deflate or gzip, deflate, br depending on that.

ixti avatar Aug 13 '18 21:08 ixti

Hum, yeah, it will probably require either to bundle http.rb with the brotli gem, or conditional Accept-Encoding headers based on gem already here or not. Can be messy though.

So far, we are using this in our app, and it's working, as a drop-in replacement of use(:auto_inflate):

class AutoInflater

  def self.inflate response
    if %w[deflate gzip x-gzip].include?(response.headers[:content_encoding])
      ActiveSupport::Gzip.decompress(response.to_s)
    elsif response.headers[:content_encoding] == 'br'
      Brotli.inflate(response.to_s).force_encoding(response.charset)
    else
      response.to_s
    end
  end

end

I was wondering if there is any downside of doing this instead of using the readpartial buffer apart having to read the full response.

I can probably tackle this weekend with a proper support of http.rb buffer and internals.

hartator avatar Aug 13 '18 22:08 hartator

The idea of using readpartial is to avoid sucking whole body into memory. :D

ixti avatar Aug 13 '18 23:08 ixti

@ixti, what's your current vision about the Brotli support comparing to three years ago?

Currently the brotli gem doesn't support streaming inflater but that's feasible.

ilyazub avatar Apr 14 '21 12:04 ilyazub