fix: derive express subdomain offset from the configured root domain - #3575
Open
MannXo wants to merge 1 commit into
Open
fix: derive express subdomain offset from the configured root domain#3575MannXo wants to merge 1 commit into
MannXo wants to merge 1 commit into
Conversation
Express derives `req.subdomains` by dropping a fixed number of labels from the right of the hostname, defaulting to 2. A deployment whose `domain` has more than two labels therefore reads its own root domain as an active subdomain, so the user-site redirect sends the root origin to the static hosting domain, which sends it back. Self-hosting docs recommend exactly that shape (`puter.example.com`). Set the offset from the label count of `config.domain`. Two-label domains keep the express default, so existing deployments are unchanged. Fixes HeyPuter#3561
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Express derives
req.subdomainsby dropping a fixed number of labels from the right of the hostname, and that number defaults to 2. Any deployment whosedomainhas more than two labels reads its own root domain as an active subdomain, which sends the root origin into the user-site redirect and back again. This sets the offset from the label count ofconfig.domain.Context
Reported in #3561 for
puter.bhhaihuan.com. It is not an exotic setup. doc/self-hosting.md tells self-hosters to use exactly that shape ("If users reach the box atputer.example.com, thendomainmust beputer.example.com").With
domain: puter.example.com, a request to the bare root domain parses as subdomainputer. Two things follow.createUserSubdomainRedirectsees a non-reserved subdomain and 302s to the static hosting domain, which redirects back. And every root-only route stops matching, because#materializeRoutegates those onreq.subdomains.length === 0. The redirect runs ahead of the routes, so the whole deployment is unreachable, not just/.Eight call sites read
req.subdomains, and all of them reason about labels relative to the main domain, so the main domain's own label count is the right offset for all of them.Changes
subdomainOffsetForDomain()insrc/backend/util/subdomains.ts. It counts the root domain's labels and tolerates casing, surrounding space, a port and a leading dot, falling back to express's 2 whendomainis unset.src/backend/server.tssetssubdomain offsetalongsidetrust proxy.server.test.tsthat boots a server on a three-label domain.Two-label domains (
puter.com,puter.localhost) resolve to 2, so nothing changes for existing deployments.Testing
CI=true npm run test:backend -- --coveragenpm run typecheckreports no new errors (83 known, baselined).The regression test fails without the fix:
I also ran the real server rather than only the suite, with
PUTER_CONFIG_PATHpointing at an override that setsdomain: puter.example.localhostand the hosting domains under it, then curled it with matchingHostheaders.Before, both the root path and
/healthcheckbounce off the root origin:After:
Notes for reviewers
The offset is a single global value, so a deployment whose hosting domain has fewer labels than its main domain would compute an empty
req.subdomainson hosting hosts. That path is already safe.puterSitematches hosting hosts by suffix itself and terminates the request before any route runs, and the documented layout (site.<domain>) never produces a hosting domain shorter than the main one.I did not add the env-var override from the issue's workaround.
config.domainis already the authoritative value, so a second knob would only be another thing to set inconsistently.Fixes #3561