From ba434f69b860ea246210fa35bf9aff8d7709d46b Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Tue, 27 Aug 2024 20:10:32 -0700 Subject: [PATCH 1/2] fix(adapter-nextjs): duplicate response Set-Cookie headers in pages router --- ...okieStorageAdapterFromNextServerContext.ts | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts index 843235b7288..52996006167 100644 --- a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts +++ b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts @@ -171,20 +171,45 @@ const createCookieStorageAdapterFromGetServerSidePropsContext = ( return allCookies; }, set(name, value, options) { - response.appendHeader( - 'Set-Cookie', - `${ensureEncodedForJSCookie(name)}=${value};${ - options ? serializeSetCookieOptions(options) : '' - }`, - ); + const encodedName = ensureEncodedForJSCookie(name); + + const existingSetCookieValues = response.getHeader('Set-Cookie'); + + // if the cookies have already been set, we don't need to set them again. + if ( + Array.isArray(existingSetCookieValues) && + existingSetCookieValues.findIndex( + cookieValue => + cookieValue.startsWith(`${encodedName}=`) && + !cookieValue.startsWith(`${encodedName}=;`), + ) > -1 + ) { + return; + } + + const setCookieValue = `${encodedName}=${value};${ + options ? serializeSetCookieOptions(options) : '' + }`; + + response.appendHeader('Set-Cookie', setCookieValue); }, delete(name) { - response.appendHeader( - 'Set-Cookie', - `${ensureEncodedForJSCookie( - name, - )}=;Expires=${DATE_IN_THE_PAST.toUTCString()}`, - ); + const encodedName = ensureEncodedForJSCookie(name); + const setCookieValue = `${encodedName}=;Expires=${DATE_IN_THE_PAST.toUTCString()}`; + const existingSetCookieValues = response.getHeader('Set-Cookie'); + + // if the value for cookie deletion is already in the Set-Cookie header, we + // don't need to add the deletion value again. + if ( + (typeof existingSetCookieValues === 'string' && + existingSetCookieValues === setCookieValue) || + (Array.isArray(existingSetCookieValues) && + existingSetCookieValues.includes(setCookieValue)) + ) { + return; + } + + response.appendHeader('Set-Cookie', setCookieValue); }, }; }; From 2ec0b64dc97cc5f84ad9d74aa951967358d26847 Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Wed, 28 Aug 2024 12:59:54 -0700 Subject: [PATCH 2/2] chore: add unit tests --- ...torageAdapterFromNextServerContext.test.ts | 35 +++++++++++++++++++ ...okieStorageAdapterFromNextServerContext.ts | 34 ++++++++++-------- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts index e27b4243511..a42ec085e9c 100644 --- a/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts +++ b/packages/adapter-nextjs/__tests__/utils/createCookieStorageAdapterFromNextServerContext.test.ts @@ -412,6 +412,41 @@ describe('createCookieStorageAdapterFromNextServerContext', () => { 'key3=;Expires=Thu, 01 Jan 1970 00:00:00 GMT', ]); }); + + it('does not add duplicate cookies when the cookies are defined in the response Set-Cookie headers', () => { + const mockExistingSetCookieValues = [ + 'CognitoIdentityServiceProvider.1234.accessToken=1234;Path=/', + 'CognitoIdentityServiceProvider.1234.refreshToken=1234;Path=/', + 'CognitoIdentityServiceProvider.1234.identityId=;Expires=Thu, 01 Jan 1970 00:00:00 GMT', + ]; + + const request = new IncomingMessage(new Socket()); + const response = new ServerResponse(request); + const appendHeaderSpy = jest.spyOn(response, 'appendHeader'); + const getHeaderSpy = jest.spyOn(response, 'getHeader'); + + Object.defineProperty(request, 'cookies', { + get() { + return {}; + }, + }); + + getHeaderSpy.mockReturnValue(mockExistingSetCookieValues); + + const result = createCookieStorageAdapterFromNextServerContext({ + request: request as any, + response, + }); + + result.set('CognitoIdentityServiceProvider.1234.accessToken', '5678'); + expect(appendHeaderSpy).not.toHaveBeenCalled(); + + result.set('CognitoIdentityServiceProvider.1234.refreshToken', '5678'); + expect(appendHeaderSpy).not.toHaveBeenCalled(); + + result.delete('CognitoIdentityServiceProvider.1234.identityId'); + expect(appendHeaderSpy).not.toHaveBeenCalled(); + }); }); it('should throw error when no cookie storage adapter is created from the context', () => { diff --git a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts index 52996006167..e3f99cbf96c 100644 --- a/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts +++ b/packages/adapter-nextjs/src/utils/createCookieStorageAdapterFromNextServerContext.ts @@ -173,12 +173,13 @@ const createCookieStorageAdapterFromGetServerSidePropsContext = ( set(name, value, options) { const encodedName = ensureEncodedForJSCookie(name); - const existingSetCookieValues = response.getHeader('Set-Cookie'); + const existingValues = getExistingSetCookieValues( + response.getHeader('Set-Cookie'), + ); // if the cookies have already been set, we don't need to set them again. if ( - Array.isArray(existingSetCookieValues) && - existingSetCookieValues.findIndex( + existingValues.findIndex( cookieValue => cookieValue.startsWith(`${encodedName}=`) && !cookieValue.startsWith(`${encodedName}=;`), @@ -187,25 +188,23 @@ const createCookieStorageAdapterFromGetServerSidePropsContext = ( return; } - const setCookieValue = `${encodedName}=${value};${ - options ? serializeSetCookieOptions(options) : '' - }`; - - response.appendHeader('Set-Cookie', setCookieValue); + response.appendHeader( + 'Set-Cookie', + `${encodedName}=${value};${ + options ? serializeSetCookieOptions(options) : '' + }`, + ); }, delete(name) { const encodedName = ensureEncodedForJSCookie(name); const setCookieValue = `${encodedName}=;Expires=${DATE_IN_THE_PAST.toUTCString()}`; - const existingSetCookieValues = response.getHeader('Set-Cookie'); + const existingValues = getExistingSetCookieValues( + response.getHeader('Set-Cookie'), + ); // if the value for cookie deletion is already in the Set-Cookie header, we // don't need to add the deletion value again. - if ( - (typeof existingSetCookieValues === 'string' && - existingSetCookieValues === setCookieValue) || - (Array.isArray(existingSetCookieValues) && - existingSetCookieValues.includes(setCookieValue)) - ) { + if (existingValues.includes(setCookieValue)) { return; } @@ -275,3 +274,8 @@ const serializeSetCookieOptions = ( // we are not using those chars in the auth keys. const ensureEncodedForJSCookie = (name: string): string => encodeURIComponent(name).replace(/%(2[346B]|5E|60|7C)/g, decodeURIComponent); + +const getExistingSetCookieValues = ( + values: number | string | string[] | undefined, +): string[] => + values === undefined ? [] : Array.isArray(values) ? values : [String(values)];