Skip to content

Commit

Permalink
Remove JS Input Validation (#3939)
Browse files Browse the repository at this point in the history
  • Loading branch information
schmidt-sebastian committed Oct 20, 2020
1 parent a5768b0 commit 4b540f9
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 1,062 deletions.
6 changes: 6 additions & 0 deletions .changeset/silver-dolls-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"firebase": major
"@firebase/firestore": major
---

This releases removes all input validation. Please use our TypeScript types to validate API usage.
2 changes: 1 addition & 1 deletion packages/firestore/exp/src/api/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
parseUpdateVarargs
} from '../../../src/api/user_data_reader';
import { debugAssert } from '../../../src/util/assert';
import { cast } from '../../../lite/src/api/util';
import { cast } from '../../../src/util/input_validation';
import { DocumentSnapshot, QuerySnapshot } from './snapshot';
import {
applyFirestoreDataConverter,
Expand Down
8 changes: 7 additions & 1 deletion packages/firestore/exp/test/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ import {
} from '../../exp/index';
import { UntypedFirestoreDataConverter } from '../../src/api/user_data_reader';
import { isPartialObserver, PartialObserver } from '../../src/api/observer';
import { isPlainObject } from '../../src/util/input_validation';
import {
isPlainObject,
validateSetOptions
} from '../../src/util/input_validation';
import { Compat } from '../../src/compat/compat';

export { GeoPoint, Timestamp } from '../index';
Expand Down Expand Up @@ -193,6 +196,7 @@ export class Transaction
options?: legacy.SetOptions
): Transaction {
if (options) {
validateSetOptions('Transaction.set', options);
this._delegate.set(documentRef._delegate, unwrap(data), options);
} else {
this._delegate.set(documentRef._delegate, unwrap(data));
Expand Down Expand Up @@ -245,6 +249,7 @@ export class WriteBatch
options?: legacy.SetOptions
): WriteBatch {
if (options) {
validateSetOptions('WriteBatch.set', options);
this._delegate.set(documentRef._delegate, unwrap(data), options);
} else {
this._delegate.set(documentRef._delegate, unwrap(data));
Expand Down Expand Up @@ -324,6 +329,7 @@ export class DocumentReference<T = legacy.DocumentData>

set(data: Partial<T>, options?: legacy.SetOptions): Promise<void> {
if (options) {
validateSetOptions('DocumentReference.set', options);
return setDoc(this._delegate, unwrap(data), options);
} else {
return setDoc(this._delegate, unwrap(data));
Expand Down
3 changes: 0 additions & 3 deletions packages/firestore/lite/src/api/field_value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { validateAtLeastNumberOfArgs } from '../../../src/util/input_validation';
import {
ArrayRemoveFieldValueImpl,
ArrayUnionFieldValueImpl,
Expand Down Expand Up @@ -69,7 +68,6 @@ export function serverTimestamp(): FieldValue {
* `updateDoc()`.
*/
export function arrayUnion(...elements: unknown[]): FieldValue {
validateAtLeastNumberOfArgs('arrayUnion()', arguments, 1);
// NOTE: We don't actually parse the data until it's used in set() or
// update() since we'd need the Firestore instance to do this.
return new ArrayUnionFieldValueImpl('arrayUnion', elements);
Expand All @@ -87,7 +85,6 @@ export function arrayUnion(...elements: unknown[]): FieldValue {
* `updateDoc()`
*/
export function arrayRemove(...elements: unknown[]): FieldValue {
validateAtLeastNumberOfArgs('arrayRemove()', arguments, 1);
// NOTE: We don't actually parse the data until it's used in set() or
// update() since we'd need the Firestore instance to do this.
return new ArrayRemoveFieldValueImpl('arrayRemove', elements);
Expand Down
24 changes: 3 additions & 21 deletions packages/firestore/lite/src/api/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ import { FieldPath } from './field_path';
import {
validateCollectionPath,
validateDocumentPath,
validateExactNumberOfArgs,
validateNonEmptyArgument,
validatePositiveNumber
} from '../../../src/util/input_validation';
import { newSerializer } from '../../../src/platform/serializer';
Expand Down Expand Up @@ -335,8 +335,6 @@ export function where(
opStr: WhereFilterOp,
value: unknown
): QueryConstraint {
// TODO(firestorelite): Consider validating the enum strings (note that
// TypeScript does not support passing invalid values).
const op = opStr as Operator;
const field = fieldPathFromArgument('where', fieldPath);
return new QueryFilterConstraint(field, op, value);
Expand Down Expand Up @@ -381,8 +379,6 @@ export function orderBy(
fieldPath: string | FieldPath,
directionStr: OrderByDirection = 'asc'
): QueryConstraint {
// TODO(firestorelite): Consider validating the enum strings (note that
// TypeScript does not support passing invalid values).
const direction = directionStr as Direction;
const path = fieldPathFromArgument('orderBy', fieldPath);
return new QueryOrderByConstraint(path, direction);
Expand Down Expand Up @@ -413,7 +409,7 @@ class QueryLimitConstraint extends QueryConstraint {
* @return The created `Query`.
*/
export function limit(limit: number): QueryConstraint {
validatePositiveNumber('limit', 1, limit);
validatePositiveNumber('limit', limit);
return new QueryLimitConstraint('limit', limit, LimitType.First);
}

Expand All @@ -427,7 +423,7 @@ export function limit(limit: number): QueryConstraint {
* @return The created `Query`.
*/
export function limitToLast(limit: number): QueryConstraint {
validatePositiveNumber('limitToLast', 1, limit);
validatePositiveNumber('limitToLast', limit);
return new QueryLimitConstraint('limitToLast', limit, LimitType.Last);
}

Expand Down Expand Up @@ -597,7 +593,6 @@ function newQueryBoundFromDocOrFields<T>(
before: boolean
): Bound {
if (docOrFields[0] instanceof DocumentSnapshot) {
validateExactNumberOfArgs(methodName, docOrFields, 1);
return newQueryBoundFromDocument(
query._query,
query.firestore._databaseId,
Expand Down Expand Up @@ -1234,16 +1229,3 @@ export function newUserDataReader(
serializer
);
}

function validateNonEmptyArgument(
functionName: string,
argumentName: string,
argument?: string
): asserts argument is string {
if (!argument) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Function ${functionName}() cannot be called with an empty ${argumentName}.`
);
}
}
46 changes: 0 additions & 46 deletions packages/firestore/lite/src/api/util.ts

This file was deleted.

13 changes: 0 additions & 13 deletions packages/firestore/src/api/blob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@

import { isBase64Available } from '../platform/base64';
import { Code, FirestoreError } from '../util/error';
import {
invalidClassError,
validateArgType,
validateExactNumberOfArgs
} from '../util/input_validation';
import { ByteString } from '../util/byte_string';
import { Bytes } from '../../lite/src/api/bytes';

Expand Down Expand Up @@ -57,8 +52,6 @@ function assertBase64Available(): void {
*/
export class Blob extends Bytes {
static fromBase64String(base64: string): Blob {
validateExactNumberOfArgs('Blob.fromBase64String', arguments, 1);
validateArgType('Blob.fromBase64String', 'string', 1, base64);
assertBase64Available();
try {
return new Blob(ByteString.fromBase64String(base64));
Expand All @@ -71,22 +64,16 @@ export class Blob extends Bytes {
}

static fromUint8Array(array: Uint8Array): Blob {
validateExactNumberOfArgs('Blob.fromUint8Array', arguments, 1);
assertUint8ArrayAvailable();
if (!(array instanceof Uint8Array)) {
throw invalidClassError('Blob.fromUint8Array', 'Uint8Array', 1, array);
}
return new Blob(ByteString.fromUint8Array(array));
}

toBase64(): string {
validateExactNumberOfArgs('Blob.toBase64', arguments, 0);
assertBase64Available();
return super.toBase64();
}

toUint8Array(): Uint8Array {
validateExactNumberOfArgs('Blob.toUint8Array', arguments, 0);
assertUint8ArrayAvailable();
return super.toUint8Array();
}
Expand Down

0 comments on commit 4b540f9

Please sign in to comment.