Skip to content

Claude Code Review Report #3055

Description

@qarmin

Recently, I ran Claude Code across several projects(like image-rs, lofty of symphonia) to look for potential issues.

In this projects, a significant portion of reported findings turned out to be incorrect or minor false positives, but a non-trivial subset was valid and included real issues ranging from incorrect comments to actual logic bugs. As a result, the findings generally require manual validation to separate noise from actionable problems.

Full Report - reqwest_20260614.html

Example findings (this is only a subset of the findings, specifically those most likely to be actual bugs; for the full list, see the report above):

LOGIC_2 HIGH

Description: In connect_socks, the rustls HTTPS-over-socks branch hardcodes tls_info: false (line 587), while the native-tls branch correctly uses self.tls_info (line 561). Users requesting TLS info on rustls + SOCKS connections never receive it. Inconsistent behavior between the two TLS backends for the same feature.

Locations:

src/connect.rs:583-588
  583 |                     return Ok(Conn {
  584 |                         inner: self.verbose.wrap(RustlsTlsConn { inner: io }),
  585 |                         is_proxy: false,
  586 |                         tls_info: false,
  587 |                     });
  588 |                 }
src/connect.rs:558-562
  558 |                     return Ok(Conn {
  559 |                         inner: self.verbose.wrap(NativeTlsConn { inner: io }),
  560 |                         is_proxy: false,
  561 |                         tls_info: self.tls_info,
  562 |                     });

Fix: Use tls_info: self.tls_info in the rustls socks branch to match the native-tls branch.

ARCH_1 MEDIUM

Description: client.rs is a 3173-line file mixing many distinct responsibilities: the entire TLS backend construction (native-tls + rustls + h3, ~lines 521-896), HTTP/1 and HTTP/2 builder option plumbing, the connection-pool/connector assembly, the full ClientBuilder fluent API, the Pending/PendingRequest future state machine, proxy header injection, and Debug impls. The build() method alone is ~700 lines. This hurts readability, compile times, and makes feature-cfg interactions (see the native-tls ALPN bug) easy to get wrong.

Fix: Split into submodules: e.g. client/tls_build.rs for the TLS/connector construction, client/builder.rs for the fluent setters, and client/pending.rs for the Pending/PendingRequest/ResponseFuture future machinery. Break build() into helper functions per concern (resolver, connector, hyper builder, layered service).

CPY_1 HIGH

Description: Request::try_clone in the blocking module does NOT copy the request's extensions, while the async equivalent (src/async_impl/request.rs:145-156) does (*req.extensions_mut() = self.extensions().clone();). Extensions can carry per-request config such as the request timeout/total-timeout and other state; for the blocking client this means retries/redirects that rely on cloning a Request silently lose all extensions, diverging from async behavior. The blocking Request already exposes extensions indirectly via into_async/TryFrom<http::Request> (which sets them at request.rs:676), so dropping them on clone is a real data-loss bug introduced by the blocking copy not being kept in sync.

Locations:

src/blocking/request.rs:122-138
  122 |     pub fn try_clone(&self) -> Option<Request> {
  123 |         let body = if let Some(ref body) = self.body.as_ref() {
  124 |             if let Some(body) = body.try_clone() {
  125 |                 Some(body)
  126 |             } else {
  127 |                 return None;
  128 |             }
  129 |         } else {
  130 |             None
  131 |         };
  132 |         let mut req = Request::new(self.method().clone(), self.url().clone());
  133 |         *req.timeout_mut() = self.timeout().copied();
  134 |         *req.headers_mut() = self.headers().clone();
  135 |         *req.version_mut() = self.version().clone();
  136 |         req.body = body;
  137 |         Some(req)
  138 |     }

Fix: Add *req.extensions_mut() = self.inner.extensions().clone(); (exposing an extensions accessor on the blocking Request if needed) in try_clone, mirroring async_impl::request::Request::try_clone.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions