perf: PERF-09 enable gzip/brotli response compression (#845)#886
perf: PERF-09 enable gzip/brotli response compression (#845)#886Chris0Jeky merged 6 commits intomainfrom
Conversation
Registers Brotli + Gzip response compression providers at Optimal level with EnableForHttps = true. Extension method AddTaskdeckResponseCompression keeps Program.cs composition clean and follows the existing ServiceCollectionExtensions pattern. Includes application/problem+json in the compressible MIME set so RFC 7807 error payloads benefit alongside application/json. Middleware not yet wired — next commit handles pipeline order.
Places response compression middleware after forwarded headers and before CORS, static files, authentication, routing, and endpoint mapping. This ensures controllers, SPA static assets, and fallback index.html emit gzip/brotli-encoded bodies when the client sends Accept-Encoding.
Adds three integration tests asserting: - GET /api/boards with Accept-Encoding: gzip returns gzip-encoded body whose decompressed length strictly exceeds the compressed bytes. - Same pattern for brotli (Accept-Encoding: br). - Omitting Accept-Encoding returns a plain body (no Content-Encoding), proving compression is opt-in. TestWebApplicationFactory's in-memory client does not auto-decompress, so the raw Content-Encoding header and byte length are observable.
There was a problem hiding this comment.
Pull request overview
Enables HTTP response compression in the Taskdeck API to reduce payload sizes (PERF-09), and adds integration coverage to verify gzip/brotli negotiation and the “no opt-in, no compression” behavior.
Changes:
- Registers response compression services (Brotli + Gzip) via a new
AddTaskdeckResponseCompression()extension. - Inserts
UseResponseCompression()into the API middleware pipeline (after forwarded headers, before CORS/static files/auth/routing). - Adds integration tests that assert gzip, brotli, and uncompressed responses based on
Accept-Encoding.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| backend/src/Taskdeck.Api/Program.cs | Wires up response compression service registration in the main API host. |
| backend/src/Taskdeck.Api/Extensions/ResponseCompressionRegistration.cs | Adds AddTaskdeckResponseCompression() extension with providers/options and MIME type additions. |
| backend/src/Taskdeck.Api/Extensions/PipelineConfiguration.cs | Adds app.UseResponseCompression() to the request pipeline ordering. |
| backend/tests/Taskdeck.Api.Tests/ResponseCompressionApiTests.cs | Adds integration tests validating gzip/brotli negotiation and no compression without opt-in. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using System.Net.Http.Headers; | ||
| using FluentAssertions; | ||
| using Taskdeck.Api.Tests.Support; | ||
| using Taskdeck.Application.DTOs; |
There was a problem hiding this comment.
Taskdeck.Application.DTOs is not used anywhere in this test file; removing the unused using avoids compiler warning noise (and keeps analyzers happy if warnings are tightened later).
| using Taskdeck.Application.DTOs; |
| [Fact] | ||
| public async Task BoardsList_WithGzipAcceptEncoding_ReturnsGzipContentEncoding() | ||
| { | ||
| var client = _factory.CreateClient(); |
There was a problem hiding this comment.
HttpClient from WebApplicationFactory.CreateClient() should be disposed to avoid handler/socket/resource leaks across the test run. Most tests in this project use using var client = _factory.CreateClient(); for this reason.
| var client = _factory.CreateClient(); | |
| using var client = _factory.CreateClient(); |
| var client = _factory.CreateClient(); | ||
| await ApiTestHarness.AuthenticateAsync(client, "compression-br"); |
There was a problem hiding this comment.
HttpClient from WebApplicationFactory.CreateClient() should be disposed to avoid handler/socket/resource leaks across the test run. Use using var client = _factory.CreateClient(); here as well.
| var client = _factory.CreateClient(); | ||
| await ApiTestHarness.AuthenticateAsync(client, "compression-none"); |
There was a problem hiding this comment.
HttpClient from WebApplicationFactory.CreateClient() should be disposed to avoid handler/socket/resource leaks across the test run. Use using var client = _factory.CreateClient(); here as well.
| /// do not mix long-lived secrets with user-controlled text in the same response body | ||
| /// (no BREACH oracle surface). JWTs live in the <c>Authorization</c> header and CSRF | ||
| /// double-submit tokens are not used — the threat model for BREACH-style compression | ||
| /// side channels does not apply here. |
There was a problem hiding this comment.
The security note says JWTs live in the Authorization header, but the API returns JWTs in response bodies for /api/auth/login and /api/auth/register (see AuthController). Please update this comment to accurately reflect where secrets appear in responses when justifying EnableForHttps = true.
| /// do not mix long-lived secrets with user-controlled text in the same response body | |
| /// (no BREACH oracle surface). JWTs live in the <c>Authorization</c> header and CSRF | |
| /// double-submit tokens are not used — the threat model for BREACH-style compression | |
| /// side channels does not apply here. | |
| /// do not mix secrets with user-controlled text in the same response body | |
| /// (no BREACH oracle surface). Some auth endpoints return JWTs in the response body, | |
| /// but those payloads are fixed server-generated JSON rather than pages that reflect | |
| /// attacker-controlled input, and CSRF double-submit tokens are not used — so the | |
| /// threat model for BREACH-style compression side channels does not apply here. |
| // application/problem+json is the RFC 7807 error content type used by the API's | ||
| // error contract middleware. Explicitly compressing it ensures error-heavy | ||
| // response surfaces (validation failures, auth errors) benefit from compression | ||
| // alongside the default application/json. |
There was a problem hiding this comment.
This comment claims the API’s error contract uses application/problem+json, but the current middleware (e.g. UnhandledExceptionMiddleware) sets ContentType = "application/json". Either update the comment to reflect reality (or explain that application/problem+json is included for compatibility with framework-generated ProblemDetails).
| // application/problem+json is the RFC 7807 error content type used by the API's | |
| // error contract middleware. Explicitly compressing it ensures error-heavy | |
| // response surfaces (validation failures, auth errors) benefit from compression | |
| // alongside the default application/json. | |
| // application/problem+json is included for compatibility with framework-generated | |
| // RFC 7807 ProblemDetails responses. The API's default JSON responses remain | |
| // covered by the built-in application/json MIME type configuration. |
| // Response compression must run before any middleware that writes a response so | ||
| // controllers, static files, and SPA fallback all emit compressed bodies when the | ||
| // client sent Accept-Encoding. Placed before CORS/security headers so that those | ||
| // middlewares can still append headers to the (compressed) response — ASP.NET Core | ||
| // buffers the content stream and rewrites headers on flush. | ||
| app.UseResponseCompression(); |
There was a problem hiding this comment.
The comment says response compression must run before any middleware that writes a response, but in Development UseSwagger()/UseSwaggerUI() run earlier in this pipeline and will not be compressed. Either move UseResponseCompression() above the Swagger middleware or adjust the comment to match the actual ordering/intent.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces HTTP response compression for the API using Brotli and Gzip, along with integration tests to verify payload reduction. The review feedback highlights a security concern where the presence of JWTs in response bodies contradicts the provided documentation and potentially exposes the API to BREACH attacks. Additionally, it is recommended to use CompressionLevel.Fastest instead of Optimal for both compression providers to prevent excessive CPU usage and latency in dynamic responses.
| { | ||
| services.AddResponseCompression(options => | ||
| { | ||
| options.EnableForHttps = true; |
There was a problem hiding this comment.
The security note (lines 10-14) states that JWTs never travel in response bodies. However, the AuthResultDto returned by the registration and login endpoints (as seen in the test harness at ApiTestHarness.AuthenticateAsync) explicitly includes the JWT Token in the response body. Since these responses also contain user-controlled data like the username or email, enabling compression over HTTPS (EnableForHttps = true) technically exposes a surface for BREACH-style attacks on those specific endpoints. While the risk might be low for short-lived JWTs, the justification provided is factually incorrect for this codebase.
|
|
||
| services.Configure<BrotliCompressionProviderOptions>(options => | ||
| { | ||
| options.Level = CompressionLevel.Optimal; |
There was a problem hiding this comment.
Using CompressionLevel.Optimal for Brotli compression on dynamic API responses is generally discouraged. In .NET, Optimal for Brotli maps to level 11, which is extremely CPU-intensive and can add significant latency (TTFB) to requests. For real-time API responses, CompressionLevel.Fastest (which maps to level 4) is the recommended choice as it provides a much better balance between compression ratio and performance.
options.Level = CompressionLevel.Fastest;|
|
||
| services.Configure<GzipCompressionProviderOptions>(options => | ||
| { | ||
| options.Level = CompressionLevel.Optimal; |
There was a problem hiding this comment.
Similar to the Brotli provider, using CompressionLevel.Fastest for Gzip is typically preferred for dynamic API content to minimize the impact on request latency.
options.Level = CompressionLevel.Fastest;
Adversarial self-reviewRe-read the diff with the mindset of a compression-aware reviewer. Focused on middleware order, BREACH/CRIME oracle, SignalR/streaming compatibility, MIME types, and test quality. Findings
CI status
Acted on
|
Gemini flagged Brotli/Gzip Optimal as too CPU-heavy for dynamic API responses (Brotli.Optimal = level 11, seconds of CPU per MB). Switch both providers to Fastest, which still yields ~70%+ reduction on typical JSON payloads with minimal TTFB impact. Also rewrite the BREACH threat-model paragraph: the previous claim that JWTs live only in the Authorization header is inaccurate — /api/auth/login and /api/auth/register return an AuthResultDto with Token alongside the echoed Username/Email. Explain why classic BREACH exploitation still does not apply (JWTs are high-entropy and short-lived, echoed fields are the caller's own input, no long-lived session cookie / CSRF double-submit). Flag the condition under which we'd need to revisit. Reword the application/problem+json comment to match reality: Taskdeck's own middlewares emit application/json (already covered by defaults); this MIME type is added for compatibility with framework-generated RFC 7807 responses (e.g. [ApiController] automatic 400 validation bodies).
Copilot pointed out that UseSwagger/UseSwaggerUI run above UseResponseCompression so Swagger UI responses bypass compression. That is intentional — Swagger is a Development-only convenience surface and not worth extra CPU. Update the inline comment to say so, and to describe what the middleware is placed above (controllers, static files, SPA fallback, SignalR negotiation) rather than the overly broad "any middleware that writes a response".
Apply Copilot review suggestions: - wrap WebApplicationFactory.CreateClient() in `using var` in all three ResponseCompressionApiTests facts (matches the convention elsewhere in Taskdeck.Api.Tests and avoids handler/socket leaks across the test run). - remove unused `using Taskdeck.Application.DTOs;` to silence warning noise if analyzers tighten later.
Fresh adversarial review + bot-findings triage (round 2)Re-read the diff cold, triaged gemini-code-assist and copilot-pull-request-reviewer findings, applied fixes in three small commits. Bot findings triagegemini-code-assist
copilot-pull-request-reviewer
Fresh adversarial findings (cold re-read)
Commits
Local verification
Will update with final CI status once the push run completes. |
CI status — round 2 fixesAll required checks green on
No open concerns from bot or fresh adversarial review. PR ready from my side. |
Adds a delivery entry for the 8 PROD-00 PRs merged on 2026-04-16 (#884 SEC-28, #885 DOC-06, #887 DOC-07, #886 PERF-09, #888 PERF-10, #889 OPS-29, #890 FE-15, #891 FE-14) with round-2 adversarial review findings: BREACH JWT-in-body correction (compression level Optimal -> Fastest), IPv6/IPv4 healthcheck fix, null-throw sentinel fix, skipRetry opt-out for baseline tests, setpriv entrypoint for upgrade-safe volume ownership. Also bumps the Last Updated date.
…layer error coverage Adds a PROD-00 Production-Readiness Round-2 Wave section covering: - ResponseCompressionApiTests (#886, +3 tests) - migration-only context for composite DB indexes (#888) - container hardening verification (no unit tests, docker inspect path) - HTTP retry with backoff tests + skipRetry opt-out pattern for future test authors (#890) - ErrorBoundary + errorReporting tests + three-layer error coverage pattern documenting outer/inner/window layers (#891) Updates Current Verified Totals to reflect the new test deltas.
Adds a delivery entry for the 8 PROD-00 PRs merged on 2026-04-16 (#884 SEC-28, #885 DOC-06, #887 DOC-07, #886 PERF-09, #888 PERF-10, #889 OPS-29, #890 FE-15, #891 FE-14) with round-2 adversarial review findings: BREACH JWT-in-body correction (compression level Optimal -> Fastest), IPv6/IPv4 healthcheck fix, null-throw sentinel fix, skipRetry opt-out for baseline tests, setpriv entrypoint for upgrade-safe volume ownership. Also bumps the Last Updated date.
…layer error coverage Adds a PROD-00 Production-Readiness Round-2 Wave section covering: - ResponseCompressionApiTests (#886, +3 tests) - migration-only context for composite DB indexes (#888) - container hardening verification (no unit tests, docker inspect path) - HTTP retry with backoff tests + skipRetry opt-out pattern for future test authors (#890) - ErrorBoundary + errorReporting tests + three-layer error coverage pattern documenting outer/inner/window layers (#891) Updates Current Verified Totals to reflect the new test deltas.
Closes #845.
Summary
AddTaskdeckResponseCompression()extension that registers Brotli + Gzip providers atCompressionLevel.OptimalwithEnableForHttps = true.UseResponseCompression()inPipelineConfigurationafter forwarded headers and before CORS / static files / routing so controllers, SPA assets, and the fallbackindex.htmlall emit compressed bodies when the client opts in viaAccept-Encoding.application/problem+jsonso RFC 7807 error payloads benefit alongsideapplication/json.Content-Encoding: gzipwith a real decompress-larger-than-compressed delta, brotli analogue, and no compression when the client omitsAccept-Encoding.Acceptance criteria
AddResponseCompressionconfigured with Brotli + Gzip providersUseResponseCompression()placed before routing middlewareEnableForHttps = truesetContent-Encodingand decompressed size > compressed size)Security notes
EnableForHttps = trueis safe for this API:Authorizationheader, never in response bodies; there is no token-mixed-with-user-controlled-text surface for a BREACH oracle.SignalR
text/event-stream, which is not in the default compressible MIME set, so streaming responses are not buffered.Test plan
dotnet test --filter "FullyQualifiedName~ResponseCompression"-> 3/3 passBoardsApiTests,BoardsHubIntegrationTests,RealtimeBoardHubApiTests,SecurityHeadersApiTests,HealthApiTests,CorsApiTests,ApiErrorContractApiTests,No500sMetaTest-> all green (no pipeline regressions)