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

HTTP2 CONNECT request with body creates undescriptive INTERNAL_ERROR error #3858

Open
manuel2258 opened this issue Mar 20, 2025 · 14 comments
Open
Labels
A-client Area: client. A-http2 Area: HTTP/2 specific. C-refactor Category: refactor. This would improve the clarity of internal code. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@manuel2258
Copy link

Version

hyper: v1.6.0
hyper-tls: v0.6.0
hyper-util: v0.1.10
h2: v0.4.8
reqwest: v0.12.15

Platform

Linux workfedora 6.13.6-100.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Mar 7 21:23:12 UTC 2025 x86_64 GNU/Linux

Description

Hey, we recently stumbled across a weird undescriptive INTERNAL_ERROR hyper error when using reqwest.

We are writing a pentesting tool, and as a part of that we wanted to test whether a HTTP2 endpoint rejects invalid methods. As a part of that, we sent a CONNECT request, with a body.
That resulted in the error: hyper_util::client::legacy::Error(SendRequest, hyper::Error(Http2, Error { kind: Reason(INTERNAL_ERROR) })) }

It can be reproduced with:

use reqwest::Method;
use tracing::Level;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
    tracing_subscriber::fmt()
        .with_max_level(Level::TRACE)
        .init();

    let client = reqwest::Client::builder()
        .http2_prior_knowledge()
        .build()
        .unwrap();

    let res = client
        .request(Method::CONNECT, "https://nghttp2.org/httpbin/post")
        .body("example body")
        .send()
        .await;
    println!("{res:?}");

    Ok(())
}

Doing a bit of investigation with the backtrace crate, I gathered that the error originates from h2/client.rs.

Two weird things about that:

  1. We did not see the tracing warn message, so it was really hard to figure out what is going wrong. Not sure whether this is a configuration error on our side, but we see all the other tracing events from hyper.

Log from example:

2025-03-20T08:53:10.774367Z DEBUG reqwest::connect: starting new connection: https://nghttp2.org/
2025-03-20T08:53:10.775902Z DEBUG hyper_util::client::legacy::connect::http: connecting to [2400:8902::f03c:91ff:fe69:a454]:443
2025-03-20T08:53:11.077096Z DEBUG hyper_util::client::legacy::connect::http: connecting to 139.162.123.134:443
2025-03-20T08:53:11.344664Z DEBUG hyper_util::client::legacy::connect::http: connected to 139.162.123.134:443
2025-03-20T08:53:11.702388Z DEBUG h2::client: binding client connection
2025-03-20T08:53:11.702435Z DEBUG h2::client: client connection bound
2025-03-20T08:53:11.702457Z DEBUG h2::codec::framed_write: send frame=Settings { flags: (0x0), enable_push: 0, initial_window_size: 2097152, max_frame_size: 16384, max_header_list_size: 16384 }
2025-03-20T08:53:11.702595Z DEBUG hyper_util::client::legacy::pool: pooling idle connection for ("https", nghttp2.org)
2025-03-20T08:53:11.702641Z  WARN hyper_util::client::legacy::client: HTTP/1.1 CONNECT request stripping path: /httpbin/post
Err(reqwest::Error { kind: Request, url: "https://nghttp2.org/httpbin/post", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(Http2, Error { kind: Reason(INTERNAL_ERROR) })) })
2025-03-20T08:53:11.702817Z DEBUG Connection{peer=Client}: h2::codec::framed_write: send frame=WindowUpdate { stream_id: StreamId(0), size_increment: 5177345 }
  1. Somehow, this path is only triggered when using HTTP2 with TLS. Using HTTP2 without TLS does not trigger the error. I know that that is then rather on how reqwest is using hyper, but still, maybe there could be a better place to check on whether the body is empty?

Not sure whether there is anything to fix / change here, we primarly wanted to report this.

@manuel2258 manuel2258 added the C-bug Category: bug. Something is wrong. This is bad! label Mar 20, 2025
@0x676e67
Copy link
Contributor

Let's see if this PR has been repaired? Https://github.com/hyperium/hyper-util/pull/165

@manuel2258
Copy link
Author

Let's see if this PR has been repaired? Https://github.com/hyperium/hyper-util/pull/165

Running the provided example with:

[patch.crates-io]
hyper-util = { git = "https://github.com/hyperium/hyper-util.git", rev = "d51318df3461d40e5f5e5ca163cb3905ac960209" }

Does indeed change the flow, but results in the very same Error:

2025-03-20T10:45:52.856875Z DEBUG reqwest::connect: starting new connection: https://nghttp2.org/
2025-03-20T10:45:52.932425Z DEBUG hyper_util::client::legacy::connect::http: connecting to [2400:8902::f03c:91ff:fe69:a454]:443
2025-03-20T10:45:53.233395Z DEBUG hyper_util::client::legacy::connect::http: connecting to 139.162.123.134:443
2025-03-20T10:45:53.584046Z DEBUG hyper_util::client::legacy::connect::http: connected to 139.162.123.134:443
2025-03-20T10:45:53.864227Z DEBUG h2::client: binding client connection
2025-03-20T10:45:53.864274Z DEBUG h2::client: client connection bound
2025-03-20T10:45:53.864296Z DEBUG h2::codec::framed_write: send frame=Settings { flags: (0x0), enable_push: 0, initial_window_size: 2097152, max_frame_size: 16384, max_header_list_size: 16384 }
2025-03-20T10:45:53.864417Z DEBUG hyper_util::client::legacy::pool: pooling idle connection for ("https", nghttp2.org)
Err(reqwest::Error { kind: Request, url: "https://nghttp2.org/httpbin/post", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(Http2, Error { kind: Reason(INTERNAL_ERROR) })) })
2025-03-20T10:45:53.864616Z DEBUG Connection{peer=Client}: h2::codec::framed_write: send frame=WindowUpdate { stream_id: StreamId(0), size_increment: 5177345 }

@0x676e67
Copy link
Contributor

0x676e67 commented Mar 20, 2025

This requires in-depth debug debugging to know the reason. In addition, it is effective for websocket upgrades or other protocol upgrades. You should also need to set up http2 extension protocol.

https://github.com/hyperium/hyper-util/blob/master/tests/legacy_client.rs#L873C27-L873C31

And reqwest does not support the extension protocol of setting http2 for the time being. You need to fix it to test it.

@seanmonstar
Copy link
Member

Are you sure hyper is detecting the error? That message, INTERNAL_ERROR, comes from the HTTP/2 error code that is sent in a RST_STREAM. So if the server got the request, and then sent that back, it doesn't contain a way to display any other information.

Most errors that h2 would detect prior to sending would be in its internal UserError variant, and the message would be more descriptive.

@manuel2258
Copy link
Author

Yes, pretty sure I would say. I included a backtrace print in the Error::new function, and patched the custom hyper build into the above example. Following logs are printed when executing:

2025-03-20T12:25:51.534644Z DEBUG reqwest::connect: starting new connection: https://nghttp2.org/
2025-03-20T12:25:51.804110Z DEBUG hyper_util::client::legacy::connect::http: connecting to [2400:8902::f03c:91ff:fe69:a454]:443
2025-03-20T12:25:52.106479Z DEBUG hyper_util::client::legacy::connect::http: connecting to 139.162.123.134:443
2025-03-20T12:25:52.359310Z DEBUG hyper_util::client::legacy::connect::http: connected to 139.162.123.134:443
2025-03-20T12:25:52.678464Z DEBUG h2::client: binding client connection
2025-03-20T12:25:52.678527Z DEBUG h2::client: client connection bound
2025-03-20T12:25:52.678555Z DEBUG h2::codec::framed_write: send frame=Settings { flags: (0x0), enable_push: 0, initial_window_si
2025-03-20T12:25:52.678782Z DEBUG hyper_util::client::legacy::pool: pooling idle connection for ("https", nghttp2.org)
2025-03-20T12:25:52.678864Z  WARN hyper_util::client::legacy::client: HTTP/1.1 CONNECT request stripping path: /httpbin/post
2025-03-20T12:25:52.678887Z DEBUG Connection{peer=Client}: h2::codec::framed_write: send frame=WindowUpdate { stream_id: StreamI
NEW HYPER ERROR CREATED: Http2
   0: hyper::error::Error::new
             at /home/manuel/projects/hyper/src/error.rs:252:25
   1: hyper::error::Error::new_h2
             at /home/manuel/projects/hyper/src/error.rs:421:13
   2: <hyper::proto::h2::client::ClientTask<B,E,T> as core::future::future::Future>::poll
             at /home/manuel/projects/hyper/src/proto/h2/client.rs:678:36
   3: <hyper::client::conn::http2::Connection<T,B,E> as core::future::future::Future>::poll
             at /home/manuel/projects/hyper/src/client/conn/http2.rs:245:22
   4: <F as futures_core::future::TryFuture>::try_poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-core-0.3.31/src/future.rs:92:9
   5: <futures_util::future::try_future::into_future::IntoFuture<Fut> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/future/try_future/into
   6: <futures_util::future::future::map::Map<Fut,F> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/future/future/map.rs:5
   7: <futures_util::future::future::Map<Fut,F> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/lib.rs:86:13
   8: <futures_util::future::try_future::MapErr<Fut,F> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/lib.rs:86:13
   9: <futures_util::future::future::map::Map<Fut,F> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/future/future/map.rs:5
  10: <futures_util::future::future::Map<Fut,F> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/futures-util-0.3.31/src/lib.rs:86:13
  11: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/fut
  12: tokio::runtime::task::core::Core<T,S>::poll::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/core.rs:331:17
  13: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/loom/std/unsafe_cell.rs:16:9
  14: tokio::runtime::task::core::Core<T,S>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/core.rs:320:13
  15: tokio::runtime::task::harness::poll_future::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:532:1
  16: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwi
  17: std::panicking::try::do_call
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.r
  18: __rust_try
  19: std::panicking::try
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.r
  20: std::panic::catch_unwind
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:35
  21: tokio::runtime::task::harness::poll_future
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:520:1
  22: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:209:2
  23: tokio::runtime::task::harness::Harness<T,S>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:154:1
  24: tokio::runtime::task::raw::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/raw.rs:271:5
  25: tokio::runtime::task::raw::RawTask::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/raw.rs:201:18
  26: tokio::runtime::task::LocalNotified<S>::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/mod.rs:463:9
  27: tokio::runtime::scheduler::multi_thread::worker::Context::run_task::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  28: tokio::task::coop::with_budget
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/task/coop/mod.rs:167:5
  29: tokio::task::coop::budget
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/task/coop/mod.rs:133:5
  30: tokio::runtime::scheduler::multi_thread::worker::Context::run_task
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  31: tokio::runtime::scheduler::multi_thread::worker::Context::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  32: tokio::runtime::scheduler::multi_thread::worker::run::{{closure}}::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  33: tokio::runtime::context::scoped::Scoped<T>::set
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/context/scoped.rs:40:
  34: tokio::runtime::context::set_scheduler::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/context.rs:180:26
  35: std::thread::local::LocalKey<T>::try_with
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/loca
  36: std::thread::local::LocalKey<T>::with
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/loca
  37: tokio::runtime::context::set_scheduler
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/context.rs:180:9
  38: tokio::runtime::scheduler::multi_thread::worker::run::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  39: tokio::runtime::context::runtime::enter_runtime
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/context/runtime.rs:65
  40: tokio::runtime::scheduler::multi_thread::worker::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  41: tokio::runtime::scheduler::multi_thread::worker::Launch::launch::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/scheduler/multi_threa
  42: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/blocking/task.rs:42:2
  43: tokio::runtime::task::core::Core<T,S>::poll::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/core.rs:331:17
  44: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/loom/std/unsafe_cell.rs:16:9
  45: tokio::runtime::task::core::Core<T,S>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/core.rs:320:13
  46: tokio::runtime::task::harness::poll_future::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:532:1
  47: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwi
  48: std::panicking::try::do_call
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.r
  49: __rust_try
  50: std::panicking::try
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:547:19
  51: std::panic::catch_unwind
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:358:14
  52: tokio::runtime::task::harness::poll_future
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:520:18
  53: tokio::runtime::task::harness::Harness<T,S>::poll_inner
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:209:27
  54: tokio::runtime::task::harness::Harness<T,S>::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/harness.rs:154:15
  55: tokio::runtime::task::raw::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/raw.rs:271:5
  56: tokio::runtime::task::raw::RawTask::poll
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/raw.rs:201:18
  57: tokio::runtime::task::UnownedTask<S>::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/task/mod.rs:500:9
  58: tokio::runtime::blocking::pool::Task::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/blocking/pool.rs:161:9
  59: tokio::runtime::blocking::pool::Inner::run
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/blocking/pool.rs:511:17
  60: tokio::runtime::blocking::pool::Spawner::spawn_thread::{{closure}}
             at /home/manuel/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.1/src/runtime/blocking/pool.rs:469:13
  61: std::sys::backtrace::__rust_begin_short_backtrace
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/backtrace.rs:152:18
  62: std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:564:17
  63: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:272:9
  64: std::panicking::try::do_call
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:584:40
  65: __rust_try
  66: std::panicking::try
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:547:19
  67: std::panic::catch_unwind
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:358:14
  68: std::thread::Builder::spawn_unchecked_::{{closure}}
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:562:30
  69: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /home/manuel/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
  70: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/alloc/src/boxed.rs:1993:9
  71: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/alloc/src/boxed.rs:1993:9
  72: std::sys::pal::unix::thread::Thread::new::thread_start
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/sys/pal/unix/thread.rs:106:17
  73: start_thread
  74: clone3

Err(reqwest::Error { kind: Request, url: "https://nghttp2.org/httpbin/post", source: hyper_util::client::legacy::Error(SendRequest, hyper::Error(Http2, Error { kind: Reason(INTERNAL_ERROR) })) })

So originating from the line https://github.com/hyperium/hyper/blob/master/src/proto/h2/client.rs#L678.

And that is exactly the weird thing, that that condition is checked at such a weird place and resulting in an INTERNAL_ERROR and not some kind of User Error.

@0x676e67
Copy link
Contributor

0x676e67 commented Mar 20, 2025

warn!("h2 connect request with non-zero body not supported");

warn!("h2 connect request with non-zero body not supported");

Didn't you send a body above?

@seanmonstar
Copy link
Member

Oh I see, interesting. Yea we could improve that error message for sure. (As for why the warn isn't emitted, hyper's tracing is unstable, and requires opting into it.)

@seanmonstar seanmonstar added A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor. A-http2 Area: HTTP/2 specific. C-refactor Category: refactor. This would improve the clarity of internal code. and removed C-bug Category: bug. Something is wrong. This is bad! labels Mar 20, 2025
@manuel2258
Copy link
Author

Ah nice, then everything makes perfect sense, thank you two for clarifying!

As I see that you tagged this as easy, I might want to take a swing at fixing this later after work.
Would you mind guiding me a bit towards what you would fix here? Only changing the kind of error, or maybe moving that check somewhere else?
I'm not sure how we configured reqwest when we first ran into this issue, but we somehow force HTTP2 without TLS, which will not hit this part of the code and send out the request without issues.

@0x676e67
Copy link
Contributor

Maybe we should add detailed error messages to the errors

message: None,

@seanmonstar
Copy link
Member

The action here I believe is to return a better error message. So, not necessarily force this to be a Error::new_h2(), but probably add a new variant in the error module on the internal enum User, which provides a more descriptive explanation.

Right? The error is correct, the check should happen, it just leaves you confused?

@manuel2258
Copy link
Author

Yes, the error is correct, and the check should happen.

I'm just confused on why the same request without TLS did not cause an error, which makes me wonder whether this is the right place to check this. But then I have no idea on the internal architecture of hyper, and also it's direct usage without a high level library such as reqwest.
I could most likely try to reproduce and trace the execution path of the non TLS invocation in our application if that would be further helpful?

If not, I can just create a PR to add a more descriptive error to https://github.com/hyperium/hyper/blob/master/src/proto/h2/client.rs#L678.

@seanmonstar
Copy link
Member

I would find it surprising if it skipped this check, I don't see anything in the surrounding code that would do so. Perhaps it's possible that when you force HTTP/2 without TLS, the server rejects that during the connection handshake, and so hyper never gets to the point of processing the request?

But yes, the steps would be:

  1. Add a new variant to hyper::error::User (it's a crate-private enum), with a better message in the fmt matches.
  2. Change that line you linked to be Error::new_user(User::WhateverItWasNamed).

It probably doesn't need to be a warn either... There's no need to return an error and log an error. It could be reduced to a debug!, I think.

@manuel2258
Copy link
Author

I tried again with our application, which forces HTTP/2 without TLS, and the request goes through without the INTERNAL_ERROR, and we receive a response from the server.
I can tomorrow try to reproduce that setup in a scoped down example.

@manuel2258
Copy link
Author

I found the issue. Checking a PCAP file of the request, it seems like HTTP2 was actually not enabled, and the requests were done with HTTP1.1. Changing that, I can also reproduce the INTERNAL_ERROR without TLS.

So yep, everything is fine, and we only have to improve the error kind and message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: client. A-http2 Area: HTTP/2 specific. C-refactor Category: refactor. This would improve the clarity of internal code. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Development

No branches or pull requests

3 participants