Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve decodeBase64() to throw on invalid input rather than silently accept it #7019

Merged
merged 8 commits into from
Feb 11, 2023
8 changes: 8 additions & 0 deletions common/api-review/util.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ export function createSubscribe<T>(executor: Executor<T>, onNoObservers?: Execut
// @public
export const decode: (token: string) => DecodedToken;

// Warning: (ae-missing-release-tag) "DecodeBase64StringError" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export class DecodeBase64StringError extends Error {
// (undocumented)
readonly name = "DecodeBase64StringError";
}

// Warning: (ae-missing-release-tag) "deepCopy" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
Expand Down
3 changes: 2 additions & 1 deletion packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ import { Document } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { Mutation } from '../model/mutation';
import { toByteStreamReader } from '../platform/byte_stream_reader';
import { newSerializer, newTextEncoder } from '../platform/serializer';
import { newSerializer } from '../platform/serializer';
import { newTextEncoder } from '../platform/text_serializer';
import { Datastore } from '../remote/datastore';
import {
canUseNetwork,
Expand Down
13 changes: 12 additions & 1 deletion packages/firestore/src/platform/base64.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,24 @@
* limitations under the License.
*/

import { Base64DecodeError } from '../util/base64_decode_error';

// This file is only used under ts-node.
// eslint-disable-next-line @typescript-eslint/no-require-imports
const platform = require(`./${process.env.TEST_PLATFORM ?? 'node'}/base64`);

/** Converts a Base64 encoded string to a binary string. */
export function decodeBase64(encoded: string): string {
return platform.decodeBase64(encoded);
const decoded = platform.decodeBase64(encoded);

// A quick sanity check as node and rn will not throw error if input is an
// invalid base64 string, e.g., "A===".
const expectedEncodedLength = 4 * Math.ceil(decoded.length / 3);
if (encoded.length !== expectedEncodedLength) {
throw new Base64DecodeError('Invalid base64 string');
}

return decoded;
}

/** Converts a binary string to a Base64 encoded string. */
Expand Down
12 changes: 11 additions & 1 deletion packages/firestore/src/platform/browser/base64.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,19 @@
* limitations under the License.
*/

import { Base64DecodeError } from '../../util/base64_decode_error';

/** Converts a Base64 encoded string to a binary string. */
export function decodeBase64(encoded: string): string {
return atob(encoded);
try {
return atob(encoded);
} catch (e) {
if (e instanceof DOMException) {
throw new Base64DecodeError('Invalid base64 string: ' + e);
} else {
throw e;
}
}
}

/** Converts a binary string to a Base64 encoded string. */
Expand Down
14 changes: 0 additions & 14 deletions packages/firestore/src/platform/browser/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,3 @@ import { JsonProtoSerializer } from '../../remote/serializer';
export function newSerializer(databaseId: DatabaseId): JsonProtoSerializer {
return new JsonProtoSerializer(databaseId, /* useProto3Json= */ true);
}

/**
* An instance of the Platform's 'TextEncoder' implementation.
*/
export function newTextEncoder(): TextEncoder {
return new TextEncoder();
}

/**
* An instance of the Platform's 'TextDecoder' implementation.
*/
export function newTextDecoder(): TextDecoder {
return new TextDecoder('utf-8');
}
30 changes: 30 additions & 0 deletions packages/firestore/src/platform/browser/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* An instance of the Platform's 'TextEncoder' implementation.
*/
export function newTextEncoder(): TextEncoder {
return new TextEncoder();
}

/**
* An instance of the Platform's 'TextDecoder' implementation.
*/
export function newTextDecoder(): TextDecoder {
return new TextDecoder('utf-8');
}
18 changes: 18 additions & 0 deletions packages/firestore/src/platform/browser_lite/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from '../browser/text_serializer';
16 changes: 0 additions & 16 deletions packages/firestore/src/platform/node/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,9 @@
*/

/** Return the Platform-specific serializer monitor. */
import { TextDecoder, TextEncoder } from 'util';

import { DatabaseId } from '../../core/database_info';
import { JsonProtoSerializer } from '../../remote/serializer';

export function newSerializer(databaseId: DatabaseId): JsonProtoSerializer {
return new JsonProtoSerializer(databaseId, /* useProto3Json= */ false);
}

/**
* An instance of the Platform's 'TextEncoder' implementation.
*/
export function newTextEncoder(): TextEncoder {
return new TextEncoder();
}

/**
* An instance of the Platform's 'TextDecoder' implementation.
*/
export function newTextDecoder(): TextDecoder {
return new TextDecoder('utf-8');
}
32 changes: 32 additions & 0 deletions packages/firestore/src/platform/node/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { TextDecoder, TextEncoder } from 'util';

/**
* An instance of the Platform's 'TextEncoder' implementation.
*/
export function newTextEncoder(): TextEncoder {
return new TextEncoder();
}

/**
* An instance of the Platform's 'TextDecoder' implementation.
*/
export function newTextDecoder(): TextDecoder {
return new TextDecoder('utf-8');
}
18 changes: 18 additions & 0 deletions packages/firestore/src/platform/node_lite/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from '../browser_lite/text_serializer';
26 changes: 18 additions & 8 deletions packages/firestore/src/platform/rn/base64.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,31 @@
* limitations under the License.
*/

import { base64 } from '@firebase/util';
import { base64, DecodeBase64StringError } from '@firebase/util';

import { Base64DecodeError } from '../../util/base64_decode_error';

// WebSafe uses a different URL-encoding safe alphabet that doesn't match
// the encoding used on the backend.
const WEB_SAFE = false;

/** Converts a Base64 encoded string to a binary string. */
export function decodeBase64(encoded: string): string {
return String.fromCharCode.apply(
null,
// We use `decodeStringToByteArray()` instead of `decodeString()` since
// `decodeString()` returns Unicode strings, which doesn't match the values
// returned by `atob()`'s Latin1 representation.
base64.decodeStringToByteArray(encoded, WEB_SAFE)
);
try {
return String.fromCharCode.apply(
null,
// We use `decodeStringToByteArray()` instead of `decodeString()` since
// `decodeString()` returns Unicode strings, which doesn't match the values
// returned by `atob()`'s Latin1 representation.
base64.decodeStringToByteArray(encoded, WEB_SAFE)
);
} catch (e) {
if (e instanceof DecodeBase64StringError) {
throw new Base64DecodeError('Invalid base64 string: ' + e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't followed these all the way up the chain but will they eventually get wrapped in a FirestoreError?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the error won't be re-thrown as a FirestoreError. The specific use case is a future optimization that involves decoding base64. If, for some reason, we get invalid base64 from the backend then we will just ignore it and disable the optimization. We don't expect to ever get invalid base64, but it could theoretically happen and if it ever does we don't want to crash.

The problem was that it was previously not possible to reliably detect an invalid base64 string. This PR fixes that by having the new Base64DecodeError always thrown if an invalid base64 string is given as input. The future code will catch this exception and use it to disable the optimization.

} else {
throw e;
}
}
}

/** Converts a binary string to a Base64 encoded string. */
Expand Down
6 changes: 1 addition & 5 deletions packages/firestore/src/platform/rn/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,4 @@
* limitations under the License.
*/

export {
newSerializer,
newTextEncoder,
newTextDecoder
} from '../browser/serializer';
export { newSerializer } from '../browser/serializer';
18 changes: 18 additions & 0 deletions packages/firestore/src/platform/rn/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export { newTextEncoder, newTextDecoder } from '../browser/text_serializer';
18 changes: 18 additions & 0 deletions packages/firestore/src/platform/rn_lite/text_serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from '../browser_lite/text_serializer';
32 changes: 0 additions & 32 deletions packages/firestore/src/platform/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,13 @@
* limitations under the License.
*/

import { isNode, isReactNative } from '@firebase/util';

import { DatabaseId } from '../core/database_info';
import { JsonProtoSerializer } from '../remote/serializer';

import * as browser from './browser/serializer';
import * as node from './node/serializer';
import * as rn from './rn/serializer';

// This file is only used under ts-node.
// eslint-disable-next-line @typescript-eslint/no-require-imports
const platform = require(`./${process.env.TEST_PLATFORM ?? 'node'}/serializer`);

export function newSerializer(databaseId: DatabaseId): JsonProtoSerializer {
return platform.newSerializer(databaseId);
}

/**
* An instance of the Platform's 'TextEncoder' implementation.
*/
export function newTextEncoder(): TextEncoder {
if (isNode()) {
return node.newTextEncoder();
} else if (isReactNative()) {
return rn.newTextEncoder();
} else {
return browser.newTextEncoder();
}
}

/**
* An instance of the Platform's 'TextDecoder' implementation.
*/
export function newTextDecoder(): TextDecoder {
if (isNode()) {
return node.newTextDecoder() as TextDecoder;
} else if (isReactNative()) {
return rn.newTextDecoder();
} else {
return browser.newTextDecoder();
}
}
Loading