Skip to content

PHPLIB-1927: Reject "." and NUL bytes in database and collection names - #1967

Merged
kevinAlbs merged 1 commit into
mongodb:v2.4from
GromNaN:PHPLIB-1927-v2.4
Aug 27, 2026
Merged

PHPLIB-1927: Reject "." and NUL bytes in database and collection names#1967
kevinAlbs merged 1 commit into
mongodb:v2.4from
GromNaN:PHPLIB-1927-v2.4

Conversation

@GromNaN

@GromNaN GromNaN commented Aug 27, 2026

Copy link
Copy Markdown
Member

Database and collection names were only checked for being non-empty. A "." in a database name, or a NUL byte in a database or collection name, shifts the namespace split performed by the server, so operations end up targeting a different database or collection than the caller intended.

Reject "." and NUL bytes in database names, and NUL bytes in collection names. Dots remain legal in collection names.

Add a create_namespace() helper that validates both names and returns the concatenated namespace, and use it in Collection, Database, and all Operation classes that build a namespace by concatenating database and collection names (BulkWrite, Delete, Find, InsertMany, InsertOne, RenameCollection, Update).

Move the shared invalid name data provider to the base TestCase, and add prose test 17 for database and collection name validation.

Database and collection names were only checked for being non-empty. A "." in a database name, or a NUL byte in a database or collection name, shifts the namespace split performed by the server, so operations end up targeting a different database or collection than the caller intended.

Reject "." and NUL bytes in database names, and NUL bytes in collection names. Dots remain legal in collection names.

Add a create_namespace() helper that validates both names and returns the concatenated namespace, and use it in Collection, Database, and all Operation classes that build a namespace by concatenating database and collection names (BulkWrite, Delete, Find, InsertMany, InsertOne, RenameCollection, Update).

Move the shared invalid name data provider to the base TestCase, and add prose test 17 for database and collection name validation.
@GromNaN
GromNaN requested a review from a team as a code owner August 27, 2026 14:28
@GromNaN
GromNaN requested review from paulinevos and a lite review from Copilot August 27, 2026 14:28
@GromNaN
GromNaN enabled auto-merge (squash) August 27, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens namespace handling by validating database/collection names against characters that can change how the server splits namespaces, preventing operations from silently targeting an unintended database or collection.

Changes:

  • Added centralized namespace construction via MongoDB\create_namespace() and updated CRUD operations to use it instead of string concatenation.
  • Tightened Database and Collection constructors to reject . and NUL bytes in database names, and NUL bytes in collection names (dots still allowed in collection names).
  • Expanded test coverage with shared invalid-name providers and CRUD prose test 17.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/TestCase.php Adds shared data provider for invalid db/collection name combinations.
tests/SpecTests/Crud/Prose17_DatabaseAndCollectionNameValidationTest.php New prose spec test ensuring invalid names are rejected through public APIs (including ClientBulkWrite).
tests/Operation/UpdateTest.php Verifies Update rejects invalid db/collection names.
tests/Operation/RenameCollectionTest.php Verifies RenameCollection rejects invalid “from/to” db/collection names.
tests/Operation/InsertOneTest.php Verifies InsertOne rejects invalid db/collection names.
tests/Operation/InsertManyTest.php Verifies InsertMany rejects invalid db/collection names.
tests/Operation/FindTest.php Verifies Find rejects invalid db/collection names.
tests/Operation/DeleteTest.php Verifies Delete rejects invalid db/collection names.
tests/Operation/BulkWriteTest.php Verifies BulkWrite rejects invalid db/collection names.
tests/Database/DatabaseFunctionalTest.php Extends invalid database-name cases to include . and NUL byte.
tests/Collection/CollectionFunctionalTest.php Splits invalid db vs collection validation and explicitly asserts dots remain allowed in collection names.
src/Operation/Update.php Uses create_namespace() and stores a validated namespace for execution.
src/Operation/RenameCollection.php Uses create_namespace() for both from and to namespaces.
src/Operation/InsertOne.php Uses create_namespace() to ensure validated namespace for writes.
src/Operation/InsertMany.php Uses create_namespace() to ensure validated namespace for writes.
src/Operation/Find.php Uses create_namespace() to ensure validated namespace for queries.
src/Operation/Delete.php Uses create_namespace() to ensure validated namespace for writes.
src/Operation/BulkWrite.php Uses create_namespace() to ensure validated namespace for bulk writes.
src/functions.php Introduces create_namespace() helper with validation and namespace concatenation.
src/Database.php Rejects . and NUL bytes in database names at construction time.
src/Collection.php Rejects . and NUL bytes in database names; rejects NUL bytes in collection names.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/functions.php
Comment thread src/functions.php
Comment thread src/Database.php
Comment thread src/Collection.php
Comment thread src/Collection.php
@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
5968 1 5967 1168
View the full list of 7 ❄️ flaky test(s)
MongoDB.Tests.Operation.WatchFunctionalTest::testResumeAfterOption with data set "Codec"

Flake rate in main: 12.50% (Passed 7 times, Failed 1 times)

Stack Traces | 2.13s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testResumeAfterOption with data set "Codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A90AAE2000000032B042C0100296E5A10046ABF2B39C170470B986F9AE43D79D70E463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A90AAE2000000032B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:868
MongoDB.Tests.Operation.WatchFunctionalTest::testResumeAfterOption with data set "No codec"

Flake rate in main: 11.76% (Passed 30 times, Failed 4 times)

Stack Traces | 2.14s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testResumeAfterOption with data set "No codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A909CE3000000012B042C0100296E5A1004B5E99AA3E0EC43B4BECA18FCD897F7E2463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A909CE3000000012B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:868
MongoDB.Tests.Operation.WatchFunctionalTest::testRewindExtractsResumeTokenAndNextResumes with data set "Codec"

Flake rate in main: 9.09% (Passed 10 times, Failed 1 times)

Stack Traces | 2.13s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testRewindExtractsResumeTokenAndNextResumes with data set "Codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A90997D000000012B042C0100296E5A100462233D40480C4ED39A75DB61DF005E07463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A90997D000000012B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:814
MongoDB.Tests.Operation.WatchFunctionalTest::testRewindExtractsResumeTokenAndNextResumes with data set "No codec"

Flake rate in main: 8.33% (Passed 11 times, Failed 1 times)

Stack Traces | 2.14s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testRewindExtractsResumeTokenAndNextResumes with data set "No codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A9051CF000000032B042C0100296E5A100420F71F25DCCD4C56A533E1A002413A3E463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A9051CF000000032B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:814
MongoDB.Tests.Operation.WatchFunctionalTest::testStartAfterOption with data set "Codec"

Flake rate in main: 7.89% (Passed 35 times, Failed 3 times)

Stack Traces | 2.13s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testStartAfterOption with data set "Codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A909CE8000000012B042C0100296E5A1004937C71365E084D23919AF15E3DCA086F463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A909CE8000000012B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:916
MongoDB.Tests.Operation.WatchFunctionalTest::testStartAfterOption with data set "No codec"

Flake rate in main: 3.33% (Passed 29 times, Failed 1 times)

Stack Traces | 2.14s run time
MongoDB\Tests\Operation\WatchFunctionalTest::testStartAfterOption with data set "No codec"
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'{ "_data" : "826A905249000000032B042C0100296E5A1004F3EE02EEA381461AA6325A3C55B81372463C6F7065726174696F6E54797065003C696E736572740046646F63756D656E744B657900461E5F6964002B02000004" }'
+'{ "_data" : "826A905249000000032B0429296E1404" }'

.../src/tests/TestCase.php:108
.../tests/Operation/WatchFunctionalTest.php:916
MongoDB.Tests.UnifiedSpecTests.UnifiedSpecTest::testSessions with data set "sessions/implicit sessions default causal consistency: afterClusterTime is not sent on retried read in implicit session when readConcern level is snapshot"

Flake rate in main: 11.11% (Passed 8 times, Failed 1 times)

Stack Traces | 0.725s run time
MongoDB\Tests\UnifiedSpecTests\UnifiedSpecTest::testSessions with data set "sessions/implicit sessions default causal consistency: afterClusterTime is not sent on retried read in implicit session when readConcern level is snapshot"
Failed asserting that expected value matches actual value.
$actual count is 0, expected 1
--- Expected
+++ Actual
@@ @@
-MongoDB\Model\BSONArray Object #149365 (
-    0 => MongoDB\Model\BSONDocument Object #149257 (
-        '_id' => 1,
-        'x' => 'snapshot',
-    ),
-)
+MongoDB\Model\BSONArray Object #149231 ()

.../UnifiedSpecTests/Constraint/Matches.php:112
.../tests/UnifiedSpecTests/ExpectedResult.php:48
.../tests/UnifiedSpecTests/Operation.php:164
.../tests/UnifiedSpecTests/UnifiedTestRunner.php:214
.../tests/UnifiedSpecTests/UnifiedTestRunner.php:106
.../tests/UnifiedSpecTests/UnifiedSpecTest.php:299

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@GromNaN
GromNaN disabled auto-merge August 27, 2026 14:39
@kevinAlbs
kevinAlbs merged commit b72cc2e into mongodb:v2.4 Aug 27, 2026
28 of 31 checks passed
@GromNaN
GromNaN deleted the PHPLIB-1927-v2.4 branch August 27, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants