Skip to content

fix(server): preserve asset content types when compression is accepted - #10948

Open
Gigioxx wants to merge 1 commit into
pingdotgg:mainfrom
Gigioxx:t3code/fix-issue-10935
Open

fix(server): preserve asset content types when compression is accepted#10948
Gigioxx wants to merge 1 commit into
pingdotgg:mainfrom
Gigioxx:t3code/fix-issue-10935

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Browser compression strips Content-Type from file assets in Effect rc.112, so HTML previews show source and sibling stylesheets fail to load.

Add no-transform to asset Cache-Control headers, including the guarded-video override, as recommended in the issue triage. This bypasses the broken compression path while preserving caching and sandbox headers. The upstream fix is tracked in Effect-TS/effect#8146.

Reproduced on a real Node HTTP test server: an HTML response keeps its type with identity encoding and loses it when browser compression is accepted. Four regression cases now verify HTML, CSS, JavaScript, and SVG response types, bodies, and security headers with both encodings. All 25 HTTP tests, server typecheck, and targeted lint pass.

Fixes #10935.

Implemented with GPT-6 in Codex.

Summary by CodeRabbit

  • Bug Fixes
    • Asset responses now explicitly prevent intermediary transformations, helping preserve original file content and formats.
    • Video responses hosted in place are no longer cached, improving reliability when serving media files.
    • Asset delivery now preserves correct content types for HTML, CSS, JavaScript, and SVG files across supported encoding requests.
    • Security-related response headers remain consistent, including content sniffing protection and sandbox policies for applicable asset types.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 9, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 2f188c0

Macroscope's review found this PR approvable — This is a narrow server bug fix that prevents broken compression from stripping asset MIME types and preserves existing security headers. Its only runtime tradeoff is disabling transformation/compression for asset responses, with targeted HTTP regression coverage and no broader API, schema, or infrastructure impact.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d27b491b-b892-4171-a7ba-939469ce6fd3

📥 Commits

Reviewing files that changed from the base of the PR and between 6c58362 and 2f188c0.

📒 Files selected for processing (2)
  • apps/server/src/http.test.ts
  • apps/server/src/http.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Asset responses now include no-transform in Cache-Control. Tests verify that HTML, CSS, JavaScript, and SVG assets preserve content types and security headers when clients send identity or multiple compression encodings.

Changes

Asset response handling

Layer / File(s) Summary
Asset cache-control directives
apps/server/src/http.ts, apps/server/src/http.test.ts
Asset responses add no-transform to their Cache-Control headers. Tests update expectations for streamed assets, raster images, and inline videos.
Compression response coverage
apps/server/src/http.test.ts
Tests serve HTML, CSS, JavaScript, and SVG assets through HttpRouter and httpCompressionLayer. They verify status, content type, body content, security headers, and absent content encoding.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 2f188

Asset responses now avoid transformation when compression is accepted, preserving content types and security headers for browser-rendered files. The updated behavior is covered for common asset types and is ready to merge.

Suggested reviewers: juliusmarminge, t3dotgg, bil0000

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the server fix that preserves asset content types when compression is accepted.
Description check ✅ Passed The description explains the problem, the no-transform workaround, regression coverage, validation results, and linked issue. It does not use the template headings or checklist, but it contains the re…
Linked Issues check ✅ Passed The changes satisfy issue #10935 by adding no-transform to asset cache headers, including the guarded-video override, and by adding regression tests for HTML, CSS, JavaScript, and SVG content types, b…
Out of Scope Changes check ✅ Passed The changes are limited to the asset cache-header workaround, the guarded-video override, and focused regression tests for the linked compression bug. No unrelated code changes are identified.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Compressed asset responses lose their Content-Type, so HTML previews render as plain text

1 participant