CSS Image URL rewriting not working for CSS hosted external servers.

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Create a CSS, foo.css, which has a background image url
.foo {
  background-image: url( "/images/some.png" );
}
2. Host the CSS in a different web-server
3. In localhost servlet-context, create a group out of that CSS in wro.xml 
<group name="gcss">
 <css>http://staticserver.com/css/foo.css</css>
</group>

What is the expected output? What do you see instead?

The group gcss.css accessed via http://localhost:8080/wro/wro/gcss.css will 
have the image url rewritten wrongly as below -

.foo {
  background-image: url( "http://staticserver.com/css/images/some.png" );
}

it should have been either - http://staticserver.com/images/some.png or 
/images/some.png as it was.

What version of the product are you using? On what operating system?

latest 1.4.6

Additional information below:-

It is similar to -  bug Issue 322 or Issue 91. The fixes for them  seems to be 
specific only to the resources whose uri is a valid ServletContext 
resource.(ie. if the css belongs to the servlet context)

The bug still remains for conditions where you have the css hosted in an 
external server (ie. when we use the UrlUriLocator)

The fix probably should have been made in the 
CssUrlRewritingProcessor.computeNewImageLocation(), the image url starts with 
the '/' it should have left alone, shouldn't it ?

Original issue reported on code.google.com by doncarea...@gmail.com on 13 Jun 2012 at 7:05

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 13 Jun 2012 at 7:10

• Changed state: Accepted

[GoogleCodeExporter]

Hi Alex,
Do you have any update on this issue?
Thanks.

Original comment by doncarea...@gmail.com on 18 Jun 2012 at 7:07

[GoogleCodeExporter]

Could you test it against the 1.4.x branch? Even better, could you create a 
unit test which proves the problem and create a pull request? 
Currently I'm in the middle of another task and it will take a while until I 
can work on this one.

Original comment by alex.obj...@gmail.com on 18 Jun 2012 at 7:17

[GoogleCodeExporter]

I think this problem is questionable. It is rather a problem to use 
servletContext relative backgrounds in css hosted on a different server. 

How would you guarantee that the root context is mapped to "/" on that host?
I'm not sure that changing the implementation would be correct.
What is your opinion?

Original comment by alex.obj...@gmail.com on 18 Jun 2012 at 8:51

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 18 Jun 2012 at 8:52

• Changed state: New

[GoogleCodeExporter]

Agreed partially, but as explained above the existing url rewrite pre-processor 
is also doing a wrong assumption. In this case, when we access the group via 
http://localhost:8080/wro/wro/gcss.css, the url is getting rewritten assuming 
the context-root as 'css' as shown below -

.foo {
  background-image: url( "http://staticserver.com/css/images/some.png" );
}

The original resource is from http://staticserver.com/css/foo.css and it had 
this - background-image: url( "/images/some.png" );

In my case, the expected output should have been - 
http://staticserver.com/images/some.png

The assumption that the root context is mapped to '/' can be made by assuring 
it doesn't end up in java.io.FileNotFoundException: 
http://staticserver.com/images/some.png, when you validate getting an input 
stream from.

Original comment by doncarea...@gmail.com on 20 Jun 2012 at 1:01

[GoogleCodeExporter]

Ok, I think it could be an acceptable approach to use '/' as root context by 
default. Otherwise, it would be hard to find the correct mapping. 

Original comment by alex.obj...@gmail.com on 20 Jun 2012 at 2:00

• Changed state: Accepted
• Added labels: Milestone-Release-1.4.7

[GoogleCodeExporter]

Fixed in branch 1.4.x (on github)

Original comment by alex.obj...@gmail.com on 20 Jun 2012 at 5:53

• Changed state: Fixed

[GoogleCodeExporter]

Thank you for accommodating this change and fixing it so fast. When are you 
planning the next stable release ?

Original comment by doncarea...@gmail.com on 20 Jun 2012 at 6:42

[GoogleCodeExporter]

There are several issues left to do for 1.4.7 milestone. The optimistic 
estimation is 1 week. Is this ok for you?

Original comment by alex.obj...@gmail.com on 20 Jun 2012 at 6:51

[GoogleCodeExporter]

Yes it is OK. Thanks again. 

Original comment by doncarea...@gmail.com on 20 Jun 2012 at 6:56

[GoogleCodeExporter]

Hi Alex, This has not been fixed. Seems like the url is getting rewritten 
without the scheme/protocol which is causing the resultant url to be something 
like 

.foo {
  background-image: url( "staticserver.com/css/images/some.png" );
}
 which is wrong and it should be

.foo {
  background-image: url( "http://staticserver.com/css/images/some.png" );
}

What we can do here is a small modification to the following method like below

  private String computeCssUriForExternalServer(final String cssUri) {
    String externalServerCssUri = cssUri;
    try {
      URL url = new URL(cssUri);
       //the uri should end mandatory with /
      externalServerCssUri = url.getProtocol()+ "://"+ url.getHost() + "/";
      LOG.debug("using {} host as cssUri", externalServerCssUri);
    } catch(MalformedURLException e) {
      //should never happen
    }
    return externalServerCssUri;
  }

Original comment by doncarea...@gmail.com on 22 Jun 2012 at 9:38

[GoogleCodeExporter]

Hmmm.. that's strange. I have unit tests for this test-case. Probably there is 
something wrong with them. I'll take a look on them to see what is wrong. 
Thanks for checking.

Original comment by alex.obj...@gmail.com on 23 Jun 2012 at 7:29

[GoogleCodeExporter]

You are right. The current behavior is not correct. I'll update unit tests and 
will fix the issue.

Original comment by alex.obj...@gmail.com on 23 Jun 2012 at 10:00

• Changed state: Started

[GoogleCodeExporter]

Fixed in 1.4.x. Please, try again and let me know if you find any other issues.

Original comment by alex.obj...@gmail.com on 23 Jun 2012 at 10:31

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 23 Jun 2012 at 10:36

• Changed state: Fixed

[GoogleCodeExporter]

It is working. Thanks.

Original comment by doncarea...@gmail.com on 26 Jun 2012 at 6:47