host path prepended to urls generated with interpolated strings #294

Closed
soychicka opened this Issue Jun 26, 2011 · 20 comments

Comments

Projects
None yet
@soychicka

In the latest distribution (1.1.3), if you use this paraphrased example from the lesscss.org home page

@base-url: "http://assets.fnord.com";
.thing{
background-image: url("@{base-url}/images/bg.png");
}

the compiled result will be

.thing{
background-image: url("http://localhost:3000/path/to/stylesheets/http://assets.fnord.com/images/bg.png");
}

I don't know if this is a regression, as I've never had chance to use the string interpolation functionality before.

It looks like the string that is tested for the presence of a prefix around line 2270 isn't evaluated first, so the conditional isn't testing against the full url.

I see two possible solutions:

  1. The easy way - modify the regex used to classify urls as relative/absolute, excluding any string that starts with '@{'
    I pulled it off locally - https://gist.github.com/1047106. Still, that's just a temporary fix until I can find out how to do it
  2. The right way - use the regex in its current state to test the interpolated value of all urls. I'm not familiar enough with the code base, so I don't know the proper way to eval the string... if anyone could point me in the right direction, I'd be happy to fork and resolve this issue.
@jasonkeene

This comment has been minimized.

Show comment
Hide comment
@jasonkeene

jasonkeene Jul 26, 2011

I ran into the same issue.

@media_url: "/media";

// in some other scope

background: url("@{media_url}/images/sidebar_bg.png");

produced:

background: url("http://localhost:8000/media/less//media/images/sidebar_bg.png");

It basically prepended the path to the .less file to make the URL relative when it was intended to be absolute. This was only the case when I ran the compiler in browser and not from the command line.

Temporary Fix:

@media_url: "/media";

// in some other scope

background: ~"url(@{media_url}/images/sidebar_bg.png)";

I ran into the same issue.

@media_url: "/media";

// in some other scope

background: url("@{media_url}/images/sidebar_bg.png");

produced:

background: url("http://localhost:8000/media/less//media/images/sidebar_bg.png");

It basically prepended the path to the .less file to make the URL relative when it was intended to be absolute. This was only the case when I ran the compiler in browser and not from the command line.

Temporary Fix:

@media_url: "/media";

// in some other scope

background: ~"url(@{media_url}/images/sidebar_bg.png)";
@apneadiving

This comment has been minimized.

Show comment
Hide comment
@apneadiving

apneadiving Sep 2, 2011

@jasonkeene: thanks a bunch for your temporary fix!

@jasonkeene: thanks a bunch for your temporary fix!

@tarnfeld

This comment has been minimized.

Show comment
Hide comment
@tarnfeld

tarnfeld Sep 11, 2011

Any idea if this will be fixed any time soon? :)

Any idea if this will be fixed any time soon? :)

@ochagnon

This comment has been minimized.

Show comment
Hide comment
@ochagnon

ochagnon Oct 28, 2011

Bump!

This break string interpolation as documented

Bump!

This break string interpolation as documented

@zachstronaut

This comment has been minimized.

Show comment
Hide comment
@zachstronaut

zachstronaut Jan 9, 2012

+1

This bug still exists in 1.2.0 when run in the browser.

+1

This bug still exists in 1.2.0 when run in the browser.

@lggarrison

This comment has been minimized.

Show comment
Hide comment
@lggarrison

lggarrison Feb 25, 2012

+1

This bug still exists in 1.2.1 when run in the browser.

+1

This bug still exists in 1.2.1 when run in the browser.

@danoc

This comment has been minimized.

Show comment
Hide comment
@danoc

danoc Apr 4, 2012

I'm having the issue in 1.3 as well. Thanks for the temporary fix, @jasonkeene!

danoc commented Apr 4, 2012

I'm having the issue in 1.3 as well. Thanks for the temporary fix, @jasonkeene!

@bclermont

This comment has been minimized.

Show comment
Hide comment

Me too!

@chanon

This comment has been minimized.

Show comment
Hide comment
@chanon

chanon Apr 9, 2012

This bug still exists in 1.3 when run in the browser.

chanon commented Apr 9, 2012

This bug still exists in 1.3 when run in the browser.

@chanon

This comment has been minimized.

Show comment
Hide comment
@chanon

chanon Apr 9, 2012

A quick fix in less-1.3.0.js

// I commented out some lines marked with >> to fix the issue

tree.URL = function (val, paths) {
    if (val.data) {
        this.attrs = val;
    } else {
        /* >> just comment these lines out as we don't use any relative urls
        >> // Add the base path if the URL is relative and we are in the browser
        >> if (typeof(window) !== 'undefined' && !/^(?:https?:\/\/|file:\/\/|data:|\/)/.test(val.value) && paths.length > 0) {
        >>    val.value = paths[0] + (val.value.charAt(0) === '/' ? val.value.slice(1) : val.value);
        >> }
        >> */
        this.value = val;
        this.paths = paths;
    }
};

chanon commented Apr 9, 2012

A quick fix in less-1.3.0.js

// I commented out some lines marked with >> to fix the issue

tree.URL = function (val, paths) {
    if (val.data) {
        this.attrs = val;
    } else {
        /* >> just comment these lines out as we don't use any relative urls
        >> // Add the base path if the URL is relative and we are in the browser
        >> if (typeof(window) !== 'undefined' && !/^(?:https?:\/\/|file:\/\/|data:|\/)/.test(val.value) && paths.length > 0) {
        >>    val.value = paths[0] + (val.value.charAt(0) === '/' ? val.value.slice(1) : val.value);
        >> }
        >> */
        this.value = val;
        this.paths = paths;
    }
};
@johanbove

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Apr 12, 2012

@jasonkeene Just wanted to add my appreciation for providing a temporary solution to this issue: Thanks!

@jasonkeene Just wanted to add my appreciation for providing a temporary solution to this issue: Thanks!

@WraithKenny

This comment has been minimized.

Show comment
Hide comment
@WraithKenny

WraithKenny May 15, 2012

If this is simply documented better, I'd consider it a feature ;-)

If this is simply documented better, I'd consider it a feature ;-)

@WraithKenny

This comment has been minimized.

Show comment
Hide comment
@WraithKenny

WraithKenny May 15, 2012

It's worth pointing out that if you call the less parser on a source this isn't a problem at all. It's only a problem with the "automatic" browser version:
`

<script src="less.js" type="text/javascript"></script>`

It's worth pointing out that if you call the less parser on a source this isn't a problem at all. It's only a problem with the "automatic" browser version:
`

<script src="less.js" type="text/javascript"></script>`
@WraithKenny

This comment has been minimized.

Show comment
Hide comment

@neonstalwart neonstalwart referenced this issue May 15, 2012

Closed

url() bug #807

@Umap

This comment has been minimized.

Show comment
Hide comment
@Umap

Umap Jun 5, 2012

Thanks soychicka. But is there any solution to fix that as permanently?

Umap commented Jun 5, 2012

Thanks soychicka. But is there any solution to fix that as permanently?

@MokoJumbie

This comment has been minimized.

Show comment
Hide comment
@MokoJumbie

MokoJumbie Jul 4, 2012

Unfortunately, the workaround doesn't seem to work with less.js 1.3.0 in a browser for @import rules. The host path is still prepended and the variables are not interpolated.

Probably related to: cloudhead#410

Unfortunately, the workaround doesn't seem to work with less.js 1.3.0 in a browser for @import rules. The host path is still prepended and the variables are not interpolated.

Probably related to: cloudhead#410

@ugommirikwe

This comment has been minimized.

Show comment
Hide comment
@ugommirikwe

ugommirikwe Aug 9, 2012

@jasonkeene u just saved some hairs on my head :-) thanks a zillion!

@jasonkeene u just saved some hairs on my head :-) thanks a zillion!

@dash-

This comment has been minimized.

Show comment
Hide comment
@dash-

dash- Sep 1, 2012

I don't want to join development or put out a patch, but I do have an explanation and a fix (version 1.3.0).

Permanent fix (Non-Minified Source) Line 3013 (function tree.URL):

if (typeof(window) !== 'undefined' && !/^(?:https?://|file://|data:|/|@)/.test(val.value) && paths.length > 0) {

Explanation:

@base-url: "http://assets.fnord.com";
.thing{ background-image: url("@{base-url}/images/bg.png"); }

During parsing, url("@{base-url}/images/bg.png") matches as a url and currently gets compared to the regular expression: /^(?:https?://|file://|data:|/)/

"@{base-url}/..." doesn't match (since the variable hasn't been interpolated yet).

So its considered a relative path. So it gets turned into: url("http://localhost:3000/path/to/stylesheets/@{base-url}/images/bg.png").

Then variable interpolation takes place, turning it into: url("http://localhost:3000/path/to/stylesheets/http://assets.fnord.com/images/bg.png");

The fix is to include @ as an initial dis-qualifier of relative paths (as I've done in my fix above). I think it will still be interpolated and then re-ran through the tree.URL transform. If the interpolated value is relative, I think it will then be prefixed with the absolute url; if it is absolute, it will be left alone.

I hope someone else takes this minor fix and puts the energy in to merge it into the code base. (Cuz I'm too busy to do it).

dash- commented Sep 1, 2012

I don't want to join development or put out a patch, but I do have an explanation and a fix (version 1.3.0).

Permanent fix (Non-Minified Source) Line 3013 (function tree.URL):

if (typeof(window) !== 'undefined' && !/^(?:https?://|file://|data:|/|@)/.test(val.value) && paths.length > 0) {

Explanation:

@base-url: "http://assets.fnord.com";
.thing{ background-image: url("@{base-url}/images/bg.png"); }

During parsing, url("@{base-url}/images/bg.png") matches as a url and currently gets compared to the regular expression: /^(?:https?://|file://|data:|/)/

"@{base-url}/..." doesn't match (since the variable hasn't been interpolated yet).

So its considered a relative path. So it gets turned into: url("http://localhost:3000/path/to/stylesheets/@{base-url}/images/bg.png").

Then variable interpolation takes place, turning it into: url("http://localhost:3000/path/to/stylesheets/http://assets.fnord.com/images/bg.png");

The fix is to include @ as an initial dis-qualifier of relative paths (as I've done in my fix above). I think it will still be interpolated and then re-ran through the tree.URL transform. If the interpolated value is relative, I think it will then be prefixed with the absolute url; if it is absolute, it will be left alone.

I hope someone else takes this minor fix and puts the energy in to merge it into the code base. (Cuz I'm too busy to do it).

@ayyash

This comment has been minimized.

Show comment
Hide comment
@ayyash

ayyash May 1, 2013

How is this closed 8 months ago? I still get localhost refernces after toCSS, but why does LESS do that in the first place? I currently run a script to remove them after compilation, but recently I ran into trouble where I cannot guess what the url is so I cannot remove it... how can I prevent the compiler from outputting absolute urls like that?

ayyash commented May 1, 2013

How is this closed 8 months ago? I still get localhost refernces after toCSS, but why does LESS do that in the first place? I currently run a script to remove them after compilation, but recently I ran into trouble where I cannot guess what the url is so I cannot remove it... how can I prevent the compiler from outputting absolute urls like that?

@lukeapage

This comment has been minimized.

Show comment
Hide comment
@lukeapage

lukeapage May 2, 2013

Member

@ayyash http://lesscss.org/#usage look at rootpath and relativeUrls options. post a new issue if something is going wrong

Member

lukeapage commented May 2, 2013

@ayyash http://lesscss.org/#usage look at rootpath and relativeUrls options. post a new issue if something is going wrong

strk pushed a commit to CartoDB/carto that referenced this issue Oct 28, 2013

strk pushed a commit to CartoDB/carto that referenced this issue Oct 28, 2013

Merge pull request #294 from kapouer/patch-1
layer srs is inherited from map srs

lukeapage added a commit that referenced this issue Jan 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment