encoding error with ruby 1.9

documentcloud / jammit

Industrial Strength Asset Packaging for Rails

http://documentcloud.github.com/jammit/

MIT License

1.16k stars 197 forks source link

encoding error with ruby 1.9 #58

Closed mackuba closed 14 years ago

mackuba commented 14 years ago

When using Ruby 1.9, if a file has non-ASCII characters in it (which can happen in vendor CSS/JS in comments, e.g. in author's name), an exception is thrown:

compressor.rb:138:in `gsub': invalid byte sequence in US-ASCII (ArgumentError)

Adding :external_encoding => 'BINARY' to File.read seems to help, although I'm not 100% sure if this is the right solution...

jashkenas commented 14 years ago

I've added a unicode comment to the test case, here:

http://github.com/documentcloud/jammit/commit/b15db6d773c5e9fa1fdfc2c5f709da9bb90b61f3

But the tests still pass just fine for me under both Ruby 1.8.7 and 1.9.2. Can you contribute a test case that demonstrates the problem?

mackuba commented 14 years ago

I'm getting 12 errors in your test suite on Ruby 1.9.2 which disappear when I remove the two unicode lines... The reason you don't get them is because you have different system encoding settings. Encoding.default_external returns #<Encoding:US-ASCII> on my machine, I suppose on yours it's #<Encoding:UTF-8>. If I add a line Encoding.default_external = 'utf-8' to the beginning of your Rakefile, the tests pass.

jashkenas commented 14 years ago

Here's a patch which forces CSS being read-in to be treated as binary strings, for the purposes of URL rewriting. Give it a spin and let me know if it works for you:

http://github.com/documentcloud/jammit/commit/0634505e1e7bb1fd9e8d7e2cfa8ac0601e671b08

Thanks for reporting the issue.

mackuba commented 14 years ago

I've checked the latest version and it looks like it worked - the tests are passing (except the other 5, but this is unrelated, it's probably because of Rails 3), and the stylesheet from project is parsed correctly - thanks :)

Two more comments about your tests:

I'm getting an error in test/unit/test_jammit_helpers that Jammit::Helper is not found, until I add a require 'jammit/helper' line
the test loads the globally installed version of Jammit instead of the one in lib/, to fix that I've added a line to task :test definition in Rakefile that unshifts lib/ to the load path (same way as it adds the test/ directory).

larskuhnt commented 14 years ago

Hi, I think you should force the binary encoding for compressed assets as well. I created a patch to fix this issue:

http://github.com/larskuhnt/jammit/commit/42e5720fecfef328068adbca55a48b9ac264f05c

Regards and thanks Lars