cookperm.txt index wrong, two image entries, no cookie entry

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: jmd, Assigned: mvl)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030824 Mozilla
Firebird/0.6.1+ genereates a cookperm.txt with this index/comment thing at the top:

%0>     image
%1>     image
%2>     popup

('>' are tabs)

Older builds correctly show:

%0>     cookie
%1>     image
%2>     popup

I don't think mozilla looks up the values of this index for functional purposes
and it is more a comment for user's reference. Setting severity trivial, unless
anyone knows that this is actually used for something.
(Assignee)

Comment 1

15 years ago
The headers are used, and it is correct that the numbers can change. Some
extension could add a new type ("flash" for example) and that could end up
having index 1.
(Reporter)

Comment 2

15 years ago
oh my, oh my.

Just noticed none of my cookie permissions work with this build. Had kind of
noticed something was awry yesterday, but just now figured it out while cleaning
up my cookies.

->major.
Severity: trivial → major
(Reporter)

Comment 3

15 years ago
A little more info, this is a new profile, created with the build mentioned. I
was browsing around the new Firebird preference UI a bit, looking at the
exceptions dialog, etc, so I assume some part of that code duplicated the image
entry, at the expense of cookies.

When I would click 'remember' and remove a cookie from the "Show Cookies" panel,
and it didn't show in the exceptions panel, I figured that panel just wasn't
functional yet. It works properly now that I manually added cookie to cookperm.
I had NOT touched cookperm manually until that point. Somehow Firebird generated
that file like that.
Summary: cookperm.txt index/comment thing wrong → cookperm.txt index wrong, two image entries, no cookie entry
(Assignee)

Comment 4

15 years ago
all mine
Assignee: darin → mvl
Component: Cookies → Image Blocking
(Assignee)

Comment 5

15 years ago
Created attachment 130558 [details] [diff] [review]
patch v1

This is what happened:
start with no cookperm.txt. Because the is no file, there are no types defined.
If you add say an image permission (by right clicking the image), that type
gets added with the first free index, 0. Now, restart mozilla. Now, there is a
file, and the type gets read. The other types are empty, and filled with
defaults. But the default for 1 is images. But 0 was already images too. Double
type. Problems.

This patchs adds the default types when there is no cookperm.txt.
If there is a file, it makes sure to add types that might be not in the file.
(In the correct order, to be able to read old style files)
(Assignee)

Updated

15 years ago
Attachment #130558 - Flags: review?(dwitte)
(Reporter)

Comment 6

15 years ago
> the part that defines the types

Oh good, I'm not the only one who doesn't know what to call that thing.

> // Old files, wothout

Typo.

Comment 7

15 years ago
Comment on attachment 130558 [details] [diff] [review]
patch v1

looks good to me.  sr=darin
Attachment #130558 - Flags: superreview+

Comment 8

15 years ago
Comment on attachment 130558 [details] [diff] [review]
patch v1

>+      NS_ASSERTION(GetTypeIndex(buffer.get() + stringIndex, PR_FALSE), "Corrupt cookperm.txt file");

doesn't GetTypeIndex() return -1 for not found? in which case you want
NS_ASSERTION(GetTypeIndex(...) != -1)?

r=dwitte. wow, darin has time to give out sr's without even being asked
nowadays :)

hey, this will be your first landing with your shiny new cvs account, congrats
;)
Attachment #130558 - Flags: review?(dwitte) → review+
(Assignee)

Comment 9

15 years ago
Comment on attachment 130558 [details] [diff] [review]
patch v1

Asking approval for 1.5.
This is a low risk patch that fixes that reading / creating cookperm.txt might
go wrong in some cases.
Attachment #130558 - Flags: approval1.5?

Comment 10

15 years ago
Comment on attachment 130558 [details] [diff] [review]
patch v1

a=asa (on behalf of drivers) for checkin to Mozilla 1.5
Attachment #130558 - Flags: approval1.5? → approval1.5+
(Assignee)

Comment 11

15 years ago
checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

15 years ago
The assertion is the wrong way around. Now it asserts when it shouldn't.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 14

15 years ago
Comment on attachment 130689 [details] [diff] [review]
fix assertion

r+, no need for sr here, if dbaron's okay with that.
Attachment #130689 - Flags: review+
(Assignee)

Comment 15

15 years ago
Comment on attachment 130689 [details] [diff] [review]
fix assertion

Assking approval for 1.5. This is a low risk change that only affects debug
builds.It fixes some false assertions being fired.
Attachment #130689 - Flags: approval1.5?

Comment 16

15 years ago
Comment on attachment 130689 [details] [diff] [review]
fix assertion

a=asa (on behalf of drivers) for checkin to Mozilla 1.5
Attachment #130689 - Flags: approval1.5? → approval1.5+

Updated

15 years ago
Attachment #130689 - Flags: superreview+
(Assignee)

Comment 17

15 years ago
assertion fix is in too.
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.