Skip to content

Commit d049d5d

Browse files
committed
[SEA-NodeJS] refactor(kernel): pass proxy to the kernel as a structured object
Change the kernel proxy mapping from a flattened URL string to the structured napi `proxy` object (kernel #129), mirroring the kernel's internal ProxyConfig: `{ url, username?, password?, bypassHosts? }`. `buildKernelProxyOptions` now composes `url` from `protocol://host:port` (no embedded credentials) and forwards `auth.{username,password}` as separate basic-auth fields — eliminating the URL percent-encoding of credentials. The `noProxy` host list is forwarded as `bypassHosts` (previously unexpressible through the URL-string form). Regenerated napi contract (native/kernel/index.d.ts) carries the new `ProxyInput` object type. Verified via mitmproxy (HttpProxyTests, SEA leg): http / https / proxy-with-auth all route through the proxy and the query succeeds. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 6e75e49 commit d049d5d

2 files changed

Lines changed: 55 additions & 29 deletions

File tree

lib/kernel/KernelAuth.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,21 @@ export interface KernelHttpOptions {
195195

196196
/**
197197
* HTTP(S) proxy forwarded to the napi binding's `ConnectionOptions.proxy`
198-
* (kernel `ProxyConfig.url`). The public `ConnectionOptions.proxy` is the
198+
* (kernel `ProxyConfig`). The public `ConnectionOptions.proxy` is the
199199
* Thrift-shaped `{protocol, host, port, auth}`; `buildKernelProxyOptions`
200-
* composes a single proxy URL string (with any basic-auth credentials
201-
* percent-encoded into the `userinfo`) so the SAME connection option works
202-
* on both backends. The napi contract takes a flat `proxy?: string`.
200+
* maps it onto the kernel's structured proxy input — `url` composed from
201+
* `protocol://host:port`, with `auth.{username,password}` forwarded as
202+
* separate basic-auth fields (NOT embedded in the URL, so no percent-encoding
203+
* footgun) and the `noProxy` host list forwarded as `bypassHosts`. The same
204+
* connection option therefore works identically on both backends.
203205
*/
204206
export interface KernelProxyOptions {
205-
proxy?: string;
207+
proxy?: {
208+
url: string;
209+
username?: string;
210+
password?: string;
211+
bypassHosts?: string;
212+
};
206213
}
207214

208215
export type KernelNativeConnectionOptions = KernelSessionDefaults &
@@ -541,25 +548,28 @@ export function buildKernelRetryOptions(config: {
541548

542549
/**
543550
* Map the public `ConnectionOptions.proxy` (`{protocol, host, port, auth}` —
544-
* the same shape the Thrift backend accepts) onto the kernel's napi
545-
* `proxy?: string`. Composes `protocol://[user:pass@]host:port`, percent-
546-
* encoding any `auth.{username,password}` into the URL `userinfo` so
547-
* credentials containing reserved characters (`@`, `:`, `/`) survive intact —
548-
* the kernel parses the userinfo off and applies it as basic-auth. The kernel
549-
* accepts only `http://` / `https://`; a SOCKS protocol surfaces a clear
551+
* the same shape the Thrift backend accepts) onto the kernel's structured napi
552+
* proxy input. The `url` is composed from `protocol://host:port` (no embedded
553+
* credentials); `auth.{username,password}` are forwarded as separate
554+
* basic-auth fields (the kernel applies them via reqwest `Proxy::basic_auth`),
555+
* avoiding any URL percent-encoding footgun. The `noProxy` host list (a driver
556+
* option, not on the published `.d.ts`) is forwarded as `bypassHosts`. The
557+
* kernel accepts only `http://` / `https://`; a SOCKS protocol surfaces a clear
550558
* kernel error at connect (reqwest SOCKS support is not compiled in).
551559
*/
552560
export function buildKernelProxyOptions(options: ConnectionOptions): KernelProxyOptions {
553561
const { proxy } = options;
554562
if (!proxy) {
555563
return {};
556564
}
557-
const { username, password } = proxy.auth ?? {};
558-
const userinfo =
559-
username !== undefined ? `${encodeURIComponent(username)}:${encodeURIComponent(password ?? '')}@` : '';
560-
return {
561-
proxy: `${proxy.protocol}://${userinfo}${proxy.host}:${proxy.port}`,
565+
const { noProxy } = options as ConnectionOptions & { noProxy?: string };
566+
const out: NonNullable<KernelProxyOptions['proxy']> = {
567+
url: `${proxy.protocol}://${proxy.host}:${proxy.port}`,
562568
};
569+
if (proxy.auth?.username !== undefined) out.username = proxy.auth.username;
570+
if (proxy.auth?.password !== undefined) out.password = proxy.auth.password;
571+
if (typeof noProxy === 'string' && noProxy.length > 0) out.bypassHosts = noProxy;
572+
return { proxy: out };
563573
}
564574

565575
export function buildKernelConnectionOptions(options: ConnectionOptions): KernelNativeConnectionOptions {

native/kernel/index.d.ts

Lines changed: 29 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)