77331 - cookperm.txt destroyed after allowing stylesheet image load

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P2)

Description

[Peter Janes]

On every browser restart, cookperm.txt is ignored when you navigate to a page
that uses either images or cookies (I have prompting turned on for both), at
which point you're prompted to allow the image and/or cookie.  If you select
'remember this decision' and then click 'yes' or 'no', the choice is recorded
into cookperm.txt, destroying any entries that already existed.  I'll attach
"before" and "after" files to this bug.

(Note that the home page gets images loaded independent of the cookperm.txt
setting and without any prompts; I haven't verified cookie behaviour yet.)

This occurs on Linux -sea.tar.gz build 2001042308 (late 0.9) on my RedHat Linux
6.2 workstation, but doesn't seem to happen with the same build on my 7.0 system.

I could be mistaken (it's happened before) but didn't there used to be three
fields in cookperm.txt, with one for each of 'allow cookies' and 'allow images'?
 There are only two in these files, except for the last line (even in the
profile where this works).

Comment 1

[Peter Janes]

Attachment: Original cookperm.txt

Comment 2

[Peter Janes]

Attachment: cookperm.txt after restarting browser and navigating to www-int and bugzilla

Comment 3

[Stephen P. Morse]

The fact that the image permissions are ignored is a separate bug -- that's bug 
73848.

The entries in the cookperm.txt file that you posted appear to be correct.  
There would be three fields on a line if you were blocking/accepting both images 
and cookies for the same site.

From the symptoms you are describing, it appears as though the cookperm.txt file 
is not being read in during the second browser session.  Instead a new list is 
created (devoid of the old entries) and that list is then written back to disk 
overwriting the old cookperm.txt.  This sound very much like bug 74858 except 
that that bug was platform-independent whereas this one appears to be specific 
to the redhat lynux 6.2 platform but not for redhat lynux 7.0.  Nor is it 
occuring for win32.

Could you please give a detailed set of instructions, starting with a new 
profile, that demonstrates the problem.  As you can see in bug 74858, I wasn't 
able to reproduce the problem there at first because I wasn't doing *exactly* 
what the reporter was doing (I was opening the image-blocking dialog at the 
start of each session and that was forcing a read of the cookperm.txt file).

Comment 4

[Peter Janes]

I think I've discovered the root cause: loading images from a stylesheet.  (I've
updated the summary.)  It appeared to be happening all of the time because the
internal page I was using as a test tweaks the bug.

Steps to duplicate:

- Create a new profile.
- Select 'Accept images from the originating server only' and 'Warn me before
downloading an image'.
- Load <http://www.w3.org/Style/CSS/Test/current/sec533.htm>, and click
'Remember this choice' then 'Yes' on the 'Load image?' dialog.
- Navigate to sites that send cookies (<http://www.cnn.com/> for example).
- Exit Mozilla, and examine cookperm.txt; notice that CNN and W3 are listed.
- Restart Mozilla and navigate to the W3 page again.  Notice that the 'load
image' dialog appears again, and that cookperm.txt contains only the W3 site
after dismissing the dialog.

Comment 5

[Stephen P. Morse]

I'm a little confused here as to whether we are talking about cookies or images.  
In your description you mention setting prefs for images 
(originating-server-only, warn) but nothing about cookies.  Then later you say 
to navigate to a site that sends cookies, namely cnn.  Was this step necessary?  
Furthermore, you do not mention that you got any warning dialogs from the cnn 
page.  Did you?  Not for cookies I hope (you didn't set the cookie-warning 
pref).  Later you mention that cnn was in the cookperm file.  Was it there for 
cookies or images?  If you didn't get any warning dialogs for cnn, then I don't 
understand how cnn got into the cookperm.txt file.

Comment 6

[Peter Janes]

I originally had instructions to set both, but lost the text and had to retype.
 Sorry!  You should accept any images or cookies that are prompted (CNN will
only prompt for cookies, but load all images).

Accepting a cookie doesn't trigger the problem; accepting an image (which is
only prompted for stylesheets, that I can see) does.

Comment 7

[Stephen P. Morse]

OK, sounds like we are narrowing down on the necessary steps.  Your comment 
about the stylesheet images was interesting because it explains why the 
image-warning box was coming up sometimes and not others (it got broken in bug 
73848 but apparently was working in certain situations and I didn't know why).

What's apparently happening is that the path involving stylesheet images is 
modfying (and rewriting to disk) the in-memory permission list without ever 
having read it in from the cookperm.txt file.

Comment 8

[Stephen P. Morse]

OK, I'm able to reproduce this problem on win32 so it is not a redhat 6.1 
specific problem after all.

Problem is the same as observed in bug 74858.  In that bug code was added to 
make sure the cookperm.txt file was read in before it was used for making any 
cookie decisions.  That same sort of change needs to be made before making any 
image decisions as well.  Probably the reason I forgot to do it for images was 
because imageblocking seemed to be completely broken due to bug 73848.  But as 
peterj correctly discovered, imageblocking is indeed still working for 
stylesheet images only.

Attaching patch to make sure we read in the cookperm.txt file before using it to 
make image-blocking decisions.

Comment 9

[Stephen P. Morse]

Attachment: patch to make sure cookperm.txt is read in before being used for images

Comment 10

[Stephen P. Morse]

cc'ing valeski for review and alecf for super-review.

Comment 11

[Alec Flett]

sr=alecf on the assumption that we're not re-reading the file every time we
encounter an image.

Comment 12

[Stephen P. Morse]

No, we're not rereading it, we are merely checking to make sure it has been read 
in and reading it only if we discover that it has not yet been read.

Comment 13

[Judson Valeski]

r=valeski

Comment 14

[Viswanath Ramachandran]

Steve, please check this in!

Comment 15

[Stephen P. Morse]

Fix checked in

Comment 16

[Stephen P. Morse]

Fix backed out.

Comment 17

[Judson Valeski]

was the fix backed out for any particular reason?

Comment 18

[Stephen P. Morse]

Fix checked in again.

Comment 19

[Tom Everingham]

verified 7/10 builds, all platforms