Last Comment Bug 767159 - remove synchronous DNS resolution in nsSOCKSIOLayer.cpp
: remove synchronous DNS resolution in nsSOCKSIOLayer.cpp
Product: Core
Classification: Components
Component: Networking: DNS (show other bugs)
: Trunk
: All All
-- major (vote)
: mozilla16
Assigned To: Patrick McManus [:mcmanus]
: Patrick McManus [:mcmanus]
Depends on: CVE-2013-0764
Blocks: 766973
  Show dependency treegraph
Reported: 2012-06-21 14:34 PDT by Josh Aas
Modified: 2013-01-10 09:56 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch 0 (8.83 KB, patch)
2012-06-26 13:44 PDT, Patrick McManus [:mcmanus]
cbiesinger: review+
Details | Diff | Splinter Review

Description User image Josh Aas 2012-06-21 14:34:46 PDT
219 PRStatus
220 nsSOCKSSocketInfo::ConnectToProxy(PRFileDesc *fd)
234         rv = dns->Resolve(mProxyHost, 0, getter_AddRefs(mDnsRec));

We probably shouldn't ever make synchronous DNS resolution calls, and we definitely shouldn't do it on the main thread. Removing this stuff is a high priority.
Comment 1 User image Patrick McManus [:mcmanus] 2012-06-26 13:44:59 PDT
Created attachment 636862 [details] [diff] [review]
patch 0

while I do some more testing lets call this WIP
Comment 2 User image Patrick McManus [:mcmanus] 2012-06-27 07:20:27 PDT
Comment on attachment 636862 [details] [diff] [review]
patch 0

confirmed behavior before/after patch matches for:

* normal dns based proxy
* ip based proxy
* dns resolution that takes a long time
* invalid host name
* valid hostname, but invalid proxy

Christian, do you have time to look at this?
Comment 3 User image Christian :Biesinger (don't email me, ping me on IRC) 2012-06-28 14:56:25 PDT
Comment on attachment 636862 [details] [diff] [review]
patch 0

nice :)
Comment 4 User image Patrick McManus [:mcmanus] 2012-06-28 15:27:11 PDT
Comment 5 User image Ed Morley [:emorley] 2012-06-29 00:46:14 PDT

Note You need to log in before you can comment on or make changes to this bug.