From ce85490fabeb6d945273537ec00ff527c68c3c0e Mon Sep 17 00:00:00 2001 From: Daniel Peng Date: Fri, 6 Nov 2026 15:42:14 -0500 Subject: [PATCH] fix(express): forward query strings without deprecated url.parse Ticket: WCN-4315 --- modules/express/src/clientRoutes.ts | 14 ++- .../unit/clientRoutes/proxiedQueryStrings.ts | 114 ++++++++++++++++++ 2 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 modules/express/test/unit/clientRoutes/proxiedQueryStrings.ts diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 8f8c6b8791..8cab5dfcf9 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -45,7 +45,6 @@ import debugLib from 'debug'; import express from 'express'; import type { ParamsDictionary } from 'express-serve-static-core'; import * as _ from 'lodash'; -import * as url from 'url'; import * as superagent from 'superagent'; // RequestTracer should be extracted into a separate npm package (along with @@ -333,11 +332,14 @@ function handleCalculateMinerFeeInfo(req: ExpressApiRouteRequest<'express.calcul function createAPIPath(req: express.Request) { let apiPath = '/' + req.params[0]; if (!_.isEmpty(req.query)) { - // req.params does not contain the querystring, so we manually add them here - const urlDetails = url.parse(req.url); - if (urlDetails.search) { - // "search" is the properly URL encoded query params, prefixed with "?" - apiPath += urlDetails.search; + // req.params does not contain the querystring, so we take it from the raw req.url. + // Slice instead of URL parsing to preserve the query bytes exactly as the client sent + // them (e.g. WHATWG URL would re-encode a raw "'"). The fragment part cannot be + // forwarded, matching legacy url.parse().search behavior. + const queryStart = req.url.indexOf('?'); + if (queryStart !== -1) { + const fragmentStart = req.url.indexOf('#', queryStart); + apiPath += fragmentStart === -1 ? req.url.slice(queryStart) : req.url.slice(queryStart, fragmentStart); } } return apiPath; diff --git a/modules/express/test/unit/clientRoutes/proxiedQueryStrings.ts b/modules/express/test/unit/clientRoutes/proxiedQueryStrings.ts new file mode 100644 index 0000000000..adf998cdd0 --- /dev/null +++ b/modules/express/test/unit/clientRoutes/proxiedQueryStrings.ts @@ -0,0 +1,114 @@ +import 'should'; +import nock from 'nock'; +import { common } from '@bitgo/sdk-core'; +import { setupAgent } from '../../lib/testutil'; + +const BITGO_TEST_URI = common.Environments.test.uri; + +/** + * createAPIPath() (via the REST proxy handlers) is responsible for forwarding the client's + * query string bytes verbatim to the BitGo API. These tests run real requests through the + * express app and assert the exact upstream request path the BitGo SDK receives. + */ +describe('proxied request query strings', function () { + const upstreamPaths: string[] = []; + + before(function () { + if (!nock.isActive()) { + nock.activate(); + } + // Block external network but allow supertest's local connections + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + }); + + beforeEach(function () { + nock.cleanAll(); + upstreamPaths.length = 0; + // Single interceptor capturing every upstream GET path; the SDK prefetches + // /api/v1/client/constants on every unauthenticated BitGo instance, so filter it out. + nock(BITGO_TEST_URI) + .persist(true) + .get(/.*/) + .reply(function (this: { req: { path: string } }) { + const path = this.req.path; + if (!path.startsWith('/api/v1/client/constants')) { + upstreamPaths.push(path); + } + return [200, { ok: true }] as const; + }); + }); + + afterEach(function () { + nock.cleanAll(); + }); + + after(function () { + if (nock.isActive()) { + nock.restore(); + } + }); + + it('should forward a v1 path without query string', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets']); + }); + + it('should forward a simple v1 query string verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?allTokens=true&limit=5'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets?allTokens=true&limit=5']); + }); + + it('should forward percent-encoded v1 query values verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?space=%20&apostrophe=%27&plus=%2B'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets?space=%20&apostrophe=%27&plus=%2B']); + }); + + it('should forward a plus sign in a v1 query verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?label=a+b'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets?label=a+b']); + }); + + it('should forward repeated v1 query keys verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?coin=eth&coin=btc'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets?coin=eth&coin=btc']); + }); + + it('should forward a v1 query parameter with an empty value verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?empty='); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets?empty=']); + }); + + it('should not forward a bare question mark with no query params', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v1/wallets?'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v1/wallets']); + }); + + it('should forward a v2 user path with query string verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v2/user/me?client=web'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v2/user/me?client=web']); + }); + + it('should forward a v2 coin path with query string verbatim', async function () { + const agent = setupAgent(); + const res = await agent.get('/api/v2/tbtc/wallet?allTokens=true'); + res.status.should.equal(200); + upstreamPaths.should.deepEqual(['/api/v2/tbtc/wallet?allTokens=true']); + }); +});