300301 - crash when using nsICookiePermission to set cookie access [@nsPermissionManager::Write]

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Pasi P. Liimatainen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050710 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050710 Firefox/1.0+

Using nsICookiePermission to allow cookies from an URI, set back to default and
reallowing crashes Firefox (both 1.0.4 and Deer Park Alpha 2). Happens both when
executed from XUL or Javascript console.

Reproducible: Always

Steps to Reproduce:
Execute the following:
var uri =
Components.classes["@mozilla.org/network/standard-url;1"].createInstance(Components.interfaces.nsIURI);
uri.spec = "http://fail.ing.com/";
var cookiePermissionService =
Components.classes["@mozilla.org/cookie/permission;1"].getService(Components.interfaces.nsICookiePermission);
cookiePermissionService.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_ALLOW);
cookiePermissionService.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_DEFAULT);
cookiePermissionService.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_ALLOW);
Actual Results:  
1) Crash when executing the ACCESS_ALLOW after ACCESS_DEFAULT.
2) hostperm.1 contains the expected line "host	cookie	1	fail.ing.com"
3) sometimes (depending on the timing of the crash) hostperm.1 also contains
lines such as "host	image	192	€aÔ¸Ù¥xq|", "host	install	139
€aÔ¸Ù¥xq|", and "host	cookie	212	€aÔ¸Ù¥xq|". Always the three (image,
install and cookie), but the numbers and the "domain" differ from crash to another.

Expected Results:  
Would expect cookies from "fail.ing.com" to be allowed, defaulted and reallowed
without crashing.

The first ACCESS_ALLOW always works as expected, the domain is added to hostperm.1.
The ACCESS_DEFAULT's effect on hostperm.1 is as expected, the domain is removed
from the file.
The second ACCESS_ALLOW crashes Firefox (90% of cases).

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Can you provide a stack trace or a talkback ID?

Comment 2

[Pasi P. Liimatainen]

Talkback ID TB7367168X

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Stack Signature	 nsPermissionManager::Write c63a2977
Product ID	FirefoxTrunk
Build ID	2005071006
Trigger Time	2005-07-10 15:47:50.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (0041dd10)
URL visited	Javascript console
User Comments	Executed the following: var uri =
Components.classes["@mozilla.org/network/standard-url;1"].createInstance(Components.interfaces.nsIURI);
uri.spec = "http://fail.ing.com/"; var cookiePermissionService =
Since Last Crash	47 sec
Total Uptime	139 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 912

Stack Trace 	

nsPermissionManager::Write 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 912]
nsPermissionManager::DoLazyWrite 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 826]
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 146]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 61]
kernel32.dll + 0x16d4f (0x7c816d4f)

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #0)
> var uri =
>
Components.classes["@mozilla.org/network/standard-url;1"].createInstance(Components.interfaces.nsIURI);
> uri.spec = "http://fail.ing.com/";

This is the wrong way to create an uri. You should use nsIIOService. Does it
still crash if you use that?

Comment 5

[Pasi P. Liimatainen]

Yes, it still crashes even if the URI is created via nsIIOService:

var ioService =
Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
var uri = ioService.newURI("http://fail.ing.com/", null, null);
var cookiePermission =
Components.classes["@mozilla.org/cookie/permission;1"].getService();
cookiePermission.QueryInterface(Components.interfaces.nsICookiePermission);
cookiePermission.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_ALLOW);
cookiePermission.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_DEFAULT);
cookiePermission.setAccess(uri,
Components.interfaces.nsICookiePermission.ACCESS_ALLOW);

The above does crash, but there's the difference that it only generates a single
ill formed line in hostperm.1 (like "host	cookie	78	˜íø½ñNÏ5`žÏ5`hcÝ").

Talkback ID TB7387807W.

Comment 6

[William Bumgarner [:zsinj]]

Same stack trace: [@nsPermissionManager:Write]

Stack Signature	 nsPermissionManager::Write c63a2977
Product ID	FirefoxTrunk
Build ID	2005071006
Trigger Time	2005-07-11 09:38:10.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (0041dd10)
URL visited	Javascript console
User Comments	Executed the following: var ioService =
Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);var
uri = ioService.newURI("http://fail.ing.com/", null, null);var cookiePermission =
Since Last Crash	46 sec
Total Uptime	3268 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 912
Stack Trace 	
nsPermissionManager::Write 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 912]
nsPermissionManager::DoLazyWrite 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/extensions/cookie/nsPermissionManager.cpp,
line 826]
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 146]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 61]
kernel32.dll + 0x16d4f (0x7c816d4f)

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

This patch should fix things. It adds a check after setting a permission to see
if maybe all the permissions for the given host are empty, so that the entry
can be removed.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in