Skip to content

fix(static): handle weighted accept-encoding values - #4470

Open
MFA-G wants to merge 3 commits into
nitrojs:mainfrom
MFA-G:fix-accept-encoding-quality
Open

fix(static): handle weighted accept-encoding values#4470
MFA-G wants to merge 3 commits into
nitrojs:mainfrom
MFA-G:fix-accept-encoding-quality

Conversation

@MFA-G

@MFA-G MFA-G commented Jul 23, 2026

Copy link
Copy Markdown

Linked issue

Resolves #4457

Type of change

  • Bug fix (a non-breaking change that fixes an issue)
  • Documentation
  • New feature
  • Chore
  • Breaking change

Description

Strip optional parameters from each Accept-Encoding token before looking it up in the compressed-asset encoding map. This lets standard weighted values such as gzip; q=1.0 and br; q=0.9 match their precompressed assets instead of silently falling back to the uncompressed file, matching the RFC 9110 grammar for Accept-Encoding.

A focused unit regression test verifies that a weighted header selects the gzip asset and sets Content-Encoding: gzip.

Checklist

  • I have linked an issue.
  • Documentation not needed (internal parser bug fix, no public API change).
  • I have added tests to cover my changes.
  • I have run the tests and lint locally and they pass.

Validation

  • pnpm vitest run test/unit/static-middleware.test.ts (3 passed)
  • pnpm lint
  • pnpm stub
  • pnpm typecheck

@MFA-G
MFA-G requested a review from pi0 as a code owner July 23, 2026 01:24
@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

@MFA-G is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b1ca3807-3191-41f9-9105-ac004c1f40f7

📥 Commits

Reviewing files that changed from the base of the PR and between 471cc0d and 0015bab.

📒 Files selected for processing (2)
  • src/runtime/internal/static.ts
  • test/unit/static-middleware.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/runtime/internal/static.ts

📝 Walkthrough

Walkthrough

The static middleware now normalizes Accept-Encoding tokens, excludes encodings with q=0, and supports compressed asset selection when quality parameters are present. Tests cover gzip selection and fallback to the uncompressed asset.

Changes

Accept-Encoding support

Layer / File(s) Summary
Weighted encoding lookup and validation
src/runtime/internal/static.ts, test/unit/static-middleware.test.ts
Encoding tokens are normalized before EncodingMap lookup, q=0 encodings are excluded, and tests cover gzip selection and uncompressed fallback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • nitrojs/nitro#4075: Updates related Accept-Encoding handling in the static middleware during asset resolution.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commit format and clearly describes the weighted Accept-Encoding fix.
Description check ✅ Passed The description directly matches the patch and explains the q-value parsing fix and added test coverage.
Linked Issues check ✅ Passed The code strips optional q-parameters before EncodingMap lookup, which satisfies issue #4457's main requirement.
Out of Scope Changes check ✅ Passed The changes stay focused on Accept-Encoding parsing and regression tests, with no evident unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/internal/static.ts`:
- Line 24: Update the Accept-Encoding parsing in the encoding-selection map to
read each entry’s q parameter, exclude entries with q=0 before sorting or
selecting encodings, and preserve existing behavior for positive or omitted
quality values. Ensure requests such as gzip; q=0 do not select compressed
assets.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9fe0fe8c-eccc-4a8a-820e-cc488e89fde1

📥 Commits

Reviewing files that changed from the base of the PR and between 77b77ff and 645e138.

📒 Files selected for processing (2)
  • src/runtime/internal/static.ts
  • test/unit/static-middleware.test.ts

Comment thread src/runtime/internal/static.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/internal/static.ts`:
- Line 31: Normalize the trimmed Accept-Encoding token to lowercase before
indexing EncodingMap, so case variants such as GZIP, Br, and ZSTD resolve to the
existing encodings while preserving the current lookup flow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 469981da-e6f6-408d-aa42-721b8405d1f0

📥 Commits

Reviewing files that changed from the base of the PR and between 645e138 and 471cc0d.

📒 Files selected for processing (2)
  • src/runtime/internal/static.ts
  • test/unit/static-middleware.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/unit/static-middleware.test.ts

Comment thread src/runtime/internal/static.ts Outdated
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.

static: Accept-Encoding q-values silently drop compression when client includes weights

1 participant