ndbroadbent / turbo-sprockets-rails3

Speeds up your Rails 3 assets:precompile by only recompiling changed files, and only compiling once to generate all assets
MIT License
975 stars 78 forks source link

Fix for issue #35 #64

Closed steverandy closed 11 years ago

steverandy commented 11 years ago

I have similar problem that causes by one of the javascript file. This fixes the invalid byte sequence in UTF-8 error. It passes the test suite.

ndbroadbent commented 11 years ago

Do you think it's safe to just remove unknown characters? Couldn't this cause syntax errors in JavaScript, or at least mess up some strings?

ndbroadbent commented 11 years ago

Also, 1.8.7 fails with:

undefined method `encode' for "function f1(){alert()}":String

Could you please add a check for RUBY_VERSION.to_f >= 1.9? Thanks!

steverandy commented 11 years ago

@ndbroadbent, I just tested my file. It removes about 57 chars. So I think this way isn't safe. Is there a way to skip non digest during precompile process?

ndbroadbent commented 11 years ago

Can I ask which characters are removed? Are they from a different language?

On Sun, Apr 21, 2013 at 8:43 PM, Steve Randy Tantra < notifications@github.com> wrote:

@ndbroadbent https://github.com/ndbroadbent, I just tested my file. It removes about 57 chars. So I think this way isn't safe. Is there a way to skip non digest during precompile process?

— Reply to this email directly or view it on GitHubhttps://github.com/ndbroadbent/turbo-sprockets-rails3/pull/64#issuecomment-16717884 .

steverandy commented 11 years ago

One that I found is this. https://github.com/epeli/underscore.string/blob/master/lib/underscore.string.js#L513

But the lib that actually causes this error in the first place was this. https://github.com/marijnh/CodeMirror

quezo commented 11 years ago

Just wanted to note that I had the same issue with CodeMirror and I installed this fork of Turbo Sprockets and it all worked. Thanks for the fix!

steverandy commented 11 years ago

@quezo, it works but I think the non-digested file is no longer valid, because the fix removes some part of the code (the invalid encoding).

@ndbroadbent, why not run another precompile for non-digested assets? It will only be slow for the first time right?

jakswa commented 11 years ago

at least in my case, this fix would replace (what looks like) most of the character symbols inside http://symbolset.com/ and the helper javascript for those symbols...

jakswa commented 11 years ago

After trying many different things (and even checking out the buildpack and sprockets, to try changes), I decided to try this change on the above branch and IT WORKED for me (or seems to, more testing to come). Can anyone else try that change out? I've no idea why it works, to be honest. It doesn't end up replacing characters, so I propose it as a fix, if someone else can verify!

If for some reason my link goes bad, the change I'm talking about is

-    asset_body = asset_body.encode('UTF-8', 'binary', :invalid => :replace, :undef => :replace, :replace => '')
+    asset_body = asset_body.encode('UTF-8', 'UTF-8')
jakswa commented 11 years ago

went to deploy the above change today to our staging environment and got the same failure... tests back on my dev instance show the same, now. I'll see if I can't narrow this down some more and report back.

stereoscott commented 11 years ago

I just got hit with this, too when trying to use codemirror.js. I tried to resave the file with UTF-8 encoding, force utf by renaming the file to codemirror.js.erb and adding <%# encoding: utf-8 %> at the top as a hack... nothing works. @jakswa did you have any luck?

jakswa commented 11 years ago

I didn't have any luck @stereoscott :crying_cat_face: it's a nasty bug because it's hard to debug heroku's deployment cache loading/unloading the assets. I think I did have it narrowed down to one of the time-saving tactics of turbo sprockets: The reuse of the digest asset for quickly generating the non-digest version of each asset. It seemed like the reading in of those just-written assets was what was causing the blow-up. But I wasn't able to devote much time to it... We ended up abandoning turbo sprockets for our deploys, for now.

stereoscott commented 11 years ago

The following string in a javascript file will trigger the error in turbo-sprockets. This is as specific as I could get it. There is some crazy voodoo going on somewhere!

"\udbff" && a.b

The error is:

➜  rake assets:precompile --trace
** Invoke assets:precompile (first_time)
** Execute assets:precompile
bin/rake assets:precompile:all RAILS_ENV=production RAILS_GROUPS=assets --trace
** Invoke assets:precompile:all (first_time)
** Invoke assets:cache:clean (first_time)
** Invoke assets:environment (first_time)
** Execute assets:environment
** Invoke environment (first_time)
** Execute environment
** Execute assets:cache:clean
** Execute assets:precompile:all
rake aborted!
invalid byte sequence in UTF-8
gems/turbo-sprockets-rails3-880904169535/lib/sprockets/static_non_digest_generator.rb:46:in `gsub!'

When I look in the generated javascript file, I see this:

"í¯¿"&&a.b;

This is with Ruby 2.0.

If I update line triggering the error with: asset_body = asset_body.encode('UTF-8', 'binary', :invalid => :replace, :undef => :replace, :replace => '')

That just erases the characters from the output, resulting in:

""&&a.b;
phillbaker commented 11 years ago

I believe it has to do with hexadecimal characters being interpreted by JS during compilation which outputs invalid UTF-8 characters.

Simply removing them may compile the JS, but it very likely won't be the same JS as the author intended or even operable.

One option may be a pre-compilation step which replaces \x[XXX] with a different sequence, the compiler ignores these symbols, which can then be replaced with the original after compilation.

jakswa commented 11 years ago

@phillbaker, for me this issue was happening only on heroku, so I was leaning towards something specific to the heroku environment. Asset compilation worked fine locally, which made me think that it was heroku's file system, for storing the deployment cache, maybe? Something like that.

Are everyone's assets compiling fine locally? edit: I'm on OSX for development, and wasn't able to test it on my linux box — could be some difference there as well.

stereoscott commented 11 years ago

My assets would be fine in dev without compiling them, but I couldn't run rake assets:precompile locally.

Digging in deeper I saw that even before turbo-sprockets gets it hands on the file, rails sprockets is rewriting the string in my javascript.

"\udbff" in my uncompiled asset becomes "í¯¿"; in the compiled file.

With rails 4, when I compiles assets, the resulting file has the proper string:

"\udbff"

After a lot of searching, I finally found the culprit. It's not even sprockets... it's the Uglifier. So, if you are using rails the solution for me was to put this in my production.rb:

# override default uglifier options so we don't mangle unicode
  config.assets.js_compressor = Uglifier.new(output: {ascii_only: true})

That means the compiled, compress javascript that turbo-sprockets eventually reads will contain the ascii versions of the unicode symbols, which can be read in via UTF-8 and gsub! 'd all day long.

_In the event you run into an error with Uglifier undefined, you can wrap the config.assets.js_compressor line with an if defined? Uglifier conditional... Depending on your gemfile Uglifier may only be initialized in in the assets group._

jakswa commented 11 years ago

awesome @stereoscott! At some point I'm going to test that out. Someone should paste that over in #35.

edit: it was rake assets:precompile that I was referring to, when I said it was working fine locally. that part is still a mystery to me...

steverandy commented 11 years ago

Great. Therefore this pull request can be closed.

elskwid commented 11 years ago

@jakswa, I had this issue crop up today while trying to deploy a Rails application. I had the same symptoms, error only when trying to deploy to staging. Local dev worked just fine.

My issue turned out to be with recent changes to modernizr-rails.

By locking my application to modernizr-rails 2.6.2.1 I was able to temporarily relieve the problem. I plan to investigate a more long-term solution in the near future. Thought this might help some other lost souls.