panic on TLS request without DNS name

[martinohmann]

Hi there!

I was poking around with HTTPS requests on localhost and hit the panic below, where I expected an error.

What I did was using ureq to do a bogus request to https://127.0.0.1:7777 where I had a server running which speaks HTTP only, but not HTTPS. I was expecting an error but got a panic instead.

Here is a minimal reproducer, which requests the homepage of Cloudflare's DNS service via HTTPS by IP:

fn main() {
    match ureq::get("https://1.1.1.1").call() {
        Ok(resp) => println!("ok: {:?}", resp),
        Err(err) => println!("error: {:?}", err),
    }
}

Expected

"ok" and the contents of the response

Actual

Panic backtrace:

thread 'main' panicked at 'invalid DNS name: InvalidDnsNameError', /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/stream.rs:370:48
stack backtrace:
   0: rust_begin_unwind
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/panicking.rs:101:14
   2: core::result::unwrap_failed
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/result.rs:1617:5
   3: core::result::Result<T,E>::expect
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/result.rs:1259:23
   4: ureq::stream::connect_https
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/stream.rs:370:9
   5: ureq::unit::connect_socket
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/unit.rs:349:20
   6: ureq::unit::connect_inner
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/unit.rs:241:37
   7: ureq::unit::connect
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/unit.rs:167:20
   8: ureq::request::Request::do_call
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/request.rs:116:24
   9: ureq::request::Request::call
             at /home/mohmann/.cargo/registry/src/github.com-1ecc6299db9ec823/ureq-2.3.0/src/request.rs:79:9
  10: tls_panic::main
             at ./src/main.rs:2:11
  11: core::ops::function::FnOnce::call_once
             at /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[martinohmann]

I just saw that #393 already reported a similar issue. Feel free to close it in favor of the other although i think this one is different as it's a panic.

I'm wondering if there is a way to convert the panic here into an error instead: https://github.com/algesten/ureq/blob/main/src/stream.rs#L370

I think even if rustls does not support IP based certificates yet we should error out instead of panicking here. What do you think?

I would be willing to work on the fix if you like.

[jsha]

Thanks for the report, and thanks to @AlyoshaVasilieva for the fix! I very much agree we should error out instead of panicking. I've merged the fix. I'd like to do a patch release soon. @algesten are you available to review #437 this weekend? Looks like I don't have permission to merge without review.

Also I'd like to follow this up with a unittest for the behavior when fetching an IP address URL.