rstacruz / sinatra-assetpack

Package your assets transparently in Sinatra.
http://ricostacruz.com/sinatra-assetpack/
MIT License
542 stars 97 forks source link

CSS URL Re-writing Too Aggressive? #116

Closed agilbert201 closed 11 years ago

agilbert201 commented 11 years ago

Was trying to use a google font in css as:

@import url('//fonts.googleapis.com/css?family=Open+Sans:400italic,700italic,400,700'); 

Which was getting rewritten to local host, had to expand this out to the actual @font-face entry

@font-face {
  font-family: 'Open Sans';
  font-style: normal;
  font-weight: 400;
  src: local('Open Sans'), local('OpenSans'), url(http://themes.googleusercontent.com/static/fonts/opensans/v6/cJZKeOuBrn4kERxqtaUH3bO3LdcAZYWl9Si6vvxL-qU.woff) format('woff');
}

What I don't understand is why the latter didn't suffer the same fate. In general is the rewriting supposed to take into account you could have external asset references as well as local? Seems like it does, so perhaps the query parameter in the first url is throwing off a regexp?

j15e commented 11 years ago

This : https://github.com/rstacruz/sinatra-assetpack/blob/master/lib/sinatra/assetpack/css.rb#L7

agilbert201 commented 11 years ago

Looks like really here

https://github.com/rstacruz/sinatra-assetpack/blob/master/lib/sinatra/assetpack/options.rb#L214

The regexp in css.rb is just stripping quotes best I can tell. But in this case a call to local_file_for with an argument of '/css' is returning something. It is a local directory / path but not a real asset file.

Recreated with failing test case

  test "local file for (in nonexisting files)" do
    fn = App.assets.local_file_for '/css'
    assert fn.nil?
  end
j15e commented 11 years ago

Thanks, just found this out too and made a fix on master.

I updated the local_file_for method to check for File.file? instead of File.exist? as it matched folders (css folder in this case). I also rewrote a bit of the css preproc method to be cleaner.

This way, external url should never match unless the very same file exist in your app. 

agilbert201 commented 11 years ago

Most awesome. Thank you!

j15e commented 11 years ago

Released in 0.2.8