Repository navigation
Copy of #7575 - #7748
Copy of #7575#77481 commit merged into
Conversation
|
Since last review: 0 resolved, 1 still open, 2 new. Introduces a wrapped TypeScript Hyperdrive binding and virtual-host routing for PostgreSQL and MySQL. Findings outside the diff
Carried forward from the last review: api-compat, jsg-gc (no author changes in their files since then; earlier findings stand) Not re-run: tests, docs, compat-flags, design-simplicity, kj-style, memory-safety (no author changes in their files since the last review) Reviewed commit: 65187428 · github run |
| this.#fetcher = env.fetcher; | ||
| this.#scheme = scheme; | ||
| this.#port = PORTS[scheme]; | ||
| const authority = entrypoints.mapVirtualHost(this.#fetcher, this.#port); |
There was a problem hiding this comment.
[BLOCKING] mapVirtualHost() registers only a connectOverrides entry. Although the generated .workers.alt hostname is recognized by node:dns, getCallerDnsOverride() has no corresponding DNS entry and lookup falls through to normal DNS. The binding also does not define ip, despite the public Hyperdrive interface requiring a synthetic IPv4 for drivers that need an IP literal. Consequently, drivers that resolve connectionString's host, or read hd.ip, cannot connect through this binding. Register a synthetic IPv4 as both a DNS and connect override and expose it as the binding's ip property; cover dns.lookup(hd.host) and connect(${hd.ip}:${hd.port}) in this test suite.
a migration toward rpc backed binding - uses mapVirtualHost approach instead of extendedFetcher
5fc8f95 to
6518742
Compare
| this.#fetcher = env.fetcher; | ||
| this.#scheme = scheme; | ||
| this.#port = PORTS[scheme]; | ||
| const authority = entrypoints.mapVirtualHost(this.#fetcher, this.#port); |
There was a problem hiding this comment.
[WARNING] The binding's constructor is reachable as Object.getPrototypeOf(env.HYPERDRIVE).constructor. A Worker with a Durable Object namespace can repeatedly construct wrappers around fresh getByName() stubs. Each constructor call creates a random host and permanently inserts a new entry into the isolate-global connectOverrides map; the map has no removal path. This permits untrusted code to accumulate native entries across requests until the isolate runs out of memory. Require a module-private construction token and create instances only through the two binding factories.
| const authority = entrypoints.mapVirtualHost(this.#fetcher, this.#port); | |
| const hyperdriveBindingToken = Symbol(); | |
| constructor(token: symbol, env: HyperdriveEnv, scheme: HyperdriveScheme) { | |
| if (token !== hyperdriveBindingToken) { | |
| throw new TypeError('Hyperdrive bindings cannot be constructed directly'); | |
| } | |
| super(env.fetcher); | |
| // ... | |
| } | |
| export function makeHyperdrive(env: HyperdriveEnv, scheme: HyperdriveScheme): Hyperdrive { | |
| return new Hyperdrive(hyperdriveBindingToken, env, scheme); | |
| } |
No description provided.