Skip to content

Develop#490

Merged
jeffersonrodrigues92 merged 3 commits into
mainfrom
develop
May 25, 2026
Merged

Develop#490
jeffersonrodrigues92 merged 3 commits into
mainfrom
develop

Conversation

@jeffersonrodrigues92
Copy link
Copy Markdown
Contributor

No description provided.

brunobls and others added 3 commits April 17, 2026 11:30
…HTTP/2 leak

http.DefaultTransport can be mutated at boot by other libraries (notably
OpenTelemetry registering an h2 handler in TLSNextProto). Cloning it
inherited that handler, so HTTPS connections negotiated HTTP/2 via ALPN
and hit the stdlib hpack encoder issue under the concurrent-goroutine
usage pattern of this client — reproduced in production on 2026-04-10
(61 errors on reporter-manager, 14 on reporter-worker, circuit-breaker
cascade, 503s).

Replace the Clone()-based construction with an explicit *http.Transport.
TLSNextProto is initialized to a non-nil empty map (the stdlib opt-out
signal). Proxy, DialContext, TLS, pool and timeout settings are now set
explicitly, no longer inherited from a mutable global. HTTP/1.1 is the
deliberate choice for this client because middleware and async
revalidation share the same host concurrently.

Adds client_transport_test.go with regression tests that contaminate
http.DefaultTransport with an h2 handler and assert it does not leak
into the client, plus pinning tests for pool and timeout defaults.

Refs: docs/lib-commons/incidents/2026-04-10-http2-protocol-mismatch.md
Generated-by: Claude
AI-Model: claude-opus-4-7
fix: build HTTP transport explicitly to avoid HTTP/2 leak
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fe31e17e-6914-4dfb-8346-e220dc845882

📥 Commits

Reviewing files that changed from the base of the PR and between a797a39 and a39e5d9.

📒 Files selected for processing (2)
  • commons/tenant-manager/client/client.go
  • commons/tenant-manager/client/client_transport_test.go

Walkthrough

The PR refactors HTTP client construction in commons/tenant-manager/client/ to explicitly build an isolated HTTP transport that opts out of HTTP/2 negotiation. The implementation sets TLSNextProto to a non-nil empty map and disables ForceAttemptHTTP2, ensuring HTTP/1.1 only behavior independent of http.DefaultTransport. Three tests validate HTTP1-only behavior, transport isolation, and configuration defaults.

Changes

HTTP/2 Opt-out for Isolated HTTP Client

Layer / File(s) Summary
Explicit HTTP/2 opt-out transport configuration
commons/tenant-manager/client/client.go
crypto/tls import is added. newDefaultHTTPClient() is rewritten to build a fully explicit *http.Transport instead of cloning http.DefaultTransport. The transport explicitly sets TLSNextProto to a non-nil empty map, disables HTTP/2 via ForceAttemptHTTP2: false, configures ProxyFromEnvironment and a net.Dialer, and preserves connection pooling/timeout settings. Returns http.Client with 30s timeout.
Transport isolation and configuration tests
commons/tenant-manager/client/client_transport_test.go
Three tests validate the new implementation: TestNewDefaultHTTPClient_UsesHTTP1Only verifies HTTP/2 stays disabled even when http.DefaultTransport.TLSNextProto is contaminated with an "h2" handler; TestNewDefaultHTTPClient_TransportIsIsolated asserts the created transport does not share pointer identity with http.DefaultTransport; TestNewDefaultHTTPClient_ExpectedDefaults pins client timeout and transport settings (proxy, dial context, idle/TLS/expect-continue timeouts, pool sizes) to expected defaults.

Comment @coderabbitai help to get the list of available commands and usage tips.

@jeffersonrodrigues92 jeffersonrodrigues92 merged commit 882329f into main May 25, 2026
10 of 12 checks passed
@lerian-studio
Copy link
Copy Markdown
Contributor

🔍 PR Validation Summary

🚫 PR Blocked — 2 blocking failures

Check Status Blocking
Source Branch ✅ success yes
PR Title ❌ failure yes
PR Description ❌ failure yes
PR Size ⏭️ skipped no
Auto Labels ⏭️ skipped no
PR Metadata ⏭️ skipped no

Fix the blocking checks above before merge.


🔍 View workflow run

@lerian-studio
Copy link
Copy Markdown
Contributor

🔒 Security Scan Results — lib-commons

✅ PR Mergeable — no blocking findings

Stage Status Blocking?
Filesystem Scan ✅ Clean
Docker Image Scan ➖ Skipped
Docker Hub Health Score ➖ Skipped
Pre-release Version Check ✅ Clean

Trivy

Filesystem Scan

✅ No vulnerabilities or secrets found.


Pre-release Version Check

✅ No unstable version pins found.


🔍 View full scan logs

@lerian-studio
Copy link
Copy Markdown
Contributor

📊 Unit Test Coverage Report: app

Metric Value
Overall Coverage 87.0% ✅ PASS
Threshold 80%

Coverage by Package

Package Coverage
github.com/LerianStudio/lib-commons/v5/commons/backoff 91.1%
github.com/LerianStudio/lib-commons/v5/commons/certificate 88.8%
github.com/LerianStudio/lib-commons/v5/commons/circuitbreaker 86.8%
github.com/LerianStudio/lib-commons/v5/commons/cron 94.2%
github.com/LerianStudio/lib-commons/v5/commons/crypto 95.6%
github.com/LerianStudio/lib-commons/v5/commons/dlq 81.0%
github.com/LerianStudio/lib-commons/v5/commons/errgroup 86.1%
github.com/LerianStudio/lib-commons/v5/commons/internal/nilcheck 100.0%
github.com/LerianStudio/lib-commons/v5/commons/jwt 89.4%
github.com/LerianStudio/lib-commons/v5/commons/license 96.9%
github.com/LerianStudio/lib-commons/v5/commons/mongo 89.0%
github.com/LerianStudio/lib-commons/v5/commons/net/http/idempotency 93.0%
github.com/LerianStudio/lib-commons/v5/commons/net/http/ratelimit 90.8%
github.com/LerianStudio/lib-commons/v5/commons/net/http 96.1%
github.com/LerianStudio/lib-commons/v5/commons/outbox 91.8%
github.com/LerianStudio/lib-commons/v5/commons/pointers 100.0%
github.com/LerianStudio/lib-commons/v5/commons/postgres 84.6%
github.com/LerianStudio/lib-commons/v5/commons/rabbitmq 89.3%
github.com/LerianStudio/lib-commons/v5/commons/redis 89.5%
github.com/LerianStudio/lib-commons/v5/commons/safe 99.6%
github.com/LerianStudio/lib-commons/v5/commons/secretsmanager 98.7%
github.com/LerianStudio/lib-commons/v5/commons/security/ssrf 95.9%
github.com/LerianStudio/lib-commons/v5/commons/security 100.0%
github.com/LerianStudio/lib-commons/v5/commons/server 87.3%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/cache 97.9%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/client 93.5%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/consumer 87.9%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/core 99.0%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/event 95.7%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/internal/eviction 100.0%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/log 100.0%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/middleware 92.3%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/mongo 76.3%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/postgres 86.8%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/rabbitmq 82.6%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/redis 93.8%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/s3 96.3%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/tenantcache 98.4%
github.com/LerianStudio/lib-commons/v5/commons/tenant-manager/valkey 100.0%
github.com/LerianStudio/lib-commons/v5/commons/transaction 95.1%
github.com/LerianStudio/lib-commons/v5/commons/webhook 91.5%
github.com/LerianStudio/lib-commons/v5/commons 96.4%

Generated by Go PR Analysis workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants