Using Propshaft::Asset#content with UTF-8 encoding

[denzelem]

Recently I tried to use inline styles with propshaft to render PDFs with https://github.com/Studiosity/grover. I figured out, that using Propshaft::Asset#content does not work well, when reading UTF-8 encoded files.

!!!
%html
  %head
    %meta(content='text/html; charset=UTF-8' http-equiv='Content-Type')
    %meta(name='viewport' content='width=device-width, initial-scale=1')
    %title= @title
    = inline_stylesheet_link_tag('letter.css')

  %body
    = yield

Does not work

 module AssetHelper
  def inline_stylesheet_link_tag(asset_name)
    content_tag(:style, Rails.application.assets.load_path.find(asset_name).content.html_safe )
  end
end

Encoding::CompatibilityError at /letter.html
incompatible character encodings: ASCII-8BIT and UTF-8

Works

 module AssetHelper
  def inline_stylesheet_link_tag(asset_name)
    content_tag(:style, File.read(Rails.application.assets.load_path.find(asset_name).path, encoding: 'UTF-8').html_safe )
  end
end

Do you think it make sense to e.g. add an encoding option to Propshaft::Asset#content? I can create a PR for this. Otherwise just close this issue.

[brenogazzola]

Could you test the branch in this closed PR of mine? I thought this wouldn't be needed: #166.

[denzelem]

Could you test the branch in this closed PR of mine? I thought this wouldn't be needed: #166.

For my use case your PR is working. But from my technical understanding (see this blog post) it is wrong to assume all assets are encoded with UTF-8.

File.binread(path).force_encoding("UTF-8") seems to me the same as File.read(path, encoding: 'UTF-8'). Both assume an asset encoded with UTF-8, but might return a wrong result / raise an exception for other encodings.

What do you think of adjusting your PR to something like:

def content(encoding: 'ASCII-8BIT')
  File.read(path, encoding: encoding)
end

Only the person that uses Propshaft::Asset#content can know which encoding the asset has. And falling back to ASCII-8BIT' is the best what one could do imho.

Wdyt?

[denzelem]

@brenogazzola With the suggestion above the encoding options has to be public available in Propshaft::Resolver::Dynamic#read and Propshaft::Resolver::Static#read.

[brenogazzola]

You are right. Would you like to open a PR for this?

[denzelem]

@brenogazzola I've opened a PR and close this issue for now. Reviews are welcome! Thank you!