diff --git a/Firestore/core/src/bundle/bundle_serializer.cc b/Firestore/core/src/bundle/bundle_serializer.cc index 86942236791..cf120a5a52b 100644 --- a/Firestore/core/src/bundle/bundle_serializer.cc +++ b/Firestore/core/src/bundle/bundle_serializer.cc @@ -400,7 +400,8 @@ ResourcePath BundleSerializer::DecodeName(JsonReader& reader, } auto path = ResourcePath::FromString(document_name.get_ref()); - if (!rpc_serializer_.IsLocalResourceName(path)) { + if (!rpc_serializer_.IsLocalResourceName(path) || path.size() < 5 || + path[4] != "documents") { reader.Fail("Resource name is not valid for current instance: " + path.CanonicalString()); return {}; @@ -647,6 +648,11 @@ BundledDocumentMetadata BundleSerializer::DecodeDocumentMetadata( if (!reader.ok()) { return {}; } + if (!DocumentKey::IsDocumentKey(path)) { + reader.Fail("Resource name is not a valid document key: " + + path.CanonicalString()); + return {}; + } DocumentKey key = DocumentKey(path); SnapshotVersion read_time = DecodeSnapshotVersion( @@ -678,6 +684,11 @@ BundleDocument BundleSerializer::DecodeDocument(JsonReader& reader, if (!reader.ok()) { return {}; } + if (!DocumentKey::IsDocumentKey(path)) { + reader.Fail("Resource name is not a valid document key: " + + path.CanonicalString()); + return {}; + } DocumentKey key = DocumentKey(path); SnapshotVersion update_time = DecodeSnapshotVersion( diff --git a/Firestore/core/src/remote/serializer.cc b/Firestore/core/src/remote/serializer.cc index 932064a01a9..ea86a1334c8 100644 --- a/Firestore/core/src/remote/serializer.cc +++ b/Firestore/core/src/remote/serializer.cc @@ -1521,7 +1521,8 @@ bool Serializer::IsLocalResourceName(const ResourcePath& path) const { bool Serializer::IsLocalDocumentKey(absl::string_view path) const { auto resource = ResourcePath::FromStringView(path); - return IsLocalResourceName(resource) && + return IsLocalResourceName(resource) && resource.size() >= 5 && + resource[4] == "documents" && DocumentKey::IsDocumentKey(resource.PopFirst(5)); } diff --git a/Firestore/core/test/unit/bundle/bundle_serializer_test.cc b/Firestore/core/test/unit/bundle/bundle_serializer_test.cc index 72d788c7832..ce59f477266 100644 --- a/Firestore/core/test/unit/bundle/bundle_serializer_test.cc +++ b/Firestore/core/test/unit/bundle/bundle_serializer_test.cc @@ -1166,6 +1166,39 @@ TEST_F(BundleSerializerTest, DecodeInvalidBundledDocumentMetadataFails) { } } +TEST_F(BundleSerializerTest, DecodeNonDocumentKeyResourceNameFails) { + // A resource name that points at a collection (odd number of segments after + // the `documents` prefix) passes the local-resource-name check but is not a + // valid `DocumentKey`. It must be rejected gracefully instead of tripping the + // `HARD_ASSERT` inside the `DocumentKey` constructor. + { + ProtoDocument document = TestDocument(ProtoValue()); + document.set_name(FullPath("bundle")); + std::string json_string; + MessageToJsonString(document, &json_string); + + JsonReader reader; + bundle_serializer.DecodeDocument(reader, Parse(json_string)); + EXPECT_NOT_OK(reader.status()); + } + + { + ProtoBundledDocumentMetadata metadata; + metadata.set_name(FullPath("bundle")); + metadata.set_exists(true); + google::protobuf::Timestamp t1; + t1.set_seconds(0); + t1.set_nanos(0); + *metadata.mutable_read_time() = t1; + std::string json_string; + MessageToJsonString(metadata, &json_string); + + JsonReader reader; + bundle_serializer.DecodeDocumentMetadata(reader, Parse(json_string)); + EXPECT_NOT_OK(reader.status()); + } +} + TEST_F(BundleSerializerTest, DecodeTargetWithoutImplicitOrderByOnName) { std::string json( R"({"name":"myNamedQuery",