Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local test errors #31

Closed
KristofferC opened this issue Oct 9, 2020 · 7 comments · Fixed by #48
Closed

Local test errors #31

KristofferC opened this issue Oct 9, 2020 · 7 comments · Fixed by #48

Comments

@KristofferC
Copy link
Sponsor Member

Running the tests locally I get:

errors: Test Failed at /home/kc/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:93
  Expression: startswith(err.msg, "Could not resolve host")
   Evaluated: startswith("ssl_handshake returned - mbedTLS: (-0x7780) SSL - A fatal alert message was received from our peer", "Could not resolve host")
Stacktrace:
 [1] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:93
 [2] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1144
 [3] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:84

progress: Test Failed at /home/kc/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:173
  Expression: 11  length(progress)  12
   Evaluated: 11  9  12
Stacktrace:
 [1] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:173
 [2] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1144
 [3] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:168

progress: Test Failed at /home/kc/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:176
  Expression: all((p.dl_now == max(0, i - shift) for (i, p) = enumerate(progress)))
Stacktrace:
 [1] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:176
 [2] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1144
 [3] top-level scope
   @ ~/julia/usr/share/julia/stdlib/v1.6/Downloads/test/runtests.jl:168
@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Oct 9, 2020

The progress tests are probably just doomed and need to be significantly less strict. Maybe it should be fine as long as the progress reports are strictly increasing up to the expected number of bytes.

The other one seems like your host gets a different error when doing a DNS lookup of an invalid hostname. What happens if you do host invalid? What if you do host invalid 1.1.1.1? A lot of IPS do this shady business where they give fake answers to DNS queries so they can spam you when you enter a bad domain in your browser, which would mess up this test. The domain invalid is officially not supposed to exist, so this is strictly a violation of the DNS spec, but then again, so is lying when someone asks for a non-existent domains.

@KristofferC
Copy link
Sponsor Member Author

I'm in an office hotel so maybe they do some "shady" stuff:

❯ host invalid
invalid.kuststaden.se has address 104.27.142.62
invalid.kuststaden.se has address 104.27.143.62
invalid.kuststaden.se has address 172.67.210.161
invalid.kuststaden.se has IPv6 address 2606:4700:3037::681b:8e3e
invalid.kuststaden.se has IPv6 address 2606:4700:3037::681b:8f3e
invalid.kuststaden.se has IPv6 address 2606:4700:3032::ac43:d2a1

❯ host invalid 1.1.1.1
Using domain server:
Name: 1.1.1.1
Address: 1.1.1.1#53
Aliases:

invalid.kuststaden.se has address 104.27.143.62
invalid.kuststaden.se has address 172.67.210.161
invalid.kuststaden.se has address 104.27.142.62
invalid.kuststaden.se has IPv6 address 2606:4700:3032::ac43:d2a1
invalid.kuststaden.se has IPv6 address 2606:4700:3037::681b:8e3e
invalid.kuststaden.se has IPv6 address 2606:4700:3037::681b:8f3e

@StefanKarpinski
Copy link
Sponsor Member

Oh wow, they even intercept DNS requests to Google's 1.1.1.1 DNS server—that's extra shady! I'm not sure what we should do here: invalid is the officially documented invalid top-level domain. Try host something.invalid?

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Oct 11, 2020

Here's a possibility: check first if Sockets.getaddrinfo("invalid") throws the expected DNSError, if it does, proceed with the test; if it doesn't, print a warning that the current host appears to have a dirty, lying upstream DNS server and that we're skipping the test because of that.

Another option instead of skipping the test is to do the test but use the CURLOPT_DOH_URL option with a known DNS-over-HTTPS server like https://dns.google/dns-query to avoid using the system DNS and prevent interception of DNS queries. Can you try these commands with curl and see if they work:

  • curl -I --doh-url https://dns.google/dns-query https://julialang.org (should return 200 OK)
  • curl -I --doh-url https://dns.google/dns-query https://invalid (should be a DNS lookup failure)

@KristofferC
Copy link
Sponsor Member Author

Try host something.invalid?

❯ host something.invalid
Host something.invalid not found: 2(SERVFAIL)

@StefanKarpinski
Copy link
Sponsor Member

Oh interesting! So that seems to work correctly. Or rather not work correctly? Fail correctly? You know what I mean. Ok, so we should probably do the following:

  1. Change the invalid domain to invalid.invalid or something like that.
  2. Maybe also do the Sockets.getaddrinfo(invalid) dance as well, in case others have even shadier DNS.
  3. Could add support for DNS-over-HTTPS at some point, but not pressing.

StefanKarpinski added a commit that referenced this issue Oct 21, 2020
Even though the entire `.invalid` TLD is officialy invalid so no
DNS server should ever resolve it to an IP address, it seems like
ISPs are more likely to lie when the domain is just `invalid` than
if it's `domain.invalid`, so we'll use that instead.

Fixes half of #31.
StefanKarpinski added a commit that referenced this issue Oct 21, 2020
Although these tests work well enough in ideal network conditions,
too often, especially during CI of Julia itself, we don't get a
full set of progress updates. This relaxes the test to just make
sure that the progress values are non-decreasing, which is pretty
weak, but it's something and at least we test the API.
@StefanKarpinski
Copy link
Sponsor Member

These should be better now. If you're still behind the same ISP could you check if this passes locally now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants