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

Remove JS Input Validation #3939

Merged
merged 4 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
4 changes: 4 additions & 0 deletions packages/firestore/exp/test/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { UntypedFirestoreDataConverter } from '../../src/api/user_data_reader';
import { isPartialObserver, PartialObserver } from '../../src/api/observer';
import { isPlainObject } from '../../src/util/input_validation';
import { Compat } from '../../src/compat/compat';
import { validateSetOptions } from '../../src/api/database';

export { GeoPoint, Timestamp } from '../index';
export { FieldValue } from '../../src/compat/field_value';
Expand Down Expand Up @@ -193,6 +194,7 @@ export class Transaction
options?: legacy.SetOptions
): Transaction {
if (options) {
validateSetOptions('Transaction.set', options);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still need to validate that merge and mergeFields isn't used together.

this._delegate.set(documentRef._delegate, unwrap(data), options);
} else {
this._delegate.set(documentRef._delegate, unwrap(data));
Expand Down Expand Up @@ -245,6 +247,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 +327,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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this make a little bit of sense, it is absent on Android.

// 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
Loading