577073 - PAC: does not respect network.dns.disablePrefetch and contains a "use after delete"

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Corentin Chary]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.6) Gecko/20100628 Gentoo Firefox/3.6.6
Build Identifier: 

1. mPACMAN Does not respect network.dns.disablePrefetch

If you set network.dns.disablePrefetch, mPACMAN should not try to prefetch
DNS for future pac usage.

2. mPACMAN contain a "use after delete" in OnLookupComplete

NS_RELEASE_THIS() is called before Complete().


Reproducible: Always

Steps to Reproduce:
1. Use a Proxy Pac file (always returning myproxy.com:80)
2. Use a filtered DNS server
3. Disable DNS prefetching
4. Try to load http://google.com

Actual Results:  
Firefox try to resolve google.com

Expected Results:  
Firefox should not try tro resolve google.com if no pac function need to do that.

Comment 1

[Corentin Chary]

Attachment: Fix the bug

Comment 2

[Corentin Chary]

This is a problem, because if the DNS is filtered and only answer to some request, there will be a huge freeze each time Firefox try to resolve non-necessary DNS.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Corentin, thanks for filing this and for the patch!

Do you think you could regenerate the diff with the -p option and -u8?  That would make it a lot easier to review...

Christian, do you know this code?

Comment 4

[Corentin Chary]

Attachment: 0001-fix-nsPACMAN-prefetching-policy.patch v2

Here it is. (note, -u8 was deprecated on my diff, I used -U 8 instead).

Comment 5

[Corentin Chary]

Attachment: 0001-fix-nsPACMAN-prefetching-policy.patch v3

Ooops .. previous patch was reversed.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 456545 [details] [diff] [review]
0001-fix-nsPACMAN-prefetching-policy.patch v3

The flags argument should be PRUint32.  Other than that, looks good.  Do you want to make that change, or should I?

For future reference, you want to request review on your patch from the module owner or one of the peers to make sure it doesn't get lost.  See http://www.mozilla.org/owners.html

Comment 7

[Corentin Chary]

Attachment: 0001-fix-nsPACMAN-prefetching-policy.patch v4

Here is the patch with requested change.

And thanks for the tip, I'm not very familiar with Mozilla dev Workflow :).

Comment 8

[Boris Zbarsky [:bzbarsky]]

Thanks for updating that!

The workflow is sort of documented in way-too-much-detail at http://www-archive.mozilla.org/hacking/life-cycle.html (except super-review is no longer required in most cases).

I'll get this landed tomorrow, when I can watch the tree.

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/0e1da826a604