Skip to content

Commit a36b867

Browse files
committed
CBL-6952: Revise Database name validation in C4Address.fromURL()
We won't check the validity of the database name in the URL. The format of the name should be managed as the name is created.
1 parent 6ba6809 commit a36b867

File tree

3 files changed

+14
-28
lines changed

3 files changed

+14
-28
lines changed

Replicator/c4Replicator.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ bool C4Replicator::isValidDatabaseName(slice dbName) noexcept {
139139
bool C4Address::isValidRemote(slice dbName, C4Error* outError) const noexcept {
140140
slice message;
141141
if ( !isValidReplicatorScheme(scheme) ) message = "Invalid replication URL scheme (use ws: or wss:)"_sl;
142-
else if ( !C4Replicator::isValidDatabaseName(dbName) )
143-
message = "Invalid or missing remote database name"_sl;
142+
else if ( dbName.empty() )
143+
message = "Missing remote database name"_sl;
144144
else if ( hostname.size == 0 || port == 0 )
145145
message = "Invalid replication URL (bad hostname or port)"_sl;
146146

@@ -286,7 +286,7 @@ bool C4Address::fromURL(slice url, C4Address* address, slice* dbName) {
286286

287287
address->path = slice(pathStart, str.buf);
288288
*dbName = str;
289-
return C4Replicator::isValidDatabaseName(str);
289+
return !str.empty();
290290
} else {
291291
address->path = slice(pathStart, str.end());
292292
return true;

Replicator/tests/ReplicatorAPITest.cc

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ TEST_CASE("URL Parsing", "[C][Replicator]") {
102102
REQUIRE(c4address_fromURL("wss://localhost/p/d/"_sl, &address, &dbName));
103103
REQUIRE(c4address_fromURL("wss://localhost//p//d/"_sl, &address, &dbName));
104104

105-
REQUIRE(!c4address_fromURL("ws://example.com/db@name"_sl, &address, &dbName));
105+
// We don't check the validity of the database name.
106+
REQUIRE(c4address_fromURL("ws://example.com/db@name"_sl, &address, &dbName));
106107
CHECK(dbName == "db@name"_sl);
107108

108109
// The following URLs should all be rejected:
@@ -121,7 +122,10 @@ TEST_CASE("URL Parsing", "[C][Replicator]") {
121122
CHECK(!c4address_fromURL("ws://localhost:/foo"_sl, &address, &dbName));
122123
CHECK(!c4address_fromURL("ws://localhost"_sl, &address, &dbName));
123124
CHECK(!c4address_fromURL("ws://localhost/"_sl, &address, &dbName));
124-
CHECK(!c4address_fromURL("ws://localhost/B^dn^m*"_sl, &address, &dbName));
125+
126+
// We don't check the validity of the database name.
127+
CHECK(c4address_fromURL("ws://localhost/B^dn^m*"_sl, &address, &dbName));
128+
CHECK(dbName == "B^dn^m*"_sl);
125129

126130
CHECK(!c4address_fromURL("ws://snej@example.com/db"_sl, &address, &dbName));
127131
CHECK(!c4address_fromURL("ws://snej@example.com:8080/db"_sl, &address, &dbName));
@@ -163,24 +167,6 @@ TEST_CASE_METHOD(ReplicatorAPITest, "API Invalid Scheme", "[C][Push][!throws]")
163167
CHECK(err.code == kC4NetErrInvalidURL);
164168
}
165169

166-
// Test missing or invalid database name:
167-
TEST_CASE_METHOD(ReplicatorAPITest, "API Invalid URLs", "[C][Push][!throws]") {
168-
ExpectingExceptions x;
169-
_sg.remoteDBName = ""_sl;
170-
C4Error err;
171-
CHECK(!c4repl_isValidRemote(_sg.address, _sg.remoteDBName, nullptr));
172-
REQUIRE(!startReplicator(kC4Disabled, kC4OneShot, &err));
173-
CHECK(err.domain == NetworkDomain);
174-
CHECK(err.code == kC4NetErrInvalidURL);
175-
176-
_sg.remoteDBName = "Invalid Name"_sl;
177-
err = {};
178-
CHECK(!c4repl_isValidRemote(_sg.address, _sg.remoteDBName, nullptr));
179-
REQUIRE(!startReplicator(kC4Disabled, kC4OneShot, &err));
180-
CHECK(err.domain == NetworkDomain);
181-
CHECK(err.code == kC4NetErrInvalidURL);
182-
}
183-
184170
// Test connection-refused error by connecting to a bogus port of localhost
185171
TEST_CASE_METHOD(ReplicatorAPITest, "API Connection Failure", "[C][Push]") {
186172
ExpectingExceptions x;

Replicator/tests/ReplicatorCollectionSGTest.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ TEST_CASE_METHOD(ReplicatorCollectionSGTest, "Sync with Single Collection SG", "
241241
std::vector<C4CollectionSpec> collectionSpecs{collectionCount};
242242

243243
bool continuous = false;
244-
C4Error expectedError;
244+
C4Error expectedError{};
245245

246246
SECTION("Named Collection") { collectionSpecs = {Roses}; }
247247

@@ -2249,13 +2249,13 @@ static C4Database* copy_and_open(C4Database* db, const string& idPrefix) {
22492249
struct ReplicatorTestDelegate : Replicator::Delegate {
22502250
~ReplicatorTestDelegate() override = default;
22512251

2252-
void replicatorGotTLSCertificate(slice certData) override{};
2253-
void replicatorStatusChanged(Replicator* NONNULL, const Replicator::Status&) override{};
2252+
void replicatorGotTLSCertificate(slice certData) override {};
2253+
void replicatorStatusChanged(Replicator* NONNULL, const Replicator::Status&) override {};
22542254

22552255
void replicatorConnectionClosed(Replicator* NONNULL, const CloseStatus&) override {}
22562256

2257-
void replicatorDocumentsEnded(Replicator* NONNULL, const Replicator::DocumentsEnded&) override{};
2258-
void replicatorBlobProgress(Replicator* NONNULL, const Replicator::BlobProgress&) override{};
2257+
void replicatorDocumentsEnded(Replicator* NONNULL, const Replicator::DocumentsEnded&) override {};
2258+
void replicatorBlobProgress(Replicator* NONNULL, const Replicator::BlobProgress&) override {};
22592259
};
22602260

22612261
// Wait for a replication to go busy then idle.

0 commit comments

Comments
 (0)