client/client_test.go (2)

385-389: Replace fixed sleeps with readiness polling (TLS servers)

Fixed time.Sleep(1 * time.Second) is flaky. Poll the TLS listener until it accepts connections.

// Replace time.Sleep with a small polling loop:
addr := ln.Addr().String()
deadline := time.Now().Add(3 * time.Second)
for {
    conn, err := tls.Dial("tcp", addr, clientTLSConf)
    if err == nil {
        _ = conn.Close()
        break
    }
    if time.Now().After(deadline) {
        t.Fatalf("server not ready within deadline: %v", err)
    }
    time.Sleep(50 * time.Millisecond)
}

Also applies to: 551-555

---

1849-1849: TLS config identity assertions are correct

Using require.Same avoids deep-equality pitfalls on *tls.Config and validates the returned instance is the active one.

Also applies to: 1884-1884, 565-565

client/transport.go (3)

299-303: Name magic numbers in control-char check (optionally expand to C1 controls)

Use named constants for 0x20 and 0x7f for clarity. Optionally reject C1 controls (0x80–0x9f).

Apply this diff:

@@
-// defaultRedirectLimit mirrors fasthttp's default when callers supply a negative redirect cap.
+// defaultRedirectLimit mirrors fasthttp's default when callers supply a negative redirect cap.
 const defaultRedirectLimit = 16
+
+// ASCII control ranges used for URI validation.
+const (
+	minPrintableASCII = 0x20
+	delASCII          = 0x7f
+)
@@
-	for _, b := range location {
-		if b < 0x20 || b == 0x7f {
+	for _, b := range location {
+		if b < minPrintableASCII || b == delASCII {
 			return "", fasthttp.ErrorInvalidURI
 		}
 	}

Optionally:

-		if b < minPrintableASCII || b == delASCII {
+		if b < minPrintableASCII || b == delASCII || (b >= 0x80 && b <= 0x9f) {

---

181-188: Recursive traversal of nested LBClient/HostClient is correct

walkBalancingClient delegates to walkBalancingClientWithBreak and the latter correctly recurses through LBClient and wrapper types exposing LBClient(). This addresses prior review feedback.

Also applies to: 209-230

---

246-267: Redirect behavior for maxRedirects <= 0 aligns with fasthttp

singleRequestOnly path returns the initial response (which may be a redirect) without error, matching fasthttp.Client/HostClient semantics.

client/transport_test.go (2)

339-342: Nit: unused loop index

Use _ since the index isn’t used.

-	for i := 0; i < defaultRedirectLimit+1; i++ {
+	for _ := 0; _ < defaultRedirectLimit+1; _++ {

---

266-284: Good: 303 See Other behavior covered

Tests assert POST→GET, body cleared, and content headers removed for 303. This prevents regressions.

docs/whats_new.md (1)

777-782: Clarify usage and cross-link examples for HostClient/LBClient

Add a note on HostClient.Addr formatting and link to the examples section to reduce misconfiguration.

Apply this doc tweak:

 ### Fasthttp transport integration

 - `client.NewWithHostClient` and `client.NewWithLBClient` allow you to plug existing `fasthttp` clients directly into Fiber while keeping retries, redirects, and hook logic consistent.
 - Dialer, TLS, and proxy helpers now update every host client inside a load balancer, so complex pools inherit the same configuration.
 - The Fiber client exposes `Do`, `DoTimeout`, `DoDeadline`, and `CloseIdleConnections`, matching the surface area of the wrapped fasthttp transports.
+ - When using `fasthttp.HostClient`, remember that `HostClient.Addr` must be `host:port` (no scheme), and HTTPS endpoints require `IsTLS=true`. See examples in [Reusing fasthttp transports](./client/examples.md#reusing-fasthttp-transports).

AGENTS.md (1)

50-52: Include linting in required programmatic checks

Per repo guidelines, golangci-lint is enforced via make lint. Add it to the must-run list.

 Before presenting final changes or submitting a pull request, run each of the
 following commands and ensure they succeed. Include the command outputs in your
 final response to confirm they were executed:
 ```bash
 make audit
+make lint
 make generate
 make betteralign
 make modernize
 make format
 make test

</blockquote></details>
<details>
<summary>docs/client/examples.md (2)</summary><blockquote>

`162-190`: **Prevent response leaks and clarify HostClient.Addr**

- Add `defer resp.Close()` to release pooled buffers.
- Note that `HostClient.Addr` must be `host:port` (no scheme); set `IsTLS=true` for HTTPS.



```diff
 func main() {
     hc := &fasthttp.HostClient{
-        Addr:              "api.internal:443",
+        Addr:              "api.internal:443", // host:port, no scheme
         IsTLS:             true,
         MaxConnDuration:   30 * time.Second,
         MaxIdleConnDuration: 10 * time.Second,
     }

     cc := client.NewWithHostClient(hc)

-    resp, err := cc.Get("https://api.internal:443/status")
+    resp, err := cc.Get("https://api.internal:443/status")
     if err != nil {
         log.Fatal(err)
     }
+    defer resp.Close()

     log.Printf("status=%d body=%s", resp.StatusCode(), resp.Body())
 }

---

205-225: Add response close in LB example

Release response resources to avoid test/doc copy-paste leaks.

     resp, err := cc.Get("http://service.internal/api")
     if err != nil {
         log.Fatal(err)
     }
+    defer resp.Close()

     log.Printf("status=%d body=%s", resp.StatusCode(), resp.Body())

client/client_test.go (1)

1083-1084: Remove unnecessary sleeps after startTestServerWithPort

startTestServerWithPort already signals readiness via ListenerAddrFunc. These sleeps slow tests and add flakiness; they can be removed safely.

- time.Sleep(1 * time.Second)

Also applies to: 1192-1193

client/transport.go (1)

150-156: Simplify TLSConfig: redundant empty check

extractTLSConfig already returns nil for empty slices; the len(...) == 0 guard is unnecessary.

-func (l *lbClientTransport) TLSConfig() *tls.Config {
-	if len(l.client.Clients) == 0 {
-		return nil
-	}
-	return extractTLSConfig(l.client.Clients)
-}
+func (l *lbClientTransport) TLSConfig() *tls.Config {
+	return extractTLSConfig(l.client.Clients)
+}

client/transport_test.go (1)

253-326: Add test for cross-host auth stripping if you adopt the redirect security change

If you implement dropping Authorization on cross-host redirect, add a case asserting the header is removed when Location points to a different host.

t.Run("drop auth on cross-host redirect", func(t *testing.T) {
  req.Reset(); resp.Reset()
  req.SetRequestURI("http://example.com/start")
  req.Header.SetMethod(fasthttp.MethodGet)
  req.Header.Set(fasthttp.HeaderAuthorization, "Bearer secret")
  client := &stubRedirectClient{calls: []stubRedirectCall{
    {status: ptrInt(fasthttp.StatusFound), location: ptrString("http://other.com/next")},
    {status: ptrInt(fasthttp.StatusOK)},
  }}
  require.NoError(t, doRedirectsWithClient(req, resp, 2, client))
  require.Empty(t, req.Header.Peek(fasthttp.HeaderAuthorization))
  require.Equal(t, "http://other.com/next", req.URI().String())
})

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: gofiber/fiber#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T07:16:43.444Z
Learning: Before submitting a PR with generated changes, run and pass: `make audit`, `make generate`, `make betteralign`, `make modernize`, `make format`, and `make test`

Learnt from: efectn
PR: gofiber/fiber#3162
File: app_test.go:893-895
Timestamp: 2024-11-29T12:37:27.581Z
Learning: In the `Test_App_ShutdownWithContext` function in `app_test.go`, the `clientDone` channel is used to synchronize the client's request completion before proceeding, eliminating the need for additional `time.Sleep` calls.

Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-09-25T17:05:06.991Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

client/client.go (1)

106-128: Document nil return behavior for clarity.

The accessor methods correctly return the underlying client when the transport type matches, but the nil return case when types don't match could be better documented for API clarity.

Consider adding documentation like:

 // FasthttpClient returns the underlying *fasthttp.Client if the client was created with one.
+// Returns nil if the client was created with NewWithHostClient or NewWithLBClient.
 func (c *Client) FasthttpClient() *fasthttp.Client {

Apply similar documentation to HostClient and LBClient.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

client/client.go (2)

309-316: Consider documenting the error return or making it infallible.

SetProxyURL always returns nil, suggesting the error return is unused. If errors are not possible with the current implementation, consider either:

1. Documenting why the error return exists (e.g., for future extensibility or API compatibility), or
2. Changing the signature to remove the error return if it's not needed.

For reference, if the error return is truly unused and won't be needed in the future, you could simplify to:

-func (c *Client) SetProxyURL(proxyURL string) error {
+func (c *Client) SetProxyURL(proxyURL string) *Client {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
 	c.applyDial(fasthttpproxy.FasthttpHTTPDialer(proxyURL))
-	return nil
+	return c
 }

This would also make it consistent with other setter methods that return *Client for chaining.

---

738-744: Consider clarifying transport type in documentation.

The documentation could be slightly more explicit by mentioning that this creates a client with a "standard client transport" to align with internal naming conventions and help users understand which transport type is being used.

Based on learnings from past review comments.

Example enhancement:

-// NewWithClient creates and returns a new Client object from an existing fasthttp.Client.
+// NewWithClient creates and returns a new Client object from an existing fasthttp.Client,
+// wrapping it in a standard client transport for general-purpose HTTP requests.
 func NewWithClient(c *fasthttp.Client) *Client {

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro