CSS filter ignores external CSS links with rel='Stylesheet' instead of rel='stylesheet'

[GoogleCodeExporter]

When an HTML file references an external CSS resource using <link 
rev="Stylesheet"> (note the caps "S") the CSS is not minified, image links not 
rewritten, not cache extended, etc.  If the "rev" attr. value is changed to 
"stylesheet" (lowercase) then the rewriting works as expected.

This is incorrect behavior, as the HTML spec declares that "link types are 
always ASCII case-insensitive, and must be compared as such.":

http://dev.w3.org/html5/spec/Overview.html#linkTypes

-------------------

What version of the product are you using (please check X-Mod-Pagespeed
header)?

0.10.19.3-1175

On what operating system?

n/a (RHEL 6)

Which version of Apache?
2.2.15

Original issue reported on code.google.com by inetdev...@gmail.com on 1 Dec 2011 at 8:48

[GoogleCodeExporter]

I can confirm this broken in CSS filter, but the cache extender should work 
fine.

Original comment by morlov...@google.com on 1 Dec 2011 at 9:00

[GoogleCodeExporter]

Looks like this happens around line 475 of net/instaweb/rewriter/css_filter.cc 
(revision 1212):

    StringPiece relation(element->AttributeValue(HtmlName::kRel));
    if (relation == kStylesheet) {
        ...

I'm no C/C++ whiz (more of a Java guy) but it looks like it's using the 
StringPiece "==" op from the Chromium project, which appears to be a straight 
byte-for-byte comparison.  This should be changed to a case-insensitive 
comparison.  I'll try to play with a patch, but again it's not my forte...

Original comment by inetdev...@gmail.com on 1 Dec 2011 at 9:13

[GoogleCodeExporter]

That is exactly correct, and the fix is basically to change that to 
if (StringCaseEqual(relation, kStylesheet)) 

... However the cache extender filters has separate checks of its own, which is 
already case insensitive.

(Working on patch right now).

Original comment by morlov...@google.com on 1 Dec 2011 at 9:16

[GoogleCodeExporter]

It does actually try to cache-extend the CSS itself.  The problem is that it 
doesn't rewrite the image-refs in the CSS.  Because all my statics are being 
served from another domain, I end up with a cache-extended CSS like this:

   static.mydomain.com/myStyles_pagespeed_ce.FiLeHaSh.css

... with non-rewritten img refs like this:

    url('img/whatever.gif')

The browser ultimately resolves that to:

   http://static.mydomain.com/img/whatever.gif

The "static" subdomain doesn't know how to serve that content except as proxied 
via a mod_pagespeed origin request.  Mod_pagespeed doesn't seem to know about 
it since it didn't rewrite the CSS, so all the images referenced by that CSS 
end up failing as 404s.

Original comment by inetdev...@gmail.com on 1 Dec 2011 at 9:21

[GoogleCodeExporter]

Weird, I thought we would try to absolutify links in the cache extender in such 
a case. What configuration directive are you using to move the CSS to the 
subdomain?

Original comment by morlov...@google.com on 1 Dec 2011 at 9:38

[GoogleCodeExporter]

Primary conf has:
   ModPagespeedMapRewriteDomain http://static.mydomain.com http://www.mydomain.com
   ModPagespeedRewriteLevel PassThrough
   ModPagespeedEnableFilters rewrite_css,rewrite_javascript,extend_cache

"static" subdomain has:
   ModPagespeedMapOriginDomain http://www.mydomain.com http://static.mydomain.com
   ModPagespeedRewriteLevel PassThrough
   ModPagespeedEnableFilters rewrite_css,rewrite_javascript,extend_cache

Names changed (to protect the innocent?), and it's actually a little more 
complex than that 'cause the origin domain is on a non-standard port behind a 
load-balancer, and then passes through to Tomcat.  But that's the basics.

Original comment by inetdev...@gmail.com on 1 Dec 2011 at 9:51

[GoogleCodeExporter]

Hi inetdevboy, I think this is the problem your getting with www and static:

From http://code.google.com/speed/page-speed/docs/domains.html#mapping_rewrite:

Note: It is the responsbility of the site administrator to ensure that Apache 
httpd is installed with mod_pagespeed on the domain_to_write_into_html. This 
might be a separate server, or there may be a single server with multiple 
domains mapped into it. The files must be accessible via the same path on the 
destination server as was specified in the HTML file. No other files should be 
stored on the domain_to_write_into_html -- it should be functionally equivalent 
to domain_specified_in_html.


Specifically, you asked mod_pagespeed to move all files from www to static, so 
you need to make sure that all these files can be reached on static. The 
declarations you added make sure that all the .pagespeed. rewritten resources 
will be accessible, but not the normal resources.

If static and www are served from the same machine, you could just make static 
an alias for www and that should solve the problem. If you have a more 
complicated setup, the solution will be more complicated as well, I'm afraid.

Original comment by sligocki@google.com on 1 Dec 2011 at 9:58

[GoogleCodeExporter]

I suppose that makes sense.  Obviously I have the module on both servers, and I 
guess I'd envisioned that the only things that would've been rewritten to the 
static/CDN endpoint would've been touched by mod_pagespeed, and therefore any 
internal resources would be similarly rewritten.  I thought the 
"ModPagespeedMapOriginDomain" directive on the "static" server would've been 
enough to point things in the right direction, but I guess that only applies if 
the mod_pagespeed thinks it "owns" those resources.  Your [upcoming] patch 
resolves this particular use case, but I can imagine other instances (parse 
errors, whatever) in which the parent CSS remains untouched and tries to 
reference relative resources that wouldn't be there.

I suppose it would be best to mod_proxy the "static" domain back to the origin 
domain so that it covers any cases that slip by mod_pagespeed.  

Original comment by inetdev...@gmail.com on 1 Dec 2011 at 10:09

[GoogleCodeExporter]

Yeah, mod_proxy ought to do the trick. Let me know if this works out for you 
since this issue will probably come up again with someone else.

Original comment by sligocki@google.com on 2 Dec 2011 at 4:29

• Changed state: Accepted

[GoogleCodeExporter]

I added the change mentioned in comment 3, rebuilt from source, and that fixed 
this particular use case, as expected.  Also, adding mod_proxy works fine as a 
fall-back for serving those resources from the "static" domain when they're 
shunned (for whatever reason) by mod_pagespeed.

In our final deployment we'll most likely be using the 
all-in-one-server-with-alias approach described in comment 7 so as not to have 
unnecessarily-redundant caches and origin checks, but I was trying to get it 
working in separate instances in case we decided to distribute the static 
servers to our other data centers.

Thank you both so much for your help, and the crazy-fast response time.  I 
tried to narrow it down as far as possible before submitting...

Original comment by inetdev...@gmail.com on 2 Dec 2011 at 4:08

[GoogleCodeExporter]

Committed fix in r1214; also I would encourage you to consider 
ModPagespeedLoadFromFile for static resources.

Original comment by morlov...@google.com on 7 Dec 2011 at 2:12

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by matterb...@google.com on 26 Jan 2012 at 3:02

• Added labels: release-note

[GoogleCodeExporter]

Original comment by matterb...@google.com on 7 Feb 2012 at 1:39

• Changed title: CSS filter ignores external CSS links with rel='Stylesheet' instead of rel='stylesheet'

[GoogleCodeExporter]

Original comment by jmara...@google.com on 23 May 2012 at 2:47

• Added labels: Milestone-v21