Skip to content

fix: send requests that rank text/html below another type to the endpoint - #17186

Open
SulimanAbdulrazzaq wants to merge 3 commits into
sveltejs:mainfrom
SulimanAbdulrazzaq:fix/content-negotiation-accept-quality
Open

SulimanAbdulrazzaq wants to merge 3 commits into
sveltejs:mainfrom
SulimanAbdulrazzaq:fix/content-negotiation-accept-quality

Conversation

@SulimanAbdulrazzaq

Copy link
Copy Markdown

closes #13834

When a route has both +page.svelte and +server.js, the docs say GET/POST/HEAD requests are page requests "if the accept header prioritises text/html". is_endpoint_request checked this with negotiate(accept, ['*', 'text/html']) !== 'text/html'. But '*' only matches a literal */* range. As a result, any accept header that names text/html at all was treated as a page request, whatever quality it gave it.

Feed readers hit this. SimplePie, which FreshRSS uses, sends application/atom+xml, application/rss+xml, application/rdf+xml;q=0.9, application/xml;q=0.8, text/xml;q=0.8, text/html;q=0.7, unknown/unknown;q=0.1, application/unknown;q=0.1, */*;q=0.1, and gets the page instead of the feed from +server.js.

This PR adds a prefers_html(accept) helper next to negotiate in utils/http.js, and uses it in is_endpoint_request. It returns true only when text/html (or text/*) has the highest quality in the header, ties included. negotiate itself is unchanged: its parsing and sorting moved into a shared parse_accept.

Behaviour for other headers stays the same:

  • Browser navigations still get the page.
  • */*, a missing accept header and application/json still go to the endpoint.
  • A use:enhance submission still gets the page.

The new is_endpoint_request tests cover these cases and pass both before and after the change.

Tests:

  • src/runtime/server/endpoint.spec.js (new): is_endpoint_request with a browser navigation, the SimplePie header, application/json, text/html;q=0.9, */*, no header, and a use:enhance POST.
  • src/utils/http.spec.js: prefers_html cases.
  • test/apps/basics/test/vitest/server.spec.js: a request to /routing/content-negotiation with the SimplePie header now reaches the endpoint. Before this change, the page HTML came back.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

…oint

is_endpoint_request called negotiate(accept, ['*', 'text/html']). The
'*' entry only matches a literal */* range, so any accept header that
named text/html at all was treated as a page request, whatever quality
it gave it. Feed readers such as FreshRSS send
'application/atom+xml, ..., text/html;q=0.7, ...' and got the page
instead of the +server.js feed.

Add a prefers_html helper that returns true only when text/html (or
text/*) has the highest quality in the header, which is what the docs
describe, and use it in is_endpoint_request.

Fixes sveltejs#13834
@pkg-svelte-dev

pkg-svelte-dev Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Install the latest version of @sveltejs/kit from f110b73:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/f110b735de19d546486d257032bf27d2b55241e7

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/17186

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

@changeset-bot

changeset-bot Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f110b73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@kdelay kdelay 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.

parse_accept only reads q right after the subtype, so application/json;charset=utf-8;q=0.5 parses as q=1. Since prefers_html now compares against the top q of all ranges, these go to the endpoint even though text/html ranks highest (page requests before this PR):

text/html;q=0.9, application/signed-exchange;v=b3;q=0.7
text/html;q=0.8, application/json;charset=utf-8;q=0.5

Reading q separately fixed both locally and kept the existing cases passing:

const q = /;[ \t]*q=([0-9.]+)/.exec(str)?.[1] ?? '1';

parse_accept only read q directly after the subtype, so a range such as application/json;charset=utf-8;q=0.5 was treated as q=1 and could outrank text/html.
@SulimanAbdulrazzaq

Copy link
Copy Markdown
Author

@kdelay Thanks, good catch. The parsing predates this PR (it's the regex negotiate has always used), but prefers_html compares against the top q, so it matters much more now.

Fixed in b841ee5 using your approach: q is read with /;[ \t]*q=([0-9.]+)/ wherever it sits among the parameters. I added both of your headers as regression tests in http.spec.js and endpoint.spec.js, plus a negotiate case, since negotiate gets the same fix. The existing cases still pass.

@teemingc teemingc reopened this Oct 1, 2026
@teemingc
teemingc changed the base branch from version-3 to main October 1, 2026 20:48
@SulimanAbdulrazzaq

Copy link
Copy Markdown
Author

@Rich-Harris @teemingc when you have a moment, could one of you take a look at this one?

This branch has not been deployed

No deployments
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.

Endpoint fallback broken when Accept header includes lower-priority text/html

4 participants