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

Running curl with --doh-url seems to turn on verbose output #3660

Closed
bikeseat opened this issue Mar 9, 2019 · 4 comments
Closed

Running curl with --doh-url seems to turn on verbose output #3660

bikeseat opened this issue Mar 9, 2019 · 4 comments
Labels
name lookup DNS and related tech

Comments

@bikeseat
Copy link

bikeseat commented Mar 9, 2019

I did this

curl --doh-url https://1.1.1.1/dns-query ipinfo.io/8.8.8.8

I expected the following

I expected non-verbose output. This gives me what I expect
curl --doh-url https://1.1.1.1/dns-query ipinfo.io/8.8.8.8 2>/dev/null

curl/libcurl version

curl 7.64.1-DEV (x86_64-apple-darwin18.2.0) libcurl/7.64.1-DEV SecureTransport zlib/1.2.11 libidn2/2.1.1 librtmp/2.3 Release-Date: [unreleased] Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp smb smbs smtp smtps telnet tftp Features: AsynchDNS IDN IPv6 Largefile libz NTLM NTLM_WB SSL UnixSockets

operating system

macOS Mojave 10.14.3 (18D109)

@jay
Copy link
Member

jay commented Mar 9, 2019

Confirmed.

diff --git a/lib/doh.c b/lib/doh.c
index f06ed33..746b8ca 100644
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -242,7 +242,6 @@ static CURLcode dohprobe(struct Curl_easy *data,
     ERROR_CHECK_SETOPT(CURLOPT_PROTOCOLS, CURLPROTO_HTTPS);
 #endif
     ERROR_CHECK_SETOPT(CURLOPT_TIMEOUT_MS, (long)timeout_ms);
-    ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
     doh->set.fmultidone = Curl_doh_done;
     doh->set.dohfor = data; /* identify for which transfer this is done */
     p->easy = doh;

@bagder left in by accident?

@jay
Copy link
Member

jay commented Mar 9, 2019

on second thought it should inherit from the easy handle

diff --git a/lib/doh.c b/lib/doh.c
index f06ed33..5040248 100644
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -242,7 +242,8 @@ static CURLcode dohprobe(struct Curl_easy *data,
     ERROR_CHECK_SETOPT(CURLOPT_PROTOCOLS, CURLPROTO_HTTPS);
 #endif
     ERROR_CHECK_SETOPT(CURLOPT_TIMEOUT_MS, (long)timeout_ms);
-    ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
+    if(data->set.verbose)
+      ERROR_CHECK_SETOPT(CURLOPT_VERBOSE, 1L);
     doh->set.fmultidone = Curl_doh_done;
     doh->set.dohfor = data; /* identify for which transfer this is done */
     p->easy = doh;

also what's a good test site for doh, I tried cloudflare but it's not working

curld -v --doh-url https://cloudflare-dns.com/dns-query http://www.test.com
...
* DOH: Too small type A for www.test.com
* DOH: Too small type AAAA for www.test.com

It's a bug because CAINFO is not inherited and I need that in Windows. PR in #3661 to fix that and the OP's issue. And test sites are here.

jay added a commit to jay/curl that referenced this issue Mar 9, 2019
- Inherit most SSL options for the doh handle but not SSL client certs,
  SSL engine, SSL version, SSL ciphers, SSL id cache setting,
  SSL kerberos or SSL gss-api settings.

My thinking for the options not inherited is they are most likely not
intended by the user for the DOH transfer. I did inherit insecure
because I think that should still be in control of the user.

Prior to this change doh did not work for me because CAINFO was not
inherited. Also verbose was set always which AFAICT was a bug (curl#3660).

Fixes curl#3660
Closes #xxxx
@bagder
Copy link
Member

bagder commented Mar 9, 2019

left in by accident?

Oops...

@bagder bagder added the name lookup DNS and related tech label Mar 10, 2019
@jay jay closed this as completed in 9e6af11 Mar 11, 2019
@jay
Copy link
Member

jay commented Mar 11, 2019

Thanks

@lock lock bot locked as resolved and limited conversation to collaborators Jun 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
name lookup DNS and related tech
Development

Successfully merging a pull request may close this issue.

3 participants