propshaft icon indicating copy to clipboard operation
propshaft copied to clipboard

Add encoding options to Resolver::Static#read and Resolver::Dynamic#read

Open denzelem opened this issue 1 year ago • 3 comments

PR for https://github.com/rails/propshaft/issues/176.

denzelem avatar Jan 23 '24 14:01 denzelem

Don't see how this works when the asset is a binary file? If we're assuming ASCII-8BIT, what happens when you try to read a PNG?

dhh avatar May 15 '24 23:05 dhh

Don't see how this works when the asset is a binary file? If we're assuming ASCII-8BIT, what happens when you try to read a PNG?

@dhh Do you think that File.binread(path) is different to File.read(path, encoding: 'ASCII-8BIT')? My assumption was, that binread (rb_ascii8bit_encoding C-Method) reads the bytes with the encoding ASCII-8BIT.

So reading an PNG image should be no breaking change with this PR.

require 'open-uri'
require 'tmpdir'

image_url = 'https://picsum.photos/id/237/200/300'

Dir.mktmpdir do |dir|
  image_path = File.join(dir, 'example.jpg')

  URI.open(image_url) do |image|
    File.open(image_path, 'wb') do |file|
      file.write(image.read)
    end
  end

  puts 'Encoding binread ' + File.binread(image_path).encoding.to_s
  puts 'Encoding read with ASCII-8BIT ' + File.read(image_path, encoding: 'ASCII-8BIT').encoding.to_s
end

exit
Encoding binread ASCII-8BIT
Encoding read with ASCII-8BIT ASCII-8BIT

denzelem avatar May 22 '24 09:05 denzelem

@dhh @brenogazzola I don't see any blockers for merging this - what do you think?

theodorton avatar Jun 10 '24 09:06 theodorton