From 512387bc4c59c4b2bc45c8cfc95295016d7bce16 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 7 Apr 2021 17:36:16 -0700 Subject: [PATCH 1/5] consolidate error handling --- packages/utils/src/string.ts | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index 39d1fd251094..479632875e97 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -114,12 +114,6 @@ export function isMatchingPattern(value: string, pattern: RegExp | string): bool export function unicodeToBase64(plaintext: string): string { const global = getGlobalObject(); - // Cast to a string just in case we're given something else - const stringifiedInput = String(plaintext); - const errMsg = `Unable to convert to base64: ${ - stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput - }`; - // To account for the fact that different platforms use different character encodings natively, our `tracestate` // spec calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. try { @@ -142,12 +136,17 @@ export function unicodeToBase64(plaintext: string): string { // unlike the browser, Node can go straight from bytes to base64 return bytes.toString('base64'); } + + // we shouldn't ever get here, because one of `btoa` and `Buffer` should exist, but just in case... + throw new SentryError('Neither `window.btoa` nor `global.Buffer` is defined.'); } catch (err) { + // Cast to a string just in case we're given something else + const stringifiedInput = String(plaintext); + const errMsg = `Unable to convert to base64: ${ + stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }`; throw new SentryError(`${errMsg}\nGot error: ${err}`); } - - // we shouldn't ever get here, because one of `btoa` and `Buffer` should exist, but just in case... - throw new SentryError(errMsg); } /** @@ -160,12 +159,6 @@ export function unicodeToBase64(plaintext: string): string { export function base64ToUnicode(base64String: string): string { const globalObject = getGlobalObject(); - // we cast to a string just in case we're given something else - const stringifiedInput = String(base64String); - const errMsg = `Unable to convert from base64: ${ - stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput - }`; - // To account for the fact that different platforms use different character encodings natively, our `tracestate` spec // calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. So to reverse // the process, decode from base64 to bytes, then feed those bytes to a UTF-8 decoder. @@ -188,10 +181,15 @@ export function base64ToUnicode(base64String: string): string { // decode using UTF-8 return bytes.toString('utf-8'); } + + // we shouldn't ever get here, because one of `atob` and `Buffer` should exist, but just in case... + throw new SentryError('Neither `window.atob` nor `global.Buffer` is defined.'); } catch (err) { + // we cast to a string just in case we're given something else + const stringifiedInput = String(base64String); + const errMsg = `Unable to convert from base64: ${ + stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }`; throw new SentryError(`${errMsg}\nGot error: ${err}`); } - - // we shouldn't ever get here, because one of `atob` and `Buffer` should exist, but just in case... - throw new SentryError(errMsg); } From 323a27581a9c8b88374f17d8caf8ffac2ecfab2c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 8 Apr 2021 12:06:06 -0700 Subject: [PATCH 2/5] get Buffer and atob/btoa from global object explicitly --- packages/utils/src/string.ts | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index 479632875e97..d95d6e789862 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -104,6 +104,14 @@ export function isMatchingPattern(value: string, pattern: RegExp | string): bool return false; } +type GlobalWithBase64Helpers = { + // browser + atob?: (base64String: string) => string; + btoa?: (utf8String: string) => string; + // Node + Buffer?: { from: (input: string, encoding: string) => { toString: (encoding: string) => string } }; +}; + /** * Convert a Unicode string to a base64 string. * @@ -112,26 +120,30 @@ export function isMatchingPattern(value: string, pattern: RegExp | string): bool * @returns A base64-encoded version of the string */ export function unicodeToBase64(plaintext: string): string { - const global = getGlobalObject(); + const globalObject = getGlobalObject() as GlobalWithBase64Helpers; // To account for the fact that different platforms use different character encodings natively, our `tracestate` // spec calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. try { // browser - if ('btoa' in global) { + if ('btoa' in globalObject) { // encode using UTF-8 const bytes = new TextEncoder().encode(plaintext); // decode using UTF-16 (JS's native encoding) since `btoa` requires string input const bytesAsString = String.fromCharCode(...bytes); - return btoa(bytesAsString); + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return globalObject.btoa!(bytesAsString); } // Node - if ('Buffer' in global) { + if ('Buffer' in globalObject) { // encode using UTF-8 - const bytes = Buffer.from(plaintext, 'utf-8'); + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytes = globalObject.Buffer!.from(plaintext, 'utf-8'); // unlike the browser, Node can go straight from bytes to base64 return bytes.toString('base64'); @@ -157,7 +169,7 @@ export function unicodeToBase64(plaintext: string): string { * @returns A Unicode string */ export function base64ToUnicode(base64String: string): string { - const globalObject = getGlobalObject(); + const globalObject = getGlobalObject() as GlobalWithBase64Helpers; // To account for the fact that different platforms use different character encodings natively, our `tracestate` spec // calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. So to reverse @@ -166,7 +178,9 @@ export function base64ToUnicode(base64String: string): string { // browser if ('atob' in globalObject) { // `atob` returns a string rather than bytes, so we first need to encode using the native encoding (UTF-16) - const bytesAsString = atob(base64String); + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytesAsString = globalObject.atob!(base64String); const bytes = [...bytesAsString].map(char => char.charCodeAt(0)); // decode using UTF-8 (cast the `bytes` arry to a Uint8Array just because that's the format `decode()` expects) @@ -176,7 +190,9 @@ export function base64ToUnicode(base64String: string): string { // Node if ('Buffer' in globalObject) { // unlike the browser, Node can go straight from base64 to bytes - const bytes = Buffer.from(base64String, 'base64'); + // TODO: if TS ever learns about "in", we can get rid of the non-null assertion + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const bytes = globalObject.Buffer!.from(base64String, 'base64'); // decode using UTF-8 return bytes.toString('utf-8'); From 14636e882a9a6e5137549f0937d533b2b7d5d2c0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 8 Apr 2021 12:06:35 -0700 Subject: [PATCH 3/5] add typechecking --- packages/utils/src/string.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index d95d6e789862..e20e6556b5fc 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -125,6 +125,10 @@ export function unicodeToBase64(plaintext: string): string { // To account for the fact that different platforms use different character encodings natively, our `tracestate` // spec calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. try { + if (typeof plaintext !== 'string') { + throw new Error(`Input must be a string. Received input of type ${typeof plaintext}.`); + } + // browser if ('btoa' in globalObject) { // encode using UTF-8 @@ -175,6 +179,10 @@ export function base64ToUnicode(base64String: string): string { // calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. So to reverse // the process, decode from base64 to bytes, then feed those bytes to a UTF-8 decoder. try { + if (typeof base64String !== 'string') { + throw new Error(`Input must be a string. Received input of type ${typeof base64String}.`); + } + // browser if ('atob' in globalObject) { // `atob` returns a string rather than bytes, so we first need to encode using the native encoding (UTF-16) From 9228f1e45a99cec804276d0ba797660bcf6031f1 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 8 Apr 2021 12:07:14 -0700 Subject: [PATCH 4/5] test in browser as well as node --- packages/browser/test/unit/string.test.ts | 64 +++++++++++++++++++++++ packages/utils/test/string.test.ts | 3 ++ 2 files changed, 67 insertions(+) create mode 100644 packages/browser/test/unit/string.test.ts diff --git a/packages/browser/test/unit/string.test.ts b/packages/browser/test/unit/string.test.ts new file mode 100644 index 000000000000..56119c19a906 --- /dev/null +++ b/packages/browser/test/unit/string.test.ts @@ -0,0 +1,64 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { base64ToUnicode, unicodeToBase64 } from '@sentry/utils'; +import { expect } from 'chai'; + +// See https://tools.ietf.org/html/rfc4648#section-4 for base64 spec +// eslint-disable-next-line no-useless-escape +const BASE64_REGEX = /([a-zA-Z0-9+/]{4})*(|([a-zA-Z0-9+/]{3}=)|([a-zA-Z0-9+/]{2}==))/; + +// NOTE: These tests are copied (and adapted for chai syntax) from `string.test.ts` in `@sentry/utils`. The +// base64-conversion functions have a different implementation in browser and node, so they're copied here to prove they +// work in a real live browser. If you make changes here, make sure to also port them over to that copy. +describe('base64ToUnicode/unicodeToBase64', () => { + const unicodeString = 'Dogs are great!'; + const base64String = 'RG9ncyBhcmUgZ3JlYXQh'; + + it('converts to valid base64', () => { + expect(BASE64_REGEX.test(unicodeToBase64(unicodeString))).to.be.true; + }); + + it('works as expected', () => { + expect(unicodeToBase64(unicodeString)).to.equal(base64String); + expect(base64ToUnicode(base64String)).to.equal(unicodeString); + }); + + it('conversion functions are inverses', () => { + expect(base64ToUnicode(unicodeToBase64(unicodeString))).to.equal(unicodeString); + expect(unicodeToBase64(base64ToUnicode(base64String))).to.equal(base64String); + }); + + it('can handle and preserve multi-byte characters in original string', () => { + ['🐶', 'Καλό κορίτσι, Μάιζεϊ!', 'Of margir hundar! Ég geri ráð fyrir að ég þurfi stærra rúm.'].forEach(orig => { + expect(() => { + unicodeToBase64(orig); + }).not.to.throw; + expect(base64ToUnicode(unicodeToBase64(orig))).to.equal(orig); + }); + }); + + it('throws an error when given invalid input', () => { + expect(() => { + unicodeToBase64(null as any); + }).to.throw('Unable to convert to base64'); + expect(() => { + unicodeToBase64(undefined as any); + }).to.throw('Unable to convert to base64'); + expect(() => { + unicodeToBase64({} as any); + }).to.throw('Unable to convert to base64'); + + expect(() => { + base64ToUnicode(null as any); + }).to.throw('Unable to convert from base64'); + expect(() => { + base64ToUnicode(undefined as any); + }).to.throw('Unable to convert from base64'); + expect(() => { + base64ToUnicode({} as any); + }).to.throw('Unable to convert from base64'); + + // Note that by design, in node base64 encoding and decoding will accept any string, whether or not it's valid + // base64, by ignoring all invalid characters, including whitespace. Therefore, no wacky strings have been included + // here because they don't actually error. + }); +}); diff --git a/packages/utils/test/string.test.ts b/packages/utils/test/string.test.ts index 1ed7a2d23264..fe8bfda35481 100644 --- a/packages/utils/test/string.test.ts +++ b/packages/utils/test/string.test.ts @@ -50,6 +50,9 @@ describe('isMatchingPattern()', () => { }); }); +// NOTE: These tests are copied (and adapted for chai syntax) to `string.test.ts` in `@sentry/browser`. The +// base64-conversion functions have a different implementation in browser and node, so they're copied there to prove +// they work in a real live browser. If you make changes here, make sure to also port them over to that copy. describe('base64ToUnicode/unicodeToBase64', () => { const unicodeString = 'Dogs are great!'; const base64String = 'RG9ncyBhcmUgZ3JlYXQh'; From a957d9d7fd19367ee26028c8cdad85187618de3b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Fri, 9 Apr 2021 13:16:00 -0700 Subject: [PATCH 5/5] minor fixes --- packages/utils/src/string.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index e20e6556b5fc..12e11941e1a8 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -120,13 +120,13 @@ type GlobalWithBase64Helpers = { * @returns A base64-encoded version of the string */ export function unicodeToBase64(plaintext: string): string { - const globalObject = getGlobalObject() as GlobalWithBase64Helpers; + const globalObject = getGlobalObject(); // To account for the fact that different platforms use different character encodings natively, our `tracestate` // spec calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. try { if (typeof plaintext !== 'string') { - throw new Error(`Input must be a string. Received input of type ${typeof plaintext}.`); + throw new Error(`Input must be a string. Received input of type '${typeof plaintext}'.`); } // browser @@ -157,10 +157,10 @@ export function unicodeToBase64(plaintext: string): string { throw new SentryError('Neither `window.btoa` nor `global.Buffer` is defined.'); } catch (err) { // Cast to a string just in case we're given something else - const stringifiedInput = String(plaintext); + const stringifiedInput = JSON.stringify(plaintext); const errMsg = `Unable to convert to base64: ${ - stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput - }`; + stringifiedInput?.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }.`; throw new SentryError(`${errMsg}\nGot error: ${err}`); } } @@ -173,14 +173,14 @@ export function unicodeToBase64(plaintext: string): string { * @returns A Unicode string */ export function base64ToUnicode(base64String: string): string { - const globalObject = getGlobalObject() as GlobalWithBase64Helpers; + const globalObject = getGlobalObject(); // To account for the fact that different platforms use different character encodings natively, our `tracestate` spec // calls for all jsonified data to be encoded in UTF-8 bytes before being passed to the base64 encoder. So to reverse // the process, decode from base64 to bytes, then feed those bytes to a UTF-8 decoder. try { if (typeof base64String !== 'string') { - throw new Error(`Input must be a string. Received input of type ${typeof base64String}.`); + throw new Error(`Input must be a string. Received input of type '${typeof base64String}'.`); } // browser @@ -210,10 +210,10 @@ export function base64ToUnicode(base64String: string): string { throw new SentryError('Neither `window.atob` nor `global.Buffer` is defined.'); } catch (err) { // we cast to a string just in case we're given something else - const stringifiedInput = String(base64String); + const stringifiedInput = JSON.stringify(base64String); const errMsg = `Unable to convert from base64: ${ - stringifiedInput.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput - }`; + stringifiedInput?.length > 256 ? `${stringifiedInput.slice(0, 256)}...` : stringifiedInput + }.`; throw new SentryError(`${errMsg}\nGot error: ${err}`); } }