84776 - Image blocking on port numbers other then 80 doesnt work.

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Warner Young]

VoodooExtreme has a number of images with a source like the following:
http://adserver.ugo.com:80/ads/Uplister/greenday468.gif.  After I right-click
the image to block it, I can check the Tasks->Privacy->Image Manager->View Sites
dialog and see that "adserver.ugo.com:80" is in the list of sites being blocked.
 When I visit this page again, images from that server still show up.  On other
sites, image blocking seems to be working fine.

Build 2001060804, Win98, but I suspect this affects all platforms.

Comment 1

[Warner Young]

I've figured out the problem is definitely the inclusion of a port number.  If I
edit cookperm.txt and remove the ":80" from the entry in the file, image
blocking works properly for the ads I described.  Therefore, the real problem
seems to be that, when entries are added to cookperm.txt, port numbers should be
stripped first.

Comment 2

[Keyser Sose]

marking NEW and changing summary to make it more clear.

Comment 3

[Stuart Parmenter]

->morse

Comment 4

[Warner Young]

Actually, I think the summary should be "Image blocking with a port number
doesn't work."  I don't think it's specifically port 80 (or non-port 80) that's
a problem.

Comment 5

[Stephen P. Morse]

I believe that they may have changed the website because I am unable to find any 
images on that site that will give a :80 entry in the blocked-images list.  
However I can believe that these symptoms would occur and the fix for it is 
obvious.  Attaching patch.

cc'ing vishy & blake for r and sr.

Comment 6

[Stephen P. Morse]

Attachment: remove port number from host name

Comment 7

[Viswanath Ramachandran]

r=vishy

Comment 8

[Blake Ross]

It's hard to tell from the context or I'm just stupid, what is the purpose of
this change:

+        if (portColon)
+          *portColon = ':';

Comment 9

[Stephen P. Morse]

It undoes the temporary assigment preceding it of

   *portColon = '\0';

portColon is known to initially contain ":" before the above asignment.

Comment 10

[Stephen P. Morse]

To clarify further:

I start with the URL
I do a search for the colon that separates the host from the port
I set that colon to 0
Now I have the string that includes the host and not the port
I do something with just the host part
Then I restore the string back to its original value, port number and all.

Comment 11

[Blake Ross]

sr=blake

Comment 12

[Stephen P. Morse]

Fix checked in.

Comment 13

[Terri Preston]

Verified patch checked into lxr.mozilla.org

Comment 14

[Stephen P. Morse]

The fix for this bug has introduced a regression as reported in bug 99311.  I 
now believe that the original behavior was correct, i.e., to keep the port 
number as part of the site name.  This is justified by the arguments given in 
bug 99311 -- namely that some sites use different port numbers to distinguish 
different servers on the same machine.

Therefore I'm reopening this bug report and requesting reviews for backing out 
the change.  Then I'll close this report out as wont-fix and I will close out 
bug 99311 as fixed.

cc'ing alecf and bryner for reviews for backing out this fix.

Comment 15

[Alec Flett]

Comment on attachment 43159 [details] [diff] [review]
remove port number from host name

sr=alecf on the backout

Comment 16

[Brian Ryner (not reading)]

r=bryner for the backout

Comment 17

[Stephen P. Morse]

Patch has now been backed out.  Marking as wont-fix -- see discussion in 99311 
for rationale.