Skip to content

Commit

Permalink
Merge pull request #728 from snyk/feat/add-selection-ws-vs-http-respo…
Browse files Browse the repository at this point in the history
…nse-for-srv

feat: add ws or http response selection logic
  • Loading branch information
aarlaud authored Mar 4, 2024
2 parents 15fe10f + 642bfaa commit b88e17e
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 3 deletions.
8 changes: 7 additions & 1 deletion lib/common/relay/forwardHttpRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export const forwardHttpRequest = (
// If this is the server, we should receive a Snyk-Request-Id header from upstream
// If this is the client, we will have to generate one
req.headers['snyk-request-id'] ||= uuid();
const responseWantedOverWs = req.headers['x-broker-ws-response']
? true
: false;
const logContext: ExtendedLogContext = {
url: req.url,
requestMethod: req.method,
Expand Down Expand Up @@ -212,7 +215,10 @@ export const forwardHttpRequest = (
return res.status(401).send({ message: 'blocked', reason, url: req.url });
} else {
incrementHttpRequestsTotal(false, 'inbound-request');
if (res?.locals?.capabilities?.includes('post-streams')) {
if (
res?.locals?.capabilities?.includes('post-streams') &&
!responseWantedOverWs
) {
makeWebsocketRequestWithStreamingResponse(filterResponse);
} else {
makeWebsocketRequestWithWebsocketResponse(filterResponse);
Expand Down
15 changes: 14 additions & 1 deletion lib/common/relay/forwardWebsocketRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ export const forwardWebSocketRequest = (
hashedToken: hashToken(connectionIdentifier),
transport:
websocketConnectionHandler?.socket?.transport?.name ?? 'unknown',
responseMedium: payload.headers['x-broker-ws-response']
? 'websocket'
: 'http',
};

if (!requestId) {
Expand Down Expand Up @@ -97,6 +100,13 @@ export const forwardWebSocketRequest = (
responseData,
isResponseFromRequestModule = false,
) => {
if (responseData) {
responseData['headers'] = responseData['headers'] ?? {};
responseData.headers['snyk-request-id'] = requestId;
responseData.headers['x-broker-ws-response'] =
responseData.headers['x-broker-ws-response'] ?? 'true';
}

// Traffic over websockets
if (payload.streamingID) {
if (isResponseFromRequestModule) {
Expand Down Expand Up @@ -135,7 +145,10 @@ export const forwardWebSocketRequest = (
};

if (
websocketConnectionHandler?.capabilities?.includes('receive-post-streams')
websocketConnectionHandler?.capabilities?.includes(
'receive-post-streams',
) &&
!emit
) {
// Traffic over HTTP Post
emit = postOverrideEmit;
Expand Down
1 change: 1 addition & 0 deletions lib/common/types/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ export interface ExtendedLogContext extends LogContext {
responseBodyType?: string;
ioErrorType?: string;
ioOriginalBodySize?: string;
responseMedium?: string;
}
11 changes: 10 additions & 1 deletion test/functional/server-client-universal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ describe('proxy requests originating from behind the broker server', () => {
`http://localhost:${bs.port}/broker/${process.env.BROKER_TOKEN_4}/echo-auth-header-with-bearer-auth/xyz`,
);

// const response5 = await axiosClient.get(
const response5 = await axiosClient.get(
`http://localhost:${bs.port}/broker/${process.env.BROKER_TOKEN_4}/echo-auth-header-with-bearer-auth/xyz`,
{ headers: { 'x-broker-ws-response': 'whatever' } },
);

// const response6 = await axiosClient.get(
// `http://localhost:${bs.port}/broker/${process.env.BROKER_TOKEN_3}/echo-auth-header-with-token-auth/xyz`,
// );

Expand All @@ -101,6 +106,10 @@ describe('proxy requests originating from behind the broker server', () => {
);
expect(response4.status).toEqual(200);
expect(response4.data).toEqual(`Bearer ${process.env.JIRA_PAT}`);

expect(response5.status).toEqual(200);
expect(response5.data).toEqual(`Bearer ${process.env.JIRA_PAT}`);
expect(response.headers['x-broker-ws-response']).not.toBeNull();
});

it('successfully warn logs requests without x-snyk-broker-type header', async () => {
Expand Down
32 changes: 32 additions & 0 deletions test/functional/server-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,43 @@ describe('proxy requests originating from behind the broker server', () => {
expect(Buffer.from(response.data)).toEqual(body);
});

it('successfully broker exact bytes of POST body with WS response', async () => {
// stringify the JSON unusually to ensure an unusual exact body
const body = Buffer.from(
JSON.stringify({ some: { example: 'json' } }, null, 5),
);
const response = await axiosClient.post(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-body`,
body,
{
headers: {
'content-type': 'application/json',
'x-broker-ws-response': 'whatever',
},

transformResponse: (r) => r,
},
);
expect(response.status).toEqual(200);
expect(Buffer.from(response.data)).toEqual(body);
expect(response.headers['x-broker-ws-response']).not.toBeNull();
});
it('successfully broker GET', async () => {
const response = await axiosClient.get(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-param/xyz`,
);

expect(response.headers['x-broker-ws-response']).not.toBeNull();
expect(response.status).toEqual(200);
expect(response.data).toEqual('xyz');
});

it('successfully broker GET with WS response', async () => {
const response = await axiosClient.get(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-param/xyz`,
{ headers: { 'x-broker-ws-response': 'whatever' } },
);
expect(response.headers['x-broker-ws-response']).not.toBeNull();
expect(response.status).toEqual(200);
expect(response.data).toEqual('xyz');
});
Expand Down

0 comments on commit b88e17e

Please sign in to comment.