From fdbec433d41d92bbc2e0e4c316a62e0e950fa78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= Date: Mon, 2 Aug 2021 16:37:37 +0200 Subject: [PATCH 1/5] fix: make headers lowercase to avoid case sensitive bugs --- .../httpClient/basicHttpClient.unit.spec.ts | 58 +++++++++++++++++++ .../core/src/httpClient/basicHttpClient.ts | 30 ++++++++-- packages/core/src/index.ts | 1 + 3 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts diff --git a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts new file mode 100644 index 0000000000..1d4b29508e --- /dev/null +++ b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts @@ -0,0 +1,58 @@ +// Copyright 2020 Cognite AS + +// @ts-ignore +import { + headersWithDefaultField, + HttpHeaders, +} from '../../httpClient/basicHttpClient'; + +function lengthOfHttpHeaders(headers?: HttpHeaders): number { + let counter = 0; + for (const _ in headers) { + counter += 1; + } + return counter; +} + +describe('basicHttpClient', () => { + describe('headersWithDefaultField', () => { + test('add missing', () => { + const emptyHeaders: HttpHeaders = {}; + const alteredEmptyHeaders = headersWithDefaultField( + emptyHeaders, + 'Accept', + 'application/json' + ); + expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); + expect('accept' in alteredEmptyHeaders).toBeTruthy(); + }); + test('not overwrite existing', () => { + const mediaType = 'image/png'; + const emptyHeaders: HttpHeaders = { + Accept: mediaType, + }; + const alteredEmptyHeaders = headersWithDefaultField( + emptyHeaders, + 'Accept', + 'application/json' + ); + expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); + expect('accept' in alteredEmptyHeaders).toBeTruthy(); + expect(alteredEmptyHeaders['accept']).toEqual(mediaType); + }); + test('to be case insensitive', () => { + const mediaType = 'image/png'; + const emptyHeaders: HttpHeaders = { + accept: mediaType, + }; + const alteredEmptyHeaders = headersWithDefaultField( + emptyHeaders, + 'Accept', + 'application/json' + ); + expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); + expect('accept' in alteredEmptyHeaders).toBeTruthy(); + expect(alteredEmptyHeaders['accept']).toEqual(mediaType); + }); + }); +}); diff --git a/packages/core/src/httpClient/basicHttpClient.ts b/packages/core/src/httpClient/basicHttpClient.ts index 0242525ecd..901fd27ed8 100644 --- a/packages/core/src/httpClient/basicHttpClient.ts +++ b/packages/core/src/httpClient/basicHttpClient.ts @@ -179,10 +179,11 @@ export class BasicHttpClient { request: HttpRequest ): Promise> { const url = this.constructUrl(request.path, request.params); - const headers: HttpHeaders = { - Accept: 'application/json', - ...request.headers, - }; + const headers = headersWithDefaultField( + request.headers, + 'Accept', + 'application/json' + ); let body = request.data; if (isJson(body)) { body = BasicHttpClient.transformRequestBody(body); @@ -224,6 +225,27 @@ export class BasicHttpClient { } } +function lowercaseHeaders(headers: HttpHeaders): HttpHeaders { + const lowercaseHeaders: HttpHeaders = {}; + for (const key in headers) { + lowercaseHeaders[key.toLowerCase()] = headers[key]; + } + return lowercaseHeaders; +} + +export function headersWithDefaultField( + headers: HttpHeaders = {}, + fieldName: string, + fieldValue: string +): HttpHeaders { + const lowerCaseFieldName = fieldName.toLowerCase(); + const updatedHeaders = { ...lowercaseHeaders(headers) }; + if (!(lowerCaseFieldName in updatedHeaders)) { + updatedHeaders[lowerCaseFieldName] = fieldValue; + } + return updatedHeaders; +} + export interface HttpRequest extends HttpRequestOptions { path: string; method: HttpMethod; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 39a1e13a8c..83c9f926be 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -2,6 +2,7 @@ import * as Constants from './constants'; import * as GraphUtils from './graphUtils'; import * as TestUtils from './testUtils'; + export * from './types'; export { MetadataMap } from './metadata'; From 0a05ce3f1df4bb41582ca10de512505c979190f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= Date: Tue, 10 Aug 2021 14:25:53 +0200 Subject: [PATCH 2/5] chore: do not transform user input --- .../httpClient/basicHttpClient.unit.spec.ts | 6 +++--- packages/core/src/httpClient/basicHttpClient.ts | 16 +++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts index 1d4b29508e..f1f79f1f3c 100644 --- a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts +++ b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts @@ -24,7 +24,7 @@ describe('basicHttpClient', () => { 'application/json' ); expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); - expect('accept' in alteredEmptyHeaders).toBeTruthy(); + expect('Accept' in alteredEmptyHeaders).toBeTruthy(); }); test('not overwrite existing', () => { const mediaType = 'image/png'; @@ -51,8 +51,8 @@ describe('basicHttpClient', () => { 'application/json' ); expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); - expect('accept' in alteredEmptyHeaders).toBeTruthy(); - expect(alteredEmptyHeaders['accept']).toEqual(mediaType); + expect('Accept' in alteredEmptyHeaders).toBeTruthy(); + expect(alteredEmptyHeaders['Accept']).toEqual(mediaType); }); }); }); diff --git a/packages/core/src/httpClient/basicHttpClient.ts b/packages/core/src/httpClient/basicHttpClient.ts index 901fd27ed8..0247c0913c 100644 --- a/packages/core/src/httpClient/basicHttpClient.ts +++ b/packages/core/src/httpClient/basicHttpClient.ts @@ -225,12 +225,12 @@ export class BasicHttpClient { } } -function lowercaseHeaders(headers: HttpHeaders): HttpHeaders { - const lowercaseHeaders: HttpHeaders = {}; +function lowercaseHeadersKeys(headers: HttpHeaders): string[] { + const keys: string[] = []; for (const key in headers) { - lowercaseHeaders[key.toLowerCase()] = headers[key]; + keys.push(key.toLowerCase()); } - return lowercaseHeaders; + return keys; } export function headersWithDefaultField( @@ -238,12 +238,10 @@ export function headersWithDefaultField( fieldName: string, fieldValue: string ): HttpHeaders { - const lowerCaseFieldName = fieldName.toLowerCase(); - const updatedHeaders = { ...lowercaseHeaders(headers) }; - if (!(lowerCaseFieldName in updatedHeaders)) { - updatedHeaders[lowerCaseFieldName] = fieldValue; + if (!(fieldName.toLowerCase() in lowercaseHeadersKeys(headers))) { + headers[fieldName] = fieldValue; } - return updatedHeaders; + return headers; } export interface HttpRequest extends HttpRequestOptions { From bf2449642e2df807ff29fa8b901d558a4740c3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= <7851860+andersfylling@users.noreply.github.com> Date: Tue, 10 Aug 2021 14:37:54 +0200 Subject: [PATCH 3/5] test: typo --- .../src/__tests__/httpClient/basicHttpClient.unit.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts index f1f79f1f3c..067b5eb707 100644 --- a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts +++ b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts @@ -51,8 +51,8 @@ describe('basicHttpClient', () => { 'application/json' ); expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); - expect('Accept' in alteredEmptyHeaders).toBeTruthy(); - expect(alteredEmptyHeaders['Accept']).toEqual(mediaType); + expect('accept' in alteredEmptyHeaders).toBeTruthy(); + expect(alteredEmptyHeaders['accept']).toEqual(mediaType); }); }); }); From 7d7c326ad7f0e1166f6800c19cf89ed9c7893d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= Date: Tue, 10 Aug 2021 18:43:39 +0200 Subject: [PATCH 4/5] chore: trigger tests From 36e84f818391ba35386993142369860391598224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= Date: Tue, 10 Aug 2021 19:19:01 +0200 Subject: [PATCH 5/5] fix: test if array contains lower case key --- .../src/__tests__/httpClient/basicHttpClient.unit.spec.ts | 4 ++-- packages/core/src/httpClient/basicHttpClient.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts index 067b5eb707..d98f24632e 100644 --- a/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts +++ b/packages/core/src/__tests__/httpClient/basicHttpClient.unit.spec.ts @@ -37,8 +37,8 @@ describe('basicHttpClient', () => { 'application/json' ); expect(lengthOfHttpHeaders(alteredEmptyHeaders)).toEqual(1); - expect('accept' in alteredEmptyHeaders).toBeTruthy(); - expect(alteredEmptyHeaders['accept']).toEqual(mediaType); + expect('Accept' in alteredEmptyHeaders).toBeTruthy(); + expect(alteredEmptyHeaders['Accept']).toEqual(mediaType); }); test('to be case insensitive', () => { const mediaType = 'image/png'; diff --git a/packages/core/src/httpClient/basicHttpClient.ts b/packages/core/src/httpClient/basicHttpClient.ts index f661a7f1e5..3e2f16c4f6 100644 --- a/packages/core/src/httpClient/basicHttpClient.ts +++ b/packages/core/src/httpClient/basicHttpClient.ts @@ -238,7 +238,9 @@ export function headersWithDefaultField( fieldName: string, fieldValue: string ): HttpHeaders { - if (!(fieldName.toLowerCase() in lowercaseHeadersKeys(headers))) { + const lowercaseHeaders = lowercaseHeadersKeys(headers); + const lowercaseKey = fieldName.toLowerCase(); + if (!lowercaseHeaders.includes(lowercaseKey)) { headers[fieldName] = fieldValue; } return headers;