Fast vhost - #77
Open
sheplu wants to merge 5 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| // Middlewares under test. | ||
| const staticMw = vhost('mail.example.com', handle) |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should capture a wildcard with both a prefix and a suffix', function (_, done) { | ||
| var app = createServer('user-*.example.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should 404 a single-wildcard host with the wrong prefix', function (_, done) { | ||
| var app = createServer('user-*.example.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should preserve wildcard case-insensitivity on prefix and suffix', function (_, done) { | ||
| var app = createServer('user-*.example.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should treat dot as a dot', function (_, done) { | ||
| var app = createServer('a.b.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should match case-insensitively', function (_, done) { | ||
| var app = createServer('mail.example.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
| }) | ||
|
|
||
| it('should match a mixed-case static host (fast-path regex fallback)', function (_, done) { | ||
| var app = createServer('mail.example.com', function (req, res) { |
Member
Author
There was a problem hiding this comment.
part of the test / benchmark
Member
Rewrite index.js as src/index.ts compiled with tsc to ESM in dist/, with
bundled type declarations. Matching logic is byte-for-byte faithful to v3.
Performance, while staying behaviour-identical:
- Static (non-wildcard) hostnames take an ASCII-gated fast path: a length
check + lowercase equality decide most requests without the regex, and
req.vhost is written directly with length 0. A toLowerCase shortcut was
rejected because RegExp i-folding differs from String#toLowerCase for code
points like U+212A (Kelvin) and 'İ' length-changing folds; the fast path is
gated on an ASCII pattern and defers to the regex otherwise.
- Wildcard string hostnames gain an always-safe minimum-length reject before
the regex (a match needs at least hostname.length chars). The regex still
does the matching and capture extraction.
- RegExp hostnames are unchanged.
Warm: static match ~1.37x, static no-match ~1.9x, wildcard short no-match
~2.2x vs v3; wildcard match / multi-star / RegExp stay ~1.0x.
Tooling: node:test + c8 + typescript-eslint flat config; the legacy
multi-version CI matrix is replaced with a Node 24 build-and-test job.
BREAKING CHANGE: package is now ESM-only (import, not require) and requires
Node.js 24+.
Replace the mocha suite with the built-in node:test runner (ESM) and grow it from 19 to 56 cases to lock the behavioural contract. New cases cover: multiple vhosts in series, null-prototype req.vhost, explicit length-0 static/RegExp shapes, case-insensitivity, regexp-metachar escaping, IPv6 with/without port, trailing-dot literals, and the optimization guards — the Unicode case-fold (U+212A must not match /k/i), the 'İ' length-changing fold, wildcard prefix/suffix capture + case-insensitivity, wrong-prefix and wrong-suffix 404s, non-ASCII host via the regex path, and Kelvin in a wildcard suffix. The module is imported via a stable package self-reference so the same suite runs unchanged against the compiled build. 100% function coverage via c8.
Switch README examples from require() to import, note the ESM-only / Node 24+ requirement and bundled types, and document the exported VHost type. Add the 4.0.0 HISTORY entry.
Pin the original v3 implementation at bench/v3-baseline.cjs so old-vs-new stays
reproducible. Add:
- bench/index.mjs quick single-process snapshot (tinybench)
- bench/session.mjs one fresh-process session, fixed-iteration hrtime
- bench/run-matrix.mjs N sessions × several lengths, mean/min/max/sd
- bench/collect.mjs persists raw + summary per run to bench/results/
- bench/{,run-}*experiments.mjs variant studies, each fuzz-checked for
identical captures against the regex (incl. the rejected wildcard string
matcher and the object-allocation lesson)
Store a 25-session × {10k,100k,1M} run and document method + results in
BENCHMARKS.md (medians reported alongside means since OS-scheduler spikes
inflate the mean at 1M).
Route by the framework-resolved req.hostname (Express 5, respects trust proxy / X-Forwarded-Host) when present, falling back to the raw Host header for plain Node.js / connect servers. req.vhost.host is populated from the same resolved value, so it reflects the host that routing actually used. Fixes #20, supersedes #21
sheplu
marked this pull request as ready for review
August 2, 2026 17:06
Member
Author
@UlisesGascon this should be good! |
bjohansebas
reviewed
Aug 3, 2026
| "moduleResolution": "NodeNext", | ||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "sourceMap": true, |
Member
There was a problem hiding this comment.
Do we really want the source map? It will only increase the bundle size, and since we're not using any legacy features, the generated code is practically the same as the source.
bjohansebas
reviewed
Aug 3, 2026
Comment on lines
+217
to
+230
| function hostnameof (host: string): string | undefined { | ||
| if (!host) { | ||
| return undefined | ||
| } | ||
|
|
||
| const offset = host[0] === '[' | ||
| ? host.indexOf(']') + 1 | ||
| : 0 | ||
| const index = host.indexOf(':', offset) | ||
|
|
||
| return index !== -1 | ||
| ? host.substring(0, index) | ||
| : host | ||
| } |
Member
There was a problem hiding this comment.
Hmm, well, it's not mentioned anywhere, but I think it's worth pointing out. For example, .example.com. and .example.com are considered equivalent, which is valid, although I still find it pretty tricky to understand (ref: expressjs/express#7331 (review)). The spec honestly just confuses me even more.
From what I understand, nothing actually requires us to reject that case. RFC 1034 says they represent the same domain, so in practice I think we should allow it as well, shouldn't we?
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
4 participants
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.
v4 rewrite of vhost:
req.hostnamewhen present (Express 5, respectstrust proxy/X-Forwarded-Host), falling back to theHostheader — fixes Using express vhost behind a reverse proxy #20, supersedes Add support for reverse proxies by checking req.hostname or req.host. Fixes #20 #21.req.vhost.hostnow reflects the value routing actually used.No change to the matching semantics or the
req.vhostcontract otherwise.