From 4d2169b5e4c0941d49df9235c17e6e2829c2a8b0 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 15:56:10 +0100 Subject: [PATCH 1/7] fix: imprive headers parsing --- src/index.test.ts | 8 +-- src/index.ts | 28 ++++------- src/metrics.test.ts | 31 ++++++++++++ src/metrics.ts | 24 ++++----- src/util.test.ts | 117 ++++++++++++++++++++++++++++++++++++++++++++ src/util.ts | 44 +++++++++++++++++ src/version.ts | 2 +- 7 files changed, 216 insertions(+), 38 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index fb3677d..c40cbe2 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -650,7 +650,7 @@ test('Should abort previous request', async () => { client.updateContext({ userId: '456' }); // abort 2 await client.updateContext({ userId: '789' }); - expect(abortSpy).toBeCalledTimes(2); + expect(abortSpy).toHaveBeenCalledTimes(2); abortSpy.mockRestore(); }); @@ -690,7 +690,7 @@ test('Should run without abort controller', async () => { client.updateContext({ userId: '456' }); await client.updateContext({ userId: '789' }); - expect(abortSpy).toBeCalledTimes(0); + expect(abortSpy).toHaveBeenCalledTimes(0); abortSpy.mockRestore(); }); @@ -1037,7 +1037,7 @@ test('Updating context should wait on asynchronous start', async () => { userId: '123', }); - expect(fetchMock).toBeCalledTimes(2); + expect(fetchMock).toHaveBeenCalledTimes(2); }); test('Should not replace sessionId when updating context', async () => { @@ -1254,7 +1254,7 @@ test('Initializing client twice should show a console warning', async () => { await client.start(); await client.start(); // Expect console.error to be called once before start runs. - expect(console.error).toBeCalledTimes(2); + expect(console.error).toHaveBeenCalledTimes(2); }); test('Should pass under custom header clientKey', async () => { diff --git a/src/index.ts b/src/index.ts index eff6f41..c1a1f69 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,6 +7,7 @@ import EventsHandler from './events-handler'; import { computeContextHashValue, notNullOrUndefined, + parseHeaders, urlWithContextAsQuery, } from './util'; import { sdkVersion } from './version'; @@ -470,24 +471,15 @@ export class UnleashClient extends TinyEmitter { } private getHeaders() { - const isPOST = this.usePOSTrequests; - const headers = { - [this.headerName]: this.clientKey, - Accept: 'application/json', - 'x-unleash-sdk': sdkVersion, - 'x-unleash-connection-id': this.connectionId, - 'x-unleash-appname': this.context.appName, - }; - if (isPOST) { - headers['Content-Type'] = 'application/json'; - } - if (this.etag) { - headers['If-None-Match'] = this.etag; - } - Object.entries(this.customHeaders) - .filter(notNullOrUndefined) - .forEach(([name, value]) => (headers[name] = value)); - return headers; + return parseHeaders({ + clientKey: this.clientKey, + connectionId: this.connectionId, + appName: this.context.appName, + customHeaders: this.customHeaders, + headerName: this.headerName, + etag: this.etag, + isPost: this.usePOSTrequests, + }) } private async storeToggles(toggles: IToggle[]): Promise { diff --git a/src/metrics.test.ts b/src/metrics.test.ts index a2f9dc6..a3d995f 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -303,4 +303,35 @@ describe('Custom headers for metrics', () => { expect(requestBody.headers).not.toMatchObject(customHeaders); } ); + + test('Should use case-insensitive headers', () => { + const metrics = new Metrics({ + onError: console.error, + appName: 'test', + metricsInterval: 5, + disableMetrics: false, + url: 'http://localhost:3000', + clientKey: '123', + fetch: fetchMock, + headerName: 'Authorization', + customHeaders: { + 'Custom-Header': '123', + 'custom-header': '456', + 'unleash-APPname': 'override', + 'unleash-connection-id': 'override', + }, + connectionId: '123', + metricsIntervalInitial: 2, + }); + + metrics.count('foo', true); + metrics.sendMetrics(); + + const requestBody = getTypeSafeRequest(fetchMock); + expect(requestBody.headers).toMatchObject({ + 'custom-header': '123', + 'unleash-appname': 'override', + 'unleash-connection-id': '123', + }); + }) }); diff --git a/src/metrics.ts b/src/metrics.ts index c769163..4fbe8b1 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -1,6 +1,6 @@ // Simplified version of: https://github.com/Unleash/unleash-client-node/blob/main/src/metrics.ts -import { notNullOrUndefined } from './util'; +import { notNullOrUndefined, parseHeaders } from './util'; import { sdkVersion } from './version'; export interface MetricsOptions { @@ -122,20 +122,14 @@ export default class Metrics { } private getHeaders() { - const headers = { - [this.headerName]: this.clientKey, - Accept: 'application/json', - 'Content-Type': 'application/json', - 'x-unleash-sdk': sdkVersion, - 'x-unleash-connection-id': this.connectionId, - 'x-unleash-appname': this.appName, - }; - - Object.entries(this.customHeaders) - .filter(notNullOrUndefined) - .forEach(([name, value]) => (headers[name] = value)); - - return headers; + return parseHeaders({ + clientKey: this.clientKey, + appName: this.appName, + connectionId: this.connectionId, + customHeaders: this.customHeaders, + headerName: this.headerName, + isPost: true, + }); } public async sendMetrics(): Promise { diff --git a/src/util.test.ts b/src/util.test.ts index a05fe4f..8e83872 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -3,6 +3,7 @@ import { computeContextHashValue, contextString, urlWithContextAsQuery, + parseHeaders, } from './util'; test('should not add paramters to URL', async () => { @@ -119,3 +120,119 @@ describe('computeContextHashValue', () => { ); }); }); + +describe('parseHeaders', () => { + const clientKey = 'test-client-key'; + const appName = 'appName'; + const connectionId = '1234-5678'; + + test('should set basic headers correctly', () => { + const result = parseHeaders({ + clientKey, + appName, + connectionId, + }); + + expect(result).toEqual({ + authorization: clientKey, + 'unleash-sdk': 'unleash-client-js:__VERSION__', + 'unleash-appname': appName, + accept: 'application/json', + 'unleash-connection-id': connectionId, + }); + }); + + test('should set custom client key header name', () => { + const result = parseHeaders({ + clientKey, + appName, + connectionId, + headerName: 'auth', + }); + + expect(result).toMatchObject({ auth: clientKey }); + expect(Object.keys(result)).not.toContain('authorization'); + }); + + test('should add custom headers', () => { + const customHeaders = { + 'custom-header': 'custom-value', + 'unleash-connection-id': 'should-not-be-overwritten', + 'unleash-appname': 'new-app-name', + }; + const result = parseHeaders({ + clientKey, + appName, + connectionId, + customHeaders, + }); + + expect(Object.keys(result)).toHaveLength(6); + expect(result).toMatchObject({ + 'custom-header': 'custom-value', + 'unleash-connection-id': connectionId, + 'unleash-appname': 'new-app-name', + }); + }); + + test('should include etag if provided', () => { + const etag = '12345'; + const result = parseHeaders({ + clientKey, + appName, + connectionId, + etag, + }); + + expect(result['if-none-match']).toBe(etag); + }); + + test('should handle custom headers with mixed casing and normalize them', () => { + const customHeaders = { + 'custom-HEADER': 'custom-value-1', + 'Custom-Header': 'custom-value-2', + }; + const result = parseHeaders({ + clientKey, + appName, + connectionId, + customHeaders, + }); + + expect(result).toMatchObject({ + 'custom-header': 'custom-value-2', + }); + }); + + test('should ignore null or undefined custom headers', () => { + const customHeaders = { + header1: 'value1', + header2: null as any, + header3: undefined as any, + }; + const result = parseHeaders({ + clientKey, + appName, + connectionId, + customHeaders, + }); + + expect(result).toEqual( + expect.not.objectContaining({ + header2: expect.anything(), + header3: expect.anything(), + }) + ); + }); + + test('should include content-type for POST requests', () => { + const result = parseHeaders({ + clientKey, + appName, + connectionId, + isPost: true, + }); + + expect(result['content-type']).toBe('application/json'); + }); +}); diff --git a/src/util.ts b/src/util.ts index 7a6c35b..7fe7cfe 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,4 +1,5 @@ import { IContext } from '.'; +import { sdkVersion } from './version'; export const notNullOrUndefined = ([, value]: [string, string]) => value !== undefined && value !== null; @@ -70,3 +71,46 @@ export const computeContextHashValue = async (obj: IContext) => { return value; } }; + +export const parseHeaders = ({ + clientKey, + appName, + connectionId, + customHeaders, + headerName = 'authorization', + etag, + isPost, +}: { + clientKey: string; + connectionId: string; + appName: string; + customHeaders?: Record; + headerName?: string; + etag?: string; + isPost?: boolean; +}): Record => { + const headers: Record = { + 'accept': 'application/json', + [headerName.toLocaleLowerCase()]: clientKey, + 'unleash-sdk': sdkVersion, + 'unleash-appname': appName, + }; + + if (isPost) { + headers['content-type'] = 'application/json'; + } + + if (etag) { + headers['if-none-match'] = etag; + } + + Object.entries(customHeaders || {}) + .filter(notNullOrUndefined) + .forEach( + ([name, value]) => (headers[name.toLocaleLowerCase()] = value) + ); + + headers['unleash-connection-id'] = connectionId; + + return headers; +}; diff --git a/src/version.ts b/src/version.ts index 896c627..de28b7c 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1 +1 @@ -export const sdkVersion = `unleash-js@__VERSION__`; +export const sdkVersion = `unleash-client-js:__VERSION__`; From 8cdb5e2e76d8941bf58c9476621e609008ef50e5 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:02:43 +0100 Subject: [PATCH 2/7] remove unused imports --- src/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index c1a1f69..23e8261 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,11 +6,9 @@ import LocalStorageProvider from './storage-provider-local'; import EventsHandler from './events-handler'; import { computeContextHashValue, - notNullOrUndefined, parseHeaders, urlWithContextAsQuery, } from './util'; -import { sdkVersion } from './version'; import { uuidv4 } from './uuidv4'; const DEFINED_FIELDS = [ From cdc486098fc6ed86d485f3b1117f19aeac4ab090 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:37:49 +0100 Subject: [PATCH 3/7] fix: formatting --- src/index.ts | 2 +- src/metrics.test.ts | 2 +- src/util.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 23e8261..9f29185 100644 --- a/src/index.ts +++ b/src/index.ts @@ -477,7 +477,7 @@ export class UnleashClient extends TinyEmitter { headerName: this.headerName, etag: this.etag, isPost: this.usePOSTrequests, - }) + }); } private async storeToggles(toggles: IToggle[]): Promise { diff --git a/src/metrics.test.ts b/src/metrics.test.ts index a3d995f..b6a68fc 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -333,5 +333,5 @@ describe('Custom headers for metrics', () => { 'unleash-appname': 'override', 'unleash-connection-id': '123', }); - }) + }); }); diff --git a/src/util.ts b/src/util.ts index 7fe7cfe..3839d66 100644 --- a/src/util.ts +++ b/src/util.ts @@ -90,7 +90,7 @@ export const parseHeaders = ({ isPost?: boolean; }): Record => { const headers: Record = { - 'accept': 'application/json', + accept: 'application/json', [headerName.toLocaleLowerCase()]: clientKey, 'unleash-sdk': sdkVersion, 'unleash-appname': appName, From c1be4d513c1262e58ef430ac45678ed79fe74a32 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:39:24 +0100 Subject: [PATCH 4/7] remove unused imports --- src/metrics.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/metrics.ts b/src/metrics.ts index 4fbe8b1..0ef8c55 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -1,7 +1,6 @@ // Simplified version of: https://github.com/Unleash/unleash-client-node/blob/main/src/metrics.ts -import { notNullOrUndefined, parseHeaders } from './util'; -import { sdkVersion } from './version'; +import { parseHeaders } from './util'; export interface MetricsOptions { onError: OnError; From 2b350b85c05c6a9643675947ea3d3f34b66c4034 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:41:26 +0100 Subject: [PATCH 5/7] fix: test new identification headers format --- src/index.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index c40cbe2..a7ef050 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1356,7 +1356,7 @@ test('Should pass custom headers', async () => { }); }); -test('Should add `x-unleash` headers', async () => { +test('Should add unleash identification headers', async () => { fetchMock.mockResponses( [JSON.stringify(data), { status: 200 }], [JSON.stringify(data), { status: 200 }] @@ -1382,13 +1382,13 @@ test('Should add `x-unleash` headers', async () => { const expectedHeaders = { // will be replaced at build time with the actual version - 'x-unleash-sdk': 'unleash-js@__VERSION__', - 'x-unleash-connection-id': expect.stringMatching(uuidFormat), - 'x-unleash-appname': appName, + 'unleash-sdk': 'unleash-client-js:__VERSION__', + 'unleash-connection-id': expect.stringMatching(uuidFormat), + 'unleash-appname': appName, }; const getConnectionId = (request: any) => - request.headers['x-unleash-connection-id']; + request.headers['unleash-connection-id']; expect(featureRequest.headers).toMatchObject(expectedHeaders); expect(metricsRequest.headers).toMatchObject(expectedHeaders); From 33818860676a3e15db0cdda15a8d8d822fdfaaa5 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:50:00 +0100 Subject: [PATCH 6/7] fix: case-insensitive headers --- src/index.test.ts | 6 +++--- src/metrics.test.ts | 6 +++--- src/util.test.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/index.test.ts b/src/index.test.ts index a7ef050..e28529b 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -64,7 +64,7 @@ test('Should perform an initial fetch as POST', async () => { expect(request.method).toBe('POST'); expect(body.context.appName).toBe('webAsPOST'); expect(request.headers).toMatchObject({ - 'Content-Type': 'application/json', + 'content-type': 'application/json', }); }); @@ -783,7 +783,7 @@ test('Should include etag in second request', async () => { expect(firstRequest.headers).toMatchObject({}); expect(secondRequest.headers).toMatchObject({ - 'If-None-Match': etag, + 'if-none-match': etag, }); }); @@ -805,7 +805,7 @@ test('Should add clientKey as Authorization header', async () => { const request = getTypeSafeRequest(fetchMock); expect(request.headers).toMatchObject({ - Authorization: 'some123key', + authorization: 'some123key', }); }); diff --git a/src/metrics.test.ts b/src/metrics.test.ts index b6a68fc..5ca3832 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -91,7 +91,7 @@ test('should send metrics with custom auth header', async () => { expect(fetchMock.mock.calls.length).toEqual(1); expect(requestBody.headers).toMatchObject({ - NotAuthorization: '123', + 'notauthorization': '123', }); }); @@ -271,7 +271,7 @@ describe('Custom headers for metrics', () => { test('Custom headers should override preset headers', async () => { const customHeaders = { - Authorization: 'definitely-not-the-client-key', + authorization: 'definitely-not-the-client-key', }; const requestBody = await runMetrics(customHeaders); @@ -329,7 +329,7 @@ describe('Custom headers for metrics', () => { const requestBody = getTypeSafeRequest(fetchMock); expect(requestBody.headers).toMatchObject({ - 'custom-header': '123', + 'custom-header': '456', 'unleash-appname': 'override', 'unleash-connection-id': '123', }); diff --git a/src/util.test.ts b/src/util.test.ts index 8e83872..f846dd3 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -187,7 +187,7 @@ describe('parseHeaders', () => { expect(result['if-none-match']).toBe(etag); }); - test('should handle custom headers with mixed casing and normalize them', () => { + test('should handle custom headers in a case-insensitive way and normalize them', () => { const customHeaders = { 'custom-HEADER': 'custom-value-1', 'Custom-Header': 'custom-value-2', From 92eb342effcb2781438fbb73a308d034117a0564 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:51:42 +0100 Subject: [PATCH 7/7] fix: formatting --- src/metrics.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics.test.ts b/src/metrics.test.ts index 5ca3832..3702e66 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -91,7 +91,7 @@ test('should send metrics with custom auth header', async () => { expect(fetchMock.mock.calls.length).toEqual(1); expect(requestBody.headers).toMatchObject({ - 'notauthorization': '123', + notauthorization: '123', }); });