Skip to content

Commit

Permalink
Merge pull request #766 from snyk/fix/clean-up-connection-role-qs-in-srv
Browse files Browse the repository at this point in the history
fix: strip connection_role qs in http req into server
  • Loading branch information
aarlaud authored May 10, 2024
2 parents e082992 + dacc15c commit 6a41d48
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
13 changes: 11 additions & 2 deletions lib/server/routesHandlers/httpRequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { incrementHttpRequestsTotal } from '../../common/utils/metrics';
import { hostname } from 'node:os';
import { makeStreamingRequestToDownstream } from '../../common/http/request';
import { PostFilterPreparedRequest } from '../../common/relay/prepareRequest';
import { URL, URLSearchParams } from 'node:url';

export const overloadHttpRequestWithConnectionDetailsMiddleware = async (
req: Request,
Expand Down Expand Up @@ -63,10 +64,18 @@ export const overloadHttpRequestWithConnectionDetailsMiddleware = async (
req['locals'] = {};
req['locals']['capabilities'] =
connections.get(token)[0].metadata.capabilities;

// strip the leading url
req.url = req.url.slice(`/broker/${token}`.length);
logger.debug({ url: req.url }, 'request');
if (req.url.includes('connection_role')) {
const urlParts = req.url.split('?');
if (urlParts.length > 1) {
const params = new URLSearchParams(urlParts[1]);
params.delete('connection_role');
req.url =
params.size > 0 ? `${urlParts[0]}?${params.toString()}` : urlParts[0];
}
}

logger.debug({ url: req.url }, 'request');
next();
};
8 changes: 8 additions & 0 deletions test/functional/server-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,4 +582,12 @@ describe('proxy requests originating from behind the broker server', () => {
expect(response.status).toEqual(200);
expect(response.data).toStrictEqual({ proxyMe: 'please' });
});

it('successfully broker POST removing connection_role QS', async () => {
const response = await axiosClient.get(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-query?connection_role=primary&test=value&test2=value2`,
);
expect(response.status).toEqual(200);
expect(response.data).toStrictEqual({ test: 'value', test2: 'value2' });
});
});
7 changes: 6 additions & 1 deletion test/unit/old-client-redirect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ jest.mock('../../lib/server/socket', () => {
__esModule: true,
...originalModule,
getSocketConnections: () => {
return new Map();
const map = new Map();

map.set('7fe7a57b-aa0d-416a-97fc-472061737e24', [
{ socket: {}, socketVersion: '1', metadata: { capabilities: {} } },
]);
return map;
},
};
});
Expand Down

0 comments on commit 6a41d48

Please sign in to comment.