Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double inclusion of config.asset_host in CSS url() used with .erb asset path helpers #488

Closed
jrochkind opened this issue Dec 6, 2021 · 6 comments

Comments

@jrochkind
Copy link

jrochkind commented Dec 6, 2021

I believe this began with release 3.3.0, and is a regression caused by #476

Let's say we have an application.css.erb with this in it:

@font-face {
  src: url('<%= font_path('example.eot') %>');
}

And we have config.asset_host = '//example.org'. (The // protocol-agnostic asset_host is a common setting for CDNs, often recommended in docs).

With sprockets-rails 3.3.0+, if you ./bin/rails assets:precompile, the output at ./public/assets/application-[fingerprint].css looks like:

@font-face {
  src: url(//example.com/example.com/fonts/example.eot);
}

Note that //example.com/example.com is there twice. This reproduces with sprockets rails 3.3.0 and 3.4.1. (both with sprockets 4.0.2).

If you downgrade to sprockets 3.2.2, the output is as expected:

@font-face {
  src: url('//example.com/fonts/example.eot');
}

I believe this is a regression beginning with sprockets-rails 3.3.0.

I actually ran into this using font-awesome-rails on an app that deploys to heroku, with a CDN config.asset_host. font-awesome-rails uses the font_path helper in a .css.erb here. While that font-awesome-rails gem may be no longer supported, we ought not to get a regression in a minor sprockets-rails release. But also, I then isolated the reproduction as above, which also seems to be a general problem. It may be possible to isolate/generalize further, may also occur with sass-rails helpers not just the erb ones? May occur with other helpers, not just font_path?

@jrochkind
Copy link
Author

Verified problem still reproduces (problem is still there) on sprockets-rails main branch at 4e0f168

@jrochkind
Copy link
Author

(Sorry, I keep making comments and then deleting them about whether different forms of absolute/relative url in config.assets_host exhibit or not, because I keep getting confused about what i'm observing. I am also not currently sure if it exhibits on main branch, sorry!).

@jrochkind
Copy link
Author

OK, now I think maybe the problem IS fixed at main branch at 4e0f168.

Could someone else verify this though? I don't trust myself, I keep seeing different things!

If it is fixed on main, could we get a patch release soon, to spare others this trouble?

@dhh
Copy link
Member

dhh commented Dec 11, 2021

@dhh dhh closed this as completed Dec 11, 2021
@mattbrictson
Copy link

I ran into this as well, and can confirm it is fixed in 3.4.2.

FWIW, for some reason I had to bump my config.assets.version in order to see the fix.

@jrochkind
Copy link
Author

Confirmed fixed for me too in 3.4.2, with config.asset_host = "//example.com", and also config.asset_host = "example.com".

I think the second one (hostname-only) triggered bug previously as well, and is recommended in some Rails docs.

Both seem fixed to me in 3.4.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants