Response::new panics with an invalid `TryInto<StatusCode>`

[orbli]

Hello http-types dev.

I'm facing a situation where http_types/response.rs:L63 will throw out a panic. I'm well aware that this case is very impossible but I still often encounter it with connection to some third-party software, and hence would like you to turn it into returning error instead of throwing a panic that I cannot handle on my side.

[jbr]

Http-types should never panic on malformed data, so this is a bug 👍 I'm not sure if this is a bug, details below

[jbr]

On further inspection: How are you creating a Response with an invalid StatusCode?

let invalid_status = 800;

// instead of this:
let response = http_types::Response::new(invalid_status); // panic

// try doing this:
let response = match invalid_status.try_into::<http_types::StatusCode>() {
    Ok(status) => http_types::Response::new(status),
    Err(_) => todo!("handle the error")
}

Most of the time, the status code is hardcoded by a human or pre-validated by async-h1. In the less common circumstances where neither of those is true, it seems reasonable to expect that the conversion happen outside of Response::new. This has zero computational cost due to the TryInto bound.

[Fishrock123]

https://github.com/http-rs/http-types/blob/52cc143762c0bf0d3256c8ca44a8ea37c8e6aa3f/src/response.rs#L62-L64

I believe this is covered under https://github.com/http-rs/http-types/issues/303 and in the future (http-types 3.0) Response::new() should instead return a Result.

[orbli]

@jbr

Hello for some half year passed

I've got the error today again and hence providing stacktrace:

2021-09-26 16:28:54 thread 'async-std/runtime' panicked at 'Could not convert into a valid `StatusCode`: Invalid status code', /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/http-types-2.12.0/src/response.rs:63:14
2021-09-26 16:28:54 stack backtrace:
2021-09-26 16:28:54 0: 0x55b0aae0a220 - std::backtrace_rs::backtrace::libunwind::trace::ha0ad43e8a952bfe7
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
2021-09-26 16:28:54 1: 0x55b0aae0a220 - std::backtrace_rs::backtrace::trace_unsynchronized::h6830419c0c4130dc
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
2021-09-26 16:28:54 2: 0x55b0aae0a220 - std::sys_common::backtrace::_print_fmt::h8f3516631ffa1ef5
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys_common/backtrace.rs:67:5
2021-09-26 16:28:54 3: 0x55b0aae0a220 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::he1640d5f0d93f618
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys_common/backtrace.rs:46:22
2021-09-26 16:28:54 4: 0x55b0aae2d78c - core::fmt::write::h88012e1f01caeebf
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/fmt/mod.rs:1115:17
2021-09-26 16:28:54 5: 0x55b0aae037f5 - std::io::Write::write_fmt::h360fa85b30182555
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/io/mod.rs:1665:15
2021-09-26 16:28:54 6: 0x55b0aae0c1cb - std::sys_common::backtrace::_print::ha1f00492f406a015
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys_common/backtrace.rs:49:5
2021-09-26 16:28:54 7: 0x55b0aae0c1cb - std::sys_common::backtrace::print::hd54561b13feb6af3
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys_common/backtrace.rs:36:9
2021-09-26 16:28:54 8: 0x55b0aae0c1cb - std::panicking::default_hook::{{closure}}::h84fe124cd0864662
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:208:50
2021-09-26 16:28:54 9: 0x55b0aae0bca1 - std::panicking::default_hook::h5a8e74a76ce290a7
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:225:9
2021-09-26 16:28:54 10: 0x55b0aae0c894 - std::panicking::rust_panic_with_hook::h67c812a4fe9d4c91
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:622:17
2021-09-26 16:28:54 11: 0x55b0aae0c377 - std::panicking::begin_panic_handler::{{closure}}::h33f9c1b96af300d7
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:519:13
2021-09-26 16:28:54 12: 0x55b0aae0a71c - std::sys_common::backtrace::__rust_end_short_backtrace::h51bae64be5921f0e
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys_common/backtrace.rs:141:18
2021-09-26 16:28:54 13: 0x55b0aae0c2d9 - rust_begin_unwind
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:515:5
2021-09-26 16:28:54 14: 0x55b0aa68c631 - core::panicking::panic_fmt::h12a3a3c256485fca
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:92:14
2021-09-26 16:28:54 15: 0x55b0aa68c723 - core::result::unwrap_failed::h2d8d0952e3250de9
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:1599:5
2021-09-26 16:28:54 16: 0x55b0aaa56e5e - http_types::response::Response::new::h94b90955c82cf31f
2021-09-26 16:28:54 17: 0x55b0aaa5f1f5 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h860933b672c319da
2021-09-26 16:28:54 18: 0x55b0aa974363 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h39e3d19aaf121f80
2021-09-26 16:28:54 19: 0x55b0aaa4e843 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h2ada6ea28162c611
2021-09-26 16:28:54 20: 0x55b0aa963288 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h2db9d2f841497ae6
2021-09-26 16:28:54 21: 0x55b0aa9dbeca - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hae72dc85fd89cc0d
2021-09-26 16:28:54 22: 0x55b0aa94e84d - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h1d1397b35a6be0ed
2021-09-26 16:28:54 23: 0x55b0aa7be806 - std::thread::local::LocalKey<T>::with::ha472bac7dfb329b9
2021-09-26 16:28:54 24: 0x55b0aa9a2329 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h65c810b5b65b244a
2021-09-26 16:28:54 25: 0x55b0aaa37037 - async_task::raw::RawTask<F,T,S>::run::hade3e2daf07972f2
2021-09-26 16:28:54 26: 0x55b0aaddb902 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hcd0d75067b7dcffc
2021-09-26 16:28:54 27: 0x55b0aadd9171 - async_io::driver::block_on::hb0568842c0911baa
2021-09-26 16:28:54 28: 0x55b0aaddaad6 - async_global_executor::threading::thread_main_loop::hdfbeb4f3ee8d6209
2021-09-26 16:28:54 29: 0x55b0aadd7e07 - std::sys_common::backtrace::__rust_begin_short_backtrace::hb0854a261ba462cb
2021-09-26 16:28:54 30: 0x55b0aadd4727 - core::ops::function::FnOnce::call_once{{vtable.shim}}::hb6aa4fe97fbe6416
2021-09-26 16:28:54 31: 0x55b0aae112e7 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h6bff7798948b1075
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/boxed.rs:1572:9
2021-09-26 16:28:54 32: 0x55b0aae112e7 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hc2d25ac38f6b2342
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/boxed.rs:1572:9
2021-09-26 16:28:54 33: 0x55b0aae112e7 - std::sys::unix::thread::Thread::new::thread_start::hbba5bc368baac205
2021-09-26 16:28:54 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/sys/unix/thread.rs:74:17
2021-09-26 16:28:54 34: 0x7ff59fcf0ea7 - start_thread
2021-09-26 16:28:54 35: 0x7ff59fad4def - clone
2021-09-26 16:28:54 36: 0x0 - <unknown>

the stacktrace has nothing from my program so im sorry that i cant tell anything about how im using it.

[jbr]

@orbli what is the code you're using to construct a Response?

[orbli]

I did not construct any response, I'm client side of the http connections. I mainly call from https://docs.rs/surf/2.3.1/surf/ surf::get, surf::post, which seems to be using isahc behind

[jbr]

This seems like a surf bug then. If a server responds with an invalid status, surf should return an Err, not panic.

Cc @Fishrock123