Very simply put, a strcmp was used to compare two host names instead of strcasecmp. Patch is as follows (line 189, for reference): The extra comment is irrelavent to this issue, but is related (and already discussed vaguely in another bug) Oh, and to make this a more proper bug report, this is in the trunk CVS from earlier today. RCS file: /cvsroot/mozilla/extensions/cookie/nsImages.cpp,v retrieving revision 1.18 diff -u -r1.18 nsImages.cpp --- nsImages.cpp 31 May 2002 18:40:32 -0000 1.18 +++ nsImages.cpp 24 Jun 2002 23:09:25 -0000 @@ -180,13 +180,13 @@ if (*tailFirstHostname == '.') { dotcount++; } - if (dotcount == 2) { + if (dotcount == 2) { /* XXX: so does that make a.co.uk == b.co.uk? */ tailFirstHostname++; break; } tailFirstHostname--; } - if (PL_strcmp(tailFirstHostname, tailHostname)) { + if (PL_strcasecmp(tailFirstHostname, tailHostname)) { *permission = PR_FALSE; return NS_OK; }

> /* XXX: so does that make a.co.uk == b.co.uk? */ Yes, it treats them as being in the same domain. That's an old problem -- see bug 8743 in detail. Please remove that comment from your patch since it is not related. With that change, r=morse But please tell me why this patch is needed. The user doesn't type in the hostname -- it comes from the site. So are you saying that at one time the site reported its name as xxx.com when you decided to block the image, and at some future visit when the blocking test is being made, the site reported its name as XXX.com?

My reason is that regardless of who or what produces them, domain names are always case-insensitive. I was thinking of a purely imaginary anti-image-blocking server that randomly rotated the domain case on image elements it served. When I made the patch, I knew the problem would be rare and unlikely, but I chose to fix what I saw as (insignificantly) incorrect behavior instead of leaving it as-is. Also worth noting is that bug 33576 also appears to cover (now a lot more than) the problem with using 2 levels of the fully qualified host name for comparison

Comment on attachment 89582 [details] [diff] [review] More isolated patch r=morse

Steve Morse, please confirm bugs when you see or review the patches.

I gave it an r= because the patch was completely harmless. But I can't confirm that this is really a problem or that the patch is really needed.

Mass reassigning of Image manager bugs to mstoltz@netscape.com, and futuring. Most of these bugs are enhancement requests or are otherwise low priority at this time.

I tried a page with a <img src="http://TEST.LOCALDOMAIN/warning.gif"> on it, while blocking test.localdomain. The image was blocked anyway. Is there still a need for this patch? (note that the code has been drasticly changed, so the patch won't apply anymore, but the general structure still applies)

This should be the job of the URI parser now - the permissions backend uses URI's in its API now (mostly), rather than strings. because of this, we can guarantee domain names have been parsed by the URI's (scheme-dependent) constructor. For the http case, this performs a lowercase canonicalization: http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsStandardURL.cpp#535 For other schemes, this may not be the case, but I think that's up to the URI parser to decide, not the permissions backend. -> worksforme since the permissions rewrite "fixed" this.