From 63ea41971c42739e8d58e1ec0ae6b2951e42a29e Mon Sep 17 00:00:00 2001 From: Yshay Yaacobi Date: Wed, 15 Mar 2023 22:40:04 +0200 Subject: [PATCH 1/4] refactor proxy behavior --- packages/standalone-proxy/index.ts | 4 +- packages/standalone-proxy/src/app.ts | 47 ++++++------ packages/standalone-proxy/src/proxy.ts | 100 +++++++++++++++++-------- 3 files changed, 94 insertions(+), 57 deletions(-) diff --git a/packages/standalone-proxy/index.ts b/packages/standalone-proxy/index.ts index a0cefdce..6d23ca26 100644 --- a/packages/standalone-proxy/index.ts +++ b/packages/standalone-proxy/index.ts @@ -6,6 +6,7 @@ import { sshServer as createSshServer } from './src/ssh-server' import { getSSHKeys } from './src/ssh-keys' import url from 'url' import path from 'path' +import { isProxyRequest, proxyHandlers } from './src/proxy' const __dirname = url.fileURLToPath(new URL('.', import.meta.url)) @@ -32,7 +33,8 @@ const envStore = inMemoryPreviewEnvStore({ }, }) -const app = createApp({ envStore, sshPublicKey }) + +const app = createApp({ sshPublicKey, isProxyRequest: isProxyRequest(BASE_URL), proxyHandlers: proxyHandlers(envStore) }) const sshLogger = app.log.child({ name: 'ssh_server' }) const tunnelName = (clientId: string, remotePath: string) => { diff --git a/packages/standalone-proxy/src/app.ts b/packages/standalone-proxy/src/app.ts index 1f16c4bf..71be150d 100644 --- a/packages/standalone-proxy/src/app.ts +++ b/packages/standalone-proxy/src/app.ts @@ -1,39 +1,38 @@ -import Fastify, { RawRequestDefaultExpression } from 'fastify' +import Fastify from 'fastify' import { fastifyRequestContext } from '@fastify/request-context' -import { InternalServerError } from './errors' import { appLoggerFromEnv } from './logging' -import { proxyRoutes } from './proxy' -import { PreviewEnvStore } from './preview-env' +import http from 'http' +import internal from 'stream' -const rewriteUrl = ({ url, headers: { host } }: RawRequestDefaultExpression): string => { - if (!url) { - throw new InternalServerError('no url in request') - } - if (!host) { - throw new InternalServerError('no host header in request') - } - - const target = host.split('.', 1)[0] - if (!target.includes('-')) { - return url - } - - return `/proxy/${target}${url}` -} - -export const app = ({ envStore, sshPublicKey }: { - envStore: PreviewEnvStore +export const app = ({ sshPublicKey,isProxyRequest, proxyHandlers }: { sshPublicKey: string + isProxyRequest: (req: http.IncomingMessage) => boolean + proxyHandlers: { wsHandler: (req: http.IncomingMessage, socket: internal.Duplex, head: Buffer) => void, handler: (req: http.IncomingMessage, res: http.ServerResponse) => void } }) => Fastify({ + serverFactory: (handler) => { + const {wsHandler:proxyWsHandler, handler: proxyHandler } = proxyHandlers + const server = http.createServer((req, res) => { + if (isProxyRequest(req)){ + return proxyHandler(req, res) + } + return handler(req, res) + }) + server.on('upgrade', (req, socket, head) => { + if (isProxyRequest(req)){ + proxyWsHandler(req, socket, head) + } else { + socket.end() + } + }) + return server; + }, logger: appLoggerFromEnv(), - rewriteUrl, }) .register(fastifyRequestContext) .get('/healthz', { logLevel: 'warn' }, async () => 'OK') .get('/ssh-public-key', async () => sshPublicKey) - .register(proxyRoutes, { prefix: '/proxy/', envStore }) diff --git a/packages/standalone-proxy/src/proxy.ts b/packages/standalone-proxy/src/proxy.ts index a2ca78c5..984538e3 100644 --- a/packages/standalone-proxy/src/proxy.ts +++ b/packages/standalone-proxy/src/proxy.ts @@ -1,38 +1,53 @@ -import { FastifyPluginAsync, HTTPMethods } from 'fastify' import { PreviewEnvStore } from './preview-env' -import { NotFoundError } from './errors' import httpProxy from 'http-proxy' +import { IncomingMessage, ServerResponse } from 'http' +import internal from 'stream' -const ALL_METHODS = Object.freeze(['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT', 'OPTIONS']) as HTTPMethods[] - -export const proxyRoutes: FastifyPluginAsync<{ envStore: PreviewEnvStore }> = async (app, { envStore }) => { - const proxy = httpProxy.createProxy({}) - - app.addHook('onClose', () => proxy.close()) - - // prevent FST_ERR_CTP_INVALID_MEDIA_TYPE error - app.removeAllContentTypeParsers() - app.addContentTypeParser('*', function (_request, _payload, done) { done(null) }) +export const isProxyRequest = (baseUrl: {hostname:string, port:string}) => (req: IncomingMessage)=> { + const host = req.headers["host"] + if (!host) return false + const {hostname: reqHostname, port: reqPort} = new URL(`http://${host}`) + if (reqPort !== baseUrl.port) return false + return reqHostname.endsWith(baseUrl.hostname) && reqHostname !== baseUrl.hostname +} - app.route<{ - Params: { targetHost: string; ['*']: string } - }>({ - url: ':targetHost/*', - method: ALL_METHODS, - handler: async (req, res) => { - const { targetHost, ['*']: url } = req.params - req.log.debug('proxy request: %j', { targetHost, url, params: req.params }) - const env = await envStore.get(targetHost) +function asyncHandler(fn: (...args: TArgs) => Promise, onError: (error: unknown, ...args: TArgs)=> void ) { + return async (...args: TArgs) => { + try { + await fn(...args) + } catch (err) { + onError(err, ...args) + } + } +} +export function proxyHandlers(envStore: PreviewEnvStore, log=console){ + const proxy = httpProxy.createProxy({}) + const resolveTargetEnv = async (req: IncomingMessage)=>{ + const {url} = req + const host = req.headers['host'] + const targetHost = host?.split('.', 1)[0] + const env = await envStore.get(targetHost as string) + if (!env) { + log.warn('no env for %j', { targetHost, url }) + log.warn('no host header in request') + return; + } + return env + } + return { + handler: asyncHandler(async (req: IncomingMessage, res: ServerResponse) => { + const env = await resolveTargetEnv(req) if (!env) { - throw new NotFoundError(`host ${targetHost}`) + req.statusCode = 520; + return; } - req.raw.url = `/${url}` - - proxy.web( - req.raw, - res.raw, + log.info('proxying to %j', { target: env.target, url: req.url }) + + return proxy.web( + req, + res, { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore @@ -41,11 +56,32 @@ export const proxyRoutes: FastifyPluginAsync<{ envStore: PreviewEnvStore }> = as }, }, (err) => { - req.log.warn('error in proxy %j', err, { targetHost, url }) + log.warn('error in proxy %j', err, { targetHost: env.target, url: req.url }) + } + ) + }, (err)=> log.error('error in proxy %j', err) ), + wsHandler: asyncHandler(async (req: IncomingMessage, socket: internal.Duplex, head: Buffer) => { + const env = await resolveTargetEnv(req) + if (!env) { + socket.end(); + return; + } + return proxy.ws( + req, + socket, + head, + { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + target: { + socketPath: env.target, + }, + }, + (err) => { + log.warn('error in proxy %j', err, { targetHost: env.target, url: req.url }) } ) - return res - }, - }) -} + }, (err)=> log.error('error forwarding ws traffic %j', err)) + } +} \ No newline at end of file From 2b1f6876a8828bd3049adb8b5a40f3b3fdee6fe4 Mon Sep 17 00:00:00 2001 From: Yshay Yaacobi Date: Wed, 15 Mar 2023 22:56:48 +0200 Subject: [PATCH 2/4] fix --- packages/standalone-proxy/src/proxy.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/standalone-proxy/src/proxy.ts b/packages/standalone-proxy/src/proxy.ts index 984538e3..76bb5ec8 100644 --- a/packages/standalone-proxy/src/proxy.ts +++ b/packages/standalone-proxy/src/proxy.ts @@ -39,7 +39,8 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ handler: asyncHandler(async (req: IncomingMessage, res: ServerResponse) => { const env = await resolveTargetEnv(req) if (!env) { - req.statusCode = 520; + res.statusCode = 520; + res.end(); return; } From 74c5a431c25f1b6b55ffde6735ac2e3b488eab42 Mon Sep 17 00:00:00 2001 From: Yshay Yaacobi Date: Wed, 15 Mar 2023 23:00:36 +0200 Subject: [PATCH 3/4] fix status code --- packages/standalone-proxy/src/proxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/standalone-proxy/src/proxy.ts b/packages/standalone-proxy/src/proxy.ts index 76bb5ec8..16f8e628 100644 --- a/packages/standalone-proxy/src/proxy.ts +++ b/packages/standalone-proxy/src/proxy.ts @@ -39,7 +39,7 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ handler: asyncHandler(async (req: IncomingMessage, res: ServerResponse) => { const env = await resolveTargetEnv(req) if (!env) { - res.statusCode = 520; + res.statusCode = 502; res.end(); return; } From 806c5125647d60de1f557ca5570bf3d46fc94d97 Mon Sep 17 00:00:00 2001 From: Yshay Yaacobi Date: Thu, 16 Mar 2023 10:10:18 +0200 Subject: [PATCH 4/4] CR fixes --- packages/standalone-proxy/index.ts | 8 +++++--- packages/standalone-proxy/src/app.ts | 8 +++++--- packages/standalone-proxy/src/logging.ts | 7 +++---- packages/standalone-proxy/src/proxy.ts | 25 +++++++++++++++--------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/standalone-proxy/index.ts b/packages/standalone-proxy/index.ts index 6d23ca26..dfbaa463 100644 --- a/packages/standalone-proxy/index.ts +++ b/packages/standalone-proxy/index.ts @@ -7,6 +7,8 @@ import { getSSHKeys } from './src/ssh-keys' import url from 'url' import path from 'path' import { isProxyRequest, proxyHandlers } from './src/proxy' +import { appLoggerFromEnv } from './src/logging' +import pino from 'pino' const __dirname = url.fileURLToPath(new URL('.', import.meta.url)) @@ -33,9 +35,9 @@ const envStore = inMemoryPreviewEnvStore({ }, }) - -const app = createApp({ sshPublicKey, isProxyRequest: isProxyRequest(BASE_URL), proxyHandlers: proxyHandlers(envStore) }) -const sshLogger = app.log.child({ name: 'ssh_server' }) +const logger = pino(appLoggerFromEnv()) +const app = createApp({ sshPublicKey, isProxyRequest: isProxyRequest(BASE_URL), proxyHandlers: proxyHandlers({envStore, logger}), logger }) +const sshLogger = logger.child({ name: 'ssh_server' }) const tunnelName = (clientId: string, remotePath: string) => { const serviceName = remotePath.replace(/^\//, '') diff --git a/packages/standalone-proxy/src/app.ts b/packages/standalone-proxy/src/app.ts index 71be150d..12c7fa01 100644 --- a/packages/standalone-proxy/src/app.ts +++ b/packages/standalone-proxy/src/app.ts @@ -1,12 +1,13 @@ import Fastify from 'fastify' import { fastifyRequestContext } from '@fastify/request-context' -import { appLoggerFromEnv } from './logging' import http from 'http' import internal from 'stream' +import {Logger} from 'pino' -export const app = ({ sshPublicKey,isProxyRequest, proxyHandlers }: { +export const app = ({ sshPublicKey,isProxyRequest, proxyHandlers, logger }: { sshPublicKey: string isProxyRequest: (req: http.IncomingMessage) => boolean + logger: Logger proxyHandlers: { wsHandler: (req: http.IncomingMessage, socket: internal.Duplex, head: Buffer) => void, handler: (req: http.IncomingMessage, res: http.ServerResponse) => void } }) => Fastify({ @@ -22,12 +23,13 @@ export const app = ({ sshPublicKey,isProxyRequest, proxyHandlers }: { if (isProxyRequest(req)){ proxyWsHandler(req, socket, head) } else { + logger.warn('unexpected upgrade request %j', {url: req.url, host: req.headers['host']}) socket.end() } }) return server; }, - logger: appLoggerFromEnv(), + logger, }) .register(fastifyRequestContext) diff --git a/packages/standalone-proxy/src/logging.ts b/packages/standalone-proxy/src/logging.ts index 2983c162..36c1a1e8 100644 --- a/packages/standalone-proxy/src/logging.ts +++ b/packages/standalone-proxy/src/logging.ts @@ -1,6 +1,6 @@ -import { FastifyBaseLogger, FastifyLoggerOptions, PinoLoggerOptions } from 'fastify/types/logger' +import { PinoLoggerOptions } from 'fastify/types/logger' -const envToLogger: Record = { +const envToLogger: Record = { development: { level: process.env.DEBUG ? 'debug' : 'info', transport: { @@ -13,8 +13,7 @@ const envToLogger: Record envToLogger[process.env.NODE_ENV || 'development'] diff --git a/packages/standalone-proxy/src/proxy.ts b/packages/standalone-proxy/src/proxy.ts index 16f8e628..f6ef9bc5 100644 --- a/packages/standalone-proxy/src/proxy.ts +++ b/packages/standalone-proxy/src/proxy.ts @@ -2,13 +2,14 @@ import { PreviewEnvStore } from './preview-env' import httpProxy from 'http-proxy' import { IncomingMessage, ServerResponse } from 'http' import internal from 'stream' +import type { Logger } from 'pino' export const isProxyRequest = (baseUrl: {hostname:string, port:string}) => (req: IncomingMessage)=> { const host = req.headers["host"] if (!host) return false const {hostname: reqHostname, port: reqPort} = new URL(`http://${host}`) if (reqPort !== baseUrl.port) return false - return reqHostname.endsWith(baseUrl.hostname) && reqHostname !== baseUrl.hostname + return reqHostname.endsWith(`.${baseUrl.hostname}`) && reqHostname !== baseUrl.hostname } function asyncHandler(fn: (...args: TArgs) => Promise, onError: (error: unknown, ...args: TArgs)=> void ) { @@ -21,7 +22,13 @@ function asyncHandler(fn: (...args: TArgs) => Promise{ const {url} = req @@ -29,8 +36,8 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ const targetHost = host?.split('.', 1)[0] const env = await envStore.get(targetHost as string) if (!env) { - log.warn('no env for %j', { targetHost, url }) - log.warn('no host header in request') + logger.warn('no env for %j', { targetHost, url }) + logger.warn('no host header in request') return; } return env @@ -44,7 +51,7 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ return; } - log.info('proxying to %j', { target: env.target, url: req.url }) + logger.info('proxying to %j', { target: env.target, url: req.url }) return proxy.web( req, @@ -57,10 +64,10 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ }, }, (err) => { - log.warn('error in proxy %j', err, { targetHost: env.target, url: req.url }) + logger.warn('error in proxy %j', { error:err, targetHost: env.target, url: req.url }) } ) - }, (err)=> log.error('error in proxy %j', err) ), + }, (err)=> logger.error('error forwarding traffic %j', {error:err}) ), wsHandler: asyncHandler(async (req: IncomingMessage, socket: internal.Duplex, head: Buffer) => { const env = await resolveTargetEnv(req) if (!env) { @@ -79,10 +86,10 @@ export function proxyHandlers(envStore: PreviewEnvStore, log=console){ }, }, (err) => { - log.warn('error in proxy %j', err, { targetHost: env.target, url: req.url }) + logger.warn('error in ws proxy %j', { error:err, targetHost: env.target, url: req.url }) } ) - }, (err)=> log.error('error forwarding ws traffic %j', err)) + }, (err)=> logger.error('error forwarding ws traffic %j', {error: err})) } } \ No newline at end of file