475702 - nsICertOverrideService.rememberValidityOverride leaks an nsStringBuffer for each call

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Dave Townsend [:mossop]]

The tests in bug 435743 are revealing some memory leaks, looks like a few different ones.

One is that adding the BadCertHandler to the notificationCallbacks of an XMLHttpRequest channel and then requesting a https URL where the cert is untrusted causes us to leak.

I don't think this is an EM bug but tracking it here for now.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Ok we're simply leaking the proxy to the nsIBadCertListener2 whenever it is used.

Comment 2

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Might as well fix this other PSM leak here too.

nsStringBuffer::Alloc returns an already AddRefed string buffer so assigning to the nsRefPtr pushes the refcount too high. There doesn't seem much of a need of the nsRefPtr anyway so just release it when done.

Comment 3

[Dave Townsend [:mossop]]

Attachment: patch rev 3

Ok so the badcertlistener leak already has a patch in bug 459657 so I'll just cover the stringbuffer leak here.

Comment 4

[Kai Engert (:KaiE:)]

Comment on attachment 359291 [details] [diff] [review]
patch rev 3

r=kaie thanks for the patch

Comment 5

[Dave Townsend [:mossop]]

Pushed: http://hg.mozilla.org/mozilla-central/rev/c834f54de201

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 359291 [details] [diff] [review]
patch rev 3

Looking for approval to land this small leak fix on the branch. Very low risk

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 359291 [details] [diff] [review]
patch rev 3

a191=beltzner

Comment 8

[Dave Townsend [:mossop]]

Landed on branch http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f1a71f418d91