Closed Bug 995806 Opened 6 years ago Closed 5 years ago

test_DirectoryLinksProvider.js xpcshell test makes tcp connection to example.com

Categories

(Firefox :: New Tab Page, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 32
Tracking Status
firefox31 --- fixed
firefox32 --- fixed

People

(Reporter: mcmanus, Assigned: mzhilyaev)

References

Details

Attachments

(2 files)

The test infrastrucutre is meant to be self contained - so xpcshell, mochitests, and crashtests need to be self hosted.

toolkit/modules/tests/xpcshell/test_DirectoryLinksProvider.js actually makes a connection via nsichannel to example.com 93.184.216.119 every time the test is executed.

this should be fixed to be localhost only.
Drew, your name is on this file as the primary reviewer.

Can the test just request things from the local http server that gets set up for the test?  Can you find someone to make that change or other necessary changes?
Flags: needinfo?(adw)
Olivier, Maxim, Marina, would you be able to look at this?  The example.com URLs in the test are all tile/site sources, not links metadata sources, so I don't see how they end up getting loaded.
Flags: needinfo?(adw)
Drew, I think example.com URLs are never loaded, they are always a part of the data section of the data url constructed by the test as in:

  let data = {
    "en-US": [{url: "http://example.com", title: "US"}],
    "zh-CN": [
              {url: "http://example.net", title: "CN"},
              {url:"http://example.net/2", title: "CN2"}
    ],
  };
  let dataURI = 'data:application/json,' + JSON.stringify(data);

Let me know if understood your concern correctly and addressed it
Yes, that's what it looks like to me, too, but apparently something in the test is actually loading example.com.  I thought I might have overlooked something that one of you might see.
Finally took at closer look at this and got:

 0:03.92 [31042] WARNING: Non-local hostname nosuchhost resolved: 2218907459:0: file /home/froydnj/src/gecko-dev.git/netwerk/dns/nsHostResolver.cpp, line 1193
 0:03.92 BAD CONNECT: connecting to nosuchhost 0
 0:03.92 Hit MOZ_CRASH(Attempting to connect to non-local address!) at /home/froydnj/src/gecko-dev.git/netwerk/base/src/nsSocketTransport2.cpp:1190

which is coming from here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/tests/xpcshell/test_DirectoryLinksProvider.js#208

This is one of the annoying things about disabling non-local connections: xpcshell has somewhat different behavior than the browser in this case.  (The browser returns nice errors back to the HTML page that spawned the request...sometimes.)

Patrick, do you have any networking insight into what we can do here?
Flags: needinfo?(mcmanus)
And we hit it here, too:

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/tests/xpcshell/test_DirectoryLinksProvider.js#289

I'm assuming that we'd rather not disable these bits of the test if we can help it.
These are negative tests that ensure that bad urls and unresolved hosts are handled correctly.
Patric, what would you suggest for an appropriate URL and hostname for such tests (a non-existent IP, perhaps)?
(In reply to maxim zhilyaev from comment #7)
> These are negative tests that ensure that bad urls and unresolved hosts are
> handled correctly.
> Patric, what would you suggest for an appropriate URL and hostname for such
> tests (a non-existent IP, perhaps)?

yes - if you use a hostname that doesn't resolve at all then I wouldn't expect a problem as I remember the enforcement patch. (iirc it asserts when trying to connect to a non-localhost IP and only warns on trying to look up names other than localhost).

something like nosuchhost.localhost. ought to be pretty safe
Flags: needinfo?(mcmanus)
Assignee: nobody → mzhilyaev
maksik, are you able to test this locally to see if a change would fix this problem?
Component: Tabbed Browser → New Tab Page
Tested with tcpdump on local machine whiel running the test

>> tcpdump -vvv -s 0 -l -n port 53

10:16:11.132372 IP (tos 0x0, ttl 255, id 52101, offset 0, flags [none], proto UDP (17), length 66)
    10.252.26.250.49167 > 10.252.75.5.53: [udp sum ok] 19606+ A? nosuchhost.localhost. (38)
10:16:11.135500 IP (tos 0x0, ttl 63, id 34130, offset 0, flags [none], proto UDP (17), length 107)
    10.252.75.5.53 > 10.252.26.250.49167: [udp sum ok] 19606 NXDomain* q: A? nosuchhost.localhost. 0/1/0 ns: localhost. [1d] SOA localhost. root.localhost. 42 10800 900 604800 86400 (79)

network utility show local IP as 10.252.26.250

So, the fix appears to work.
Attachment #8435913 - Flags: review?(mcmanus)
Attachment #8435913 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/mozilla-central/rev/0d34e5a9ec24
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
maxim, we'd like to backport bug 995417 to mozilla-beta but are running into this bug in the process. However, this patch doesn't apply cleanly to that branch. Can you please post a branch patch when you get a chance? :)

Thanks!
Flags: needinfo?(mzhilyaev)
Rayn, the original patch is not applicable to mozilla-beta since mozilla-beta does not have functionality that the patched test cases are testing.  However, there's an offending line that issues a non-local http request found here:
https://github.com/mozilla/gecko-dev/blob/beta/toolkit/modules/tests/xpcshell/test_DirectoryLinksProvider.js#L130

I am attaching a patch that fixes this issue.
Please note that this code will is removed in master
Flags: needinfo?(mzhilyaev)
You need to log in before you can comment on or make changes to this bug.