Skip to content

Commit 633fa55

Browse files
authored
Explicitly check that document and collection IDs given to the public API aren't empty (#8248)
Currently, the error would look like `Document references must have an even number of segments, but has 0`. Fixes #8218.
1 parent 3bde4df commit 633fa55

File tree

5 files changed

+49
-6
lines changed

5 files changed

+49
-6
lines changed

Firestore/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# Unreleased
2+
- [changed] Passing in an empty document ID, collection group ID, or collection
3+
path will now result in a more readable error (#8218).
4+
15
# v7.9.0
26
- [feature] Added support for Firestore Bundles via
37
`FIRFirestore.loadBundle`, `FIRFirestore.loadBundleStream` and

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,18 @@ - (void)testNilCollectionPathsFail {
110110
FSTAssertThrows([baseDocRef collectionWithPath:nil], nilError);
111111
}
112112

113+
- (void)testEmptyCollectionPathsFail {
114+
FIRDocumentReference *baseDocRef = [self.db documentWithPath:@"foo/bar"];
115+
NSString *emptyError = @"Collection path cannot be empty.";
116+
FSTAssertThrows([self.db collectionWithPath:@""], emptyError);
117+
FSTAssertThrows([baseDocRef collectionWithPath:@""], emptyError);
118+
}
119+
113120
- (void)testWrongLengthCollectionPathsFail {
114121
FIRDocumentReference *baseDocRef = [self.db documentWithPath:@"foo/bar"];
115-
NSArray *badAbsolutePaths = @[ @"foo/bar", @"foo/bar/baz/quu" ];
116-
NSArray *badRelativePaths = @[ @"", @"baz/quu" ];
117-
NSArray *badPathLengths = @[ @2, @4 ];
122+
NSArray *badAbsolutePaths = @[ @"foo/bar/baz/quu", @"foo/bar/baz/quu/x/y" ];
123+
NSArray *badRelativePaths = @[ @"baz/quu", @"baz/quu/x/y" ];
124+
NSArray *badPathLengths = @[ @4, @6 ];
118125
NSString *errorFormat = @"Invalid collection reference. Collection references must have an odd "
119126
@"number of segments, but %@ has %@";
120127
for (NSUInteger i = 0; i < badAbsolutePaths.count; i++) {
@@ -125,18 +132,35 @@ - (void)testWrongLengthCollectionPathsFail {
125132
}
126133
}
127134

135+
- (void)testNilCollectionGroupPathsFail {
136+
NSString *nilError = @"Collection ID cannot be nil.";
137+
FSTAssertThrows([self.db collectionGroupWithID:nil], nilError);
138+
}
139+
140+
- (void)testEmptyCollectionGroupPathsFail {
141+
NSString *emptyError = @"Collection ID cannot be empty.";
142+
FSTAssertThrows([self.db collectionGroupWithID:@""], emptyError);
143+
}
144+
128145
- (void)testNilDocumentPathsFail {
129146
FIRCollectionReference *baseCollectionRef = [self.db collectionWithPath:@"foo"];
130147
NSString *nilError = @"Document path cannot be nil.";
131148
FSTAssertThrows([self.db documentWithPath:nil], nilError);
132149
FSTAssertThrows([baseCollectionRef documentWithPath:nil], nilError);
133150
}
134151

152+
- (void)testEmptyDocumentPathsFail {
153+
FIRCollectionReference *baseCollectionRef = [self.db collectionWithPath:@"foo"];
154+
NSString *emptyError = @"Document path cannot be empty.";
155+
FSTAssertThrows([self.db documentWithPath:@""], emptyError);
156+
FSTAssertThrows([baseCollectionRef documentWithPath:@""], emptyError);
157+
}
158+
135159
- (void)testWrongLengthDocumentPathsFail {
136160
FIRCollectionReference *baseCollectionRef = [self.db collectionWithPath:@"foo"];
137-
NSArray *badAbsolutePaths = @[ @"foo", @"foo/bar/baz" ];
138-
NSArray *badRelativePaths = @[ @"", @"bar/baz" ];
139-
NSArray *badPathLengths = @[ @1, @3 ];
161+
NSArray *badAbsolutePaths = @[ @"foo/bar/baz", @"foo/bar/baz/x/y" ];
162+
NSArray *badRelativePaths = @[ @"bar/baz", @"bar/baz/x/y" ];
163+
NSArray *badPathLengths = @[ @3, @5 ];
140164
NSString *errorFormat = @"Invalid document reference. Document references must have an even "
141165
@"number of segments, but %@ has %@";
142166
for (NSUInteger i = 0; i < badAbsolutePaths.count; i++) {

Firestore/Source/API/FIRCollectionReference.mm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ - (FIRDocumentReference *)documentWithPath:(NSString *)documentPath {
104104
if (!documentPath) {
105105
ThrowInvalidArgument("Document path cannot be nil.");
106106
}
107+
if (!documentPath.length) {
108+
ThrowInvalidArgument("Document path cannot be empty.");
109+
}
107110
DocumentReference child = self.reference.Document(util::MakeString(documentPath));
108111
return [[FIRDocumentReference alloc] initWithReference:std::move(child)];
109112
}

Firestore/Source/API/FIRDocumentReference.mm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ - (FIRCollectionReference *)collectionWithPath:(NSString *)collectionPath {
129129
if (!collectionPath) {
130130
ThrowInvalidArgument("Collection path cannot be nil.");
131131
}
132+
if (!collectionPath.length) {
133+
ThrowInvalidArgument("Collection path cannot be empty.");
134+
}
132135

133136
CollectionReference child =
134137
_documentReference.GetCollectionReference(util::MakeString(collectionPath));

Firestore/Source/API/FIRFirestore.mm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ - (FIRCollectionReference *)collectionWithPath:(NSString *)collectionPath {
198198
if (!collectionPath) {
199199
ThrowInvalidArgument("Collection path cannot be nil.");
200200
}
201+
if (!collectionPath.length) {
202+
ThrowInvalidArgument("Collection path cannot be empty.");
203+
}
201204
if ([collectionPath containsString:@"//"]) {
202205
ThrowInvalidArgument("Invalid path (%s). Paths must not contain // in them.", collectionPath);
203206
}
@@ -210,6 +213,9 @@ - (FIRDocumentReference *)documentWithPath:(NSString *)documentPath {
210213
if (!documentPath) {
211214
ThrowInvalidArgument("Document path cannot be nil.");
212215
}
216+
if (!documentPath.length) {
217+
ThrowInvalidArgument("Document path cannot be empty.");
218+
}
213219
if ([documentPath containsString:@"//"]) {
214220
ThrowInvalidArgument("Invalid path (%s). Paths must not contain // in them.", documentPath);
215221
}
@@ -222,6 +228,9 @@ - (FIRQuery *)collectionGroupWithID:(NSString *)collectionID {
222228
if (!collectionID) {
223229
ThrowInvalidArgument("Collection ID cannot be nil.");
224230
}
231+
if (!collectionID.length) {
232+
ThrowInvalidArgument("Collection ID cannot be empty.");
233+
}
225234
if ([collectionID containsString:@"/"]) {
226235
ThrowInvalidArgument("Invalid collection ID (%s). Collection IDs must not contain / in them.",
227236
collectionID);

0 commit comments

Comments
 (0)