Patrick McManus [:mcmanus] Reporter
	Description • 10 years ago

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.

	Nathan Froyd [:froydnj]
	Comment 1 • 10 years ago

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?

	Drew Willcoxon :adw
	Comment 2 • 10 years ago

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.

	maxim zhilyaev Assignee
	Comment 3 • 10 years ago

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

	Drew Willcoxon :adw
	Comment 4 • 10 years ago

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.

	Nathan Froyd [:froydnj]
	Comment 5 • 10 years ago

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?

	Nathan Froyd [:froydnj]
	Comment 6 • 10 years ago

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.

	maxim zhilyaev Assignee
	Comment 7 • 10 years ago

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)?

	Patrick McManus [:mcmanus] Reporter
	Comment 8 • 10 years ago

(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

	Ed Lee :Mardak
	Comment 9 • 10 years ago

maksik, are you able to test this locally to see if a change would fix this problem?

	maxim zhilyaev Assignee
	Comment 10 • 10 years ago

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.

	Ed Lee :Mardak
	Comment 11 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/0d34e5a9ec24

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/0d34e5a9ec24

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 10 years ago

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!

	maxim zhilyaev Assignee
	Comment 14 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 10 years ago

Marvelous, thank you!
https://hg.mozilla.org/releases/mozilla-beta/rev/5c047cecbe5a