From e80bd3c9ad0e8199675122d70fc74e17cfeff956 Mon Sep 17 00:00:00 2001 From: "dan.castillo" Date: Fri, 6 Oct 2023 10:59:09 -0400 Subject: [PATCH 1/5] keep query params on proxy --- index.js | 23 ++++++++++++++++++++++- test/test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index b2752f2..3af8f54 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,7 @@ const { ServerResponse } = require('node:http') const WebSocket = require('ws') const { convertUrlToWebSocket } = require('./utils') const fp = require('fastify-plugin') +const querystring = require('node:querystring') const httpMethods = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT', 'OPTIONS'] const urlPattern = /^https?:\/\// @@ -305,6 +306,22 @@ async function fastifyHttpProxy (fastify, opts) { wsProxy = setupWebSocketProxy(fastify, opts, rewritePrefix) } + function extractUrlComponents (urlString) { + const components = { + path: '', + queryParams: {} + } + + const [path, queryString] = urlString.split('?') + components.path = path + + if (queryString) { + components.queryParams = querystring.parse(queryString) + } + + return components + } + function handler (request, reply) { if (request.raw[kWs]) { reply.hijack() @@ -319,8 +336,9 @@ async function fastifyHttpProxy (fastify, opts) { const queryParamIndex = request.raw.url.indexOf('?') let dest = request.raw.url.slice(0, queryParamIndex !== -1 ? queryParamIndex : undefined) + const { path, queryParams } = extractUrlComponents(request.url) if (this.prefix.includes(':')) { - const requestedPathElements = request.url.split('/') + const requestedPathElements = path.split('/') const prefixPathWithVariables = this.prefix.split('/').map((_, index) => requestedPathElements[index]).join('/') let rewritePrefixWithVariables = rewritePrefix @@ -329,6 +347,9 @@ async function fastifyHttpProxy (fastify, opts) { } dest = dest.replace(prefixPathWithVariables, rewritePrefixWithVariables) + if (Object.keys(queryParams).length) { + dest = `${dest}?${querystring.stringify(queryParams)}` + } } else { dest = dest.replace(this.prefix, rewritePrefix) } diff --git a/test/test.js b/test/test.js index 1c33afd..585844b 100644 --- a/test/test.js +++ b/test/test.js @@ -6,6 +6,7 @@ const proxy = require('../') const got = require('got') const { Unauthorized } = require('http-errors') const Transform = require('node:stream').Transform +const querystring = require('node:querystring') async function run () { const origin = Fastify() @@ -42,6 +43,10 @@ async function run () { return `this is "variable-api" endpoint with id ${request.params.id}` }) + origin.get('/variable-api/:id/endpoint-with-query', async (request, reply) => { + return `this is "variable-api" endpoint with id ${request.params.id} and query params ${JSON.stringify(request.query)}` + }) + origin.get('/timeout', async (request, reply) => { await new Promise((resolve) => setTimeout(resolve, 600)) return 'this is never received' @@ -890,6 +895,28 @@ async function run () { ) t.equal(resultFooRoute.body, 'Hello World (foo) - lang = en') }) + + test('keep the query params on proxy', { only: true }, async t => { + const proxyServer = Fastify() + + proxyServer.register(proxy, { + upstream: `http://localhost:${origin.server.address().port}`, + prefix: '/api/:id/endpoint', + rewritePrefix: '/variable-api/:id/endpoint-with-query' + }) + + await proxyServer.listen({ port: 0 }) + + t.teardown(() => { + proxyServer.close() + }) + + const firstProxyPrefix = await got( + `http://localhost:${proxyServer.server.address().port}/api/123/endpoint?foo=bar&foo=baz&abc=qux` + ) + const queryParams = JSON.stringify(querystring.parse('foo=bar&foo=baz&abc=qux')) + t.equal(firstProxyPrefix.body, `this is "variable-api" endpoint with id 123 and query params ${queryParams}`) + }) } run() From d36e9642dc71206ed30742143f90577812740249 Mon Sep 17 00:00:00 2001 From: "dan.castillo" Date: Fri, 6 Oct 2023 13:20:59 -0400 Subject: [PATCH 2/5] apply pr feedback --- index.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 3af8f54..86ee9be 100644 --- a/index.js +++ b/index.js @@ -307,14 +307,12 @@ async function fastifyHttpProxy (fastify, opts) { } function extractUrlComponents (urlString) { + const [path, queryString] = urlString.split('?') const components = { - path: '', + path, queryParams: {} } - const [path, queryString] = urlString.split('?') - components.path = path - if (queryString) { components.queryParams = querystring.parse(queryString) } @@ -347,8 +345,8 @@ async function fastifyHttpProxy (fastify, opts) { } dest = dest.replace(prefixPathWithVariables, rewritePrefixWithVariables) - if (Object.keys(queryParams).length) { - dest = `${dest}?${querystring.stringify(queryParams)}` + if (queryParamIndex !== -1) { + dest += `?${querystring.stringify(queryParams)}` } } else { dest = dest.replace(this.prefix, rewritePrefix) From 9e26186a670498de7d47e75623470ad579db6d00 Mon Sep 17 00:00:00 2001 From: "dan.castillo" Date: Sat, 7 Oct 2023 14:18:12 -0400 Subject: [PATCH 3/5] updated to use fast-querystring --- index.js | 6 +++--- package.json | 1 + test/test.js | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 86ee9be..a83c4dd 100644 --- a/index.js +++ b/index.js @@ -4,7 +4,7 @@ const { ServerResponse } = require('node:http') const WebSocket = require('ws') const { convertUrlToWebSocket } = require('./utils') const fp = require('fastify-plugin') -const querystring = require('node:querystring') +const qs = require('fast-querystring') const httpMethods = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT', 'OPTIONS'] const urlPattern = /^https?:\/\// @@ -314,7 +314,7 @@ async function fastifyHttpProxy (fastify, opts) { } if (queryString) { - components.queryParams = querystring.parse(queryString) + components.queryParams = qs.parse(queryString) } return components @@ -346,7 +346,7 @@ async function fastifyHttpProxy (fastify, opts) { dest = dest.replace(prefixPathWithVariables, rewritePrefixWithVariables) if (queryParamIndex !== -1) { - dest += `?${querystring.stringify(queryParams)}` + dest += `?${qs.stringify(queryParams)}` } } else { dest = dest.replace(this.prefix, rewritePrefix) diff --git a/package.json b/package.json index 90b082f..926c5fa 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ }, "dependencies": { "@fastify/reply-from": "^9.0.0", + "fast-querystring": "^1.1.2", "fastify-plugin": "^4.5.0", "ws": "^8.4.2" }, diff --git a/test/test.js b/test/test.js index 585844b..98bc258 100644 --- a/test/test.js +++ b/test/test.js @@ -6,7 +6,7 @@ const proxy = require('../') const got = require('got') const { Unauthorized } = require('http-errors') const Transform = require('node:stream').Transform -const querystring = require('node:querystring') +const qs = require('fast-querystring') async function run () { const origin = Fastify() @@ -914,7 +914,7 @@ async function run () { const firstProxyPrefix = await got( `http://localhost:${proxyServer.server.address().port}/api/123/endpoint?foo=bar&foo=baz&abc=qux` ) - const queryParams = JSON.stringify(querystring.parse('foo=bar&foo=baz&abc=qux')) + const queryParams = JSON.stringify(qs.parse('foo=bar&foo=baz&abc=qux')) t.equal(firstProxyPrefix.body, `this is "variable-api" endpoint with id 123 and query params ${queryParams}`) }) } From 60a5e256ca35cc73ad22f5a23a90d63f745cd682 Mon Sep 17 00:00:00 2001 From: "dan.castillo" Date: Sat, 7 Oct 2023 19:54:54 -0400 Subject: [PATCH 4/5] apply performace feedback from review --- index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index a83c4dd..4e1e546 100644 --- a/index.js +++ b/index.js @@ -310,7 +310,7 @@ async function fastifyHttpProxy (fastify, opts) { const [path, queryString] = urlString.split('?') const components = { path, - queryParams: {} + queryParams: null } if (queryString) { @@ -331,10 +331,9 @@ async function fastifyHttpProxy (fastify, opts) { } return } - const queryParamIndex = request.raw.url.indexOf('?') - let dest = request.raw.url.slice(0, queryParamIndex !== -1 ? queryParamIndex : undefined) - const { path, queryParams } = extractUrlComponents(request.url) + let dest = path + if (this.prefix.includes(':')) { const requestedPathElements = path.split('/') const prefixPathWithVariables = this.prefix.split('/').map((_, index) => requestedPathElements[index]).join('/') @@ -345,7 +344,7 @@ async function fastifyHttpProxy (fastify, opts) { } dest = dest.replace(prefixPathWithVariables, rewritePrefixWithVariables) - if (queryParamIndex !== -1) { + if (queryParams) { dest += `?${qs.stringify(queryParams)}` } } else { From c13bfb1c1b47a574c3fae1650698288e46ecabec Mon Sep 17 00:00:00 2001 From: "dan.castillo" Date: Sat, 21 Oct 2023 20:05:11 -0400 Subject: [PATCH 5/5] add in limit for string split --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 4e1e546..6c85d86 100644 --- a/index.js +++ b/index.js @@ -307,7 +307,7 @@ async function fastifyHttpProxy (fastify, opts) { } function extractUrlComponents (urlString) { - const [path, queryString] = urlString.split('?') + const [path, queryString] = urlString.split('?', 2) const components = { path, queryParams: null