Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23105 )

Change subject: curl_util-test: fix timeout test for newer libcurl
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23105/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23105/1//COMMIT_MSG@9
PS1, Line 9: libcurl >=7.80 rejects host names containing underscores
> Is this the native curl library version? Is there any curl documentation li
I guess it might be OS-dependent and not the curl library itself, but rather a 
library in the resolver chain. Different Linux distros often add extra patches 
on top of upstream versions, so it will require extra legwork to figure out the 
actual root cause behind the rejection of invalid DNS names reported by curl.

However, regardless of the actual reason behind the rejection of the incorrect 
DNS name, I think it makes sense to make the DNS name valid at least to avoid 
unexpected failures like described.  Maybe, also update the description of the 
changelist just to point to what's been fixed, not the 
moot-and-not-yet-verified-root-cause.

BTW, in my case (curl 8.13.0 on macOS built from MacPorts), I don't see any 
rejection of the invalid DNS name:

  mbp-alex:kudu[master]$ time curl http://not_exist_host:12345
  curl: (6) Could not resolve host: not_exist_host

  real  0m0.045s
  user  0m0.013s
  sys   0m0.017s


  mbp-alex:kudu[master]$ time curl http://non-existing-host:12345
  curl: (6) Could not resolve host: non-existing-host

  real  0m0.044s
  user  0m0.013s
  sys   0m0.017s


  mbp-alex:kudu[master]$ curl --version
  curl 8.13.0 (x86_64-apple-darwin23.6.0) libcurl/8.13.0 OpenSSL/3.5.0 
zlib/1.3.1 brotli/1.1.0 zstd/1.5.7 libidn2/2.3.8 libpsl/0.21.5 nghttp2/1.66.0
  Release-Date: 2025-04-02
  Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns 
mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp ws wss
  Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IDN IPv6 Largefile 
libz NTLM PSL SSL threadsafe TLS-SRP UnixSockets zstd


http://gerrit.cloudera.org:8080/#/c/23105/1/src/kudu/util/curl_util-test.cc
File src/kudu/util/curl_util-test.cc:

http://gerrit.cloudera.org:8080/#/c/23105/1/src/kudu/util/curl_util-test.cc@41
PS1, Line 41: notexist.host
nit: if you are updating this anyway, maybe change the name to something closer 
to correct English, e.g.:

  non-existing-host
  non.existing.host
  host.does.not.exist



--
To view, visit http://gerrit.cloudera.org:8080/23105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e1c3b58b9805e117d6ca013d868addcb59c1711
Gerrit-Change-Number: 23105
Gerrit-PatchSet: 1
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Wed, 02 Jul 2025 03:10:29 +0000
Gerrit-HasComments: Yes

Reply via email to