Skip to content

Commit

Permalink
CBL-1515: Aggregate the platform variations of URL for Checkpoint (#1102
Browse files Browse the repository at this point in the history
)

Some platforms were passing in an explicit port, even for default 80/443, while others were just leaving it blank. This led to discrepencies between the platform checkpoints, as a remote URL is used for calculating a replication checkpoint.  If a preloaded database is used on a platform that differs in this manner, then the previous checkpoint will fail to be detected.  This commit solves this by checking for both variations during the previous checkpoint search.
  • Loading branch information
borrrden committed Jan 12, 2021
1 parent 6a25805 commit fc5e9f1
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 9 deletions.
47 changes: 47 additions & 0 deletions LiteCore/tests/c4BaseTest.cc
Expand Up @@ -22,6 +22,7 @@
#include "catch.hpp"
#include "NumConversion.hh"
#include "Actor.hh"
#include "URLTransformer.hh"
#include <exception>
#include <chrono>
#include <thread>
Expand All @@ -32,6 +33,7 @@
using namespace fleece;
using namespace std;
using namespace std::chrono_literals;
using namespace litecore::repl;


// NOTE: These tests have to be in the C++ tests target, not the C tests, because they use internal
Expand Down Expand Up @@ -223,4 +225,49 @@ namespace {
actor->bad_recursive_doot();
this_thread::sleep_for(2s);
}

TEST_CASE("URL Transformation") {
slice withPort, unaffected;
alloc_slice withoutPort;
SECTION("Plain") {
withPort = "ws://duckduckgo.com:80/search"_sl;
withoutPort = "ws://duckduckgo.com/search"_sl;
unaffected = "ws://duckduckgo.com:4984/search"_sl;
}

SECTION("TLS") {
withPort = "wss://duckduckgo.com:443/search"_sl;
withoutPort = "wss://duckduckgo.com/search"_sl;
unaffected = "wss://duckduckgo.com:4984/search"_sl;
}

alloc_slice asIsWithPort = transform_url(withPort, URLTransformStrategy::AsIs);
alloc_slice asIsWithoutPort = transform_url(withoutPort, URLTransformStrategy::AsIs);
alloc_slice asInUnaffected = transform_url(unaffected, URLTransformStrategy::AsIs);

CHECK(asIsWithPort == withPort);
CHECK(asIsWithoutPort == withoutPort);
CHECK(asIsWithoutPort.buf == withoutPort.buf);
CHECK(asInUnaffected == unaffected);

alloc_slice addPortWithPort = transform_url(withPort, URLTransformStrategy::AddPort);
alloc_slice addPortWithoutPort = transform_url(withoutPort, URLTransformStrategy::AddPort);
alloc_slice addPortUnaffected = transform_url(unaffected, URLTransformStrategy::AddPort);

CHECK(addPortWithPort == withPort);
CHECK(addPortWithoutPort == withPort);
CHECK(!addPortUnaffected);

alloc_slice removePortWithPort = transform_url(withPort, URLTransformStrategy::RemovePort);
alloc_slice removePortWithoutPort = transform_url(withoutPort, URLTransformStrategy::RemovePort);
alloc_slice removePortUnaffected = transform_url(unaffected, URLTransformStrategy::RemovePort);

CHECK(removePortWithPort == withoutPort);
CHECK(removePortWithoutPort == withoutPort);
CHECK(!removePortUnaffected);

URLTransformStrategy strategy = URLTransformStrategy::AsIs;
CHECK(++strategy == URLTransformStrategy::AddPort);
CHECK(++strategy == URLTransformStrategy::RemovePort);
}
}
35 changes: 27 additions & 8 deletions Replicator/Checkpointer.cc
Expand Up @@ -205,7 +205,7 @@ namespace litecore { namespace repl {
C4UUID privateID;
if (!c4db_getUUIDs(db, nullptr, &privateID, err))
return nullslice;
_docID = docIDForUUID(privateID);
_docID = docIDForUUID(privateID, URLTransformStrategy::AsIs);
}

return _docID;
Expand All @@ -228,7 +228,7 @@ namespace litecore { namespace repl {


// Computes the ID of the checkpoint document.
string Checkpointer::docIDForUUID(const C4UUID &localUUID) {
string Checkpointer::docIDForUUID(const C4UUID &localUUID, URLTransformStrategy urlStrategy) {
// Derive docID from from db UUID, remote URL, channels, filter, and docIDs.
Array channels = _options.channels();
Value filter = _options.properties[kC4ReplicatorOptionFilter];
Expand All @@ -240,7 +240,13 @@ namespace litecore { namespace repl {
enc.beginArray();
enc.writeString({&localUUID, sizeof(C4UUID)});

enc.writeString(remoteDBIDString());
alloc_slice rawURL(remoteDBIDString());
auto encodedURL = transform_url(rawURL, urlStrategy);
if(!encodedURL) {
return "";
}

enc.writeString(encodedURL);
if (!channels.empty() || !docIDs.empty() || filter) {
// Optional stuff:
writeValueOrNull(enc, channels);
Expand Down Expand Up @@ -287,11 +293,24 @@ namespace litecore { namespace repl {
const c4::ref<C4RawDocument> doc = c4raw_get(db, kC4InfoStore,
constants::kPreviousPrivateUUIDKey, outError);
if (doc) {
// If there is one, derive a doc ID from that and look for a checkpoint there:
_initialDocID = docIDForUUID(*(C4UUID*)doc->body.buf);
body = _read(db, _initialDocID, outError);
if (!body && !isNotFoundError(*outError))
return false;
// If there is one, derive a doc ID from that and look for a checkpoint there
for(URLTransformStrategy strategy = URLTransformStrategy::AddPort; strategy <= URLTransformStrategy::RemovePort; ++strategy) {
// CBL-1515: Make sure to account for platform inconsistencies in the format
// (some have been forcing the port for standard ports and others were omitting it)
_initialDocID = docIDForUUID(*(C4UUID*)doc->body.buf, strategy);
if(!_initialDocID) {
continue;
}

body = _read(db, _initialDocID, outError);
if(body) {
break;
}

if (!isNotFoundError(*outError)) {
return false;
}
}
} else if (!isNotFoundError(*outError)) {
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion Replicator/Checkpointer.hh
Expand Up @@ -10,6 +10,7 @@
#include "Timer.hh"
#include "c4Base.h"
#include "fleece/slice.hh"
#include "URLTransformer.hh"
#include <chrono>
#include <memory>
#include <mutex>
Expand Down Expand Up @@ -156,7 +157,7 @@ namespace litecore { namespace repl {

private:
void checkpointIsInvalid();
std::string docIDForUUID(const C4UUID&);
std::string docIDForUUID(const C4UUID&, URLTransformStrategy strategy);
slice remoteDocID(C4Database *db NONNULL, C4Error* err);
alloc_slice _read(C4Database *db NONNULL, slice, C4Error*);
void initializeDocIDs();
Expand Down
82 changes: 82 additions & 0 deletions Replicator/URLTransformer.cc
@@ -0,0 +1,82 @@
//
// URLTransformer.cc
//
// Copyright (c) 2021 Couchbase. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

#include "URLTransformer.hh"
#include "c4Replicator.h"

namespace litecore::repl {
URLTransformStrategy& operator++(URLTransformStrategy& s) {
s = static_cast<URLTransformStrategy>(static_cast<unsigned>(s) + 1);
return s;
}

static alloc_slice AsIs(slice inputURL) {
return static_cast<alloc_slice>(inputURL);
}

static alloc_slice AddPort(slice inputURL) {
C4Address addr;
if(!c4address_fromURL(inputURL, &addr, nullptr) || (addr.port != 80 && addr.port != 443)) {
return nullslice;
}

if(c4SliceEqual(kC4Replicator2Scheme, addr.scheme)) {
addr.port = 80;
} else if(c4SliceEqual(kC4Replicator2TLSScheme, addr.scheme)) {
addr.port = 443;
}

return c4address_toURL(addr);
}

static alloc_slice RemovePort(slice inputURL) {
C4Address addr;
if(!c4address_fromURL(inputURL, &addr, nullptr) || (addr.port != 80 && addr.port != 443)) {
return nullslice;
}

addr.port = 0;
return c4address_toURL(addr);
}

alloc_slice transform_url(slice inputURL, URLTransformStrategy strategy) {
switch(strategy) {
case URLTransformStrategy::AsIs:
return AsIs(inputURL);
case URLTransformStrategy::AddPort:
return AddPort(inputURL);
case URLTransformStrategy::RemovePort:
return RemovePort(inputURL);
}

return nullslice;
}

alloc_slice transform_url(const alloc_slice &inputURL, URLTransformStrategy strategy) {
switch(strategy) {
case URLTransformStrategy::AsIs:
return inputURL;
case URLTransformStrategy::AddPort:
return AddPort(inputURL);
case URLTransformStrategy::RemovePort:
return RemovePort(inputURL);
}

return nullslice;
}
}
65 changes: 65 additions & 0 deletions Replicator/URLTransformer.hh
@@ -0,0 +1,65 @@
//
// URLTransformer.hh
//
// Copyright (c) 2021 Couchbase. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

#pragma once

#include "fleece/slice.hh"
#include <optional>

namespace litecore::repl {
using namespace fleece;
using namespace std;

/**
* Strategies for handling the situation in CBL-1515
* (briefly, platforms are inconsistent about including port number
* in their URLs)
*/
enum class URLTransformStrategy : unsigned {
AsIs, ///< Pass through the URL unaltered
AddPort, ///< Force the port in the URL
RemovePort ///< Force no port in the URL
};

// An operator to allow simple forward iteration on the enum values
URLTransformStrategy& operator++(URLTransformStrategy& s);

/**
* Transforms the URL passed in input using the provided strategy (note only URLs using
* standard ports 80 / 443 will be considered for transformation)
*
* @param inputURL The URL to transform
* @param strategy The strategy to use
* @returns The transformed URL. If the URL is not a candidate for AddPort or RemovePort
* (e.g. not a valid URL, or not using a standard port) then nullslice is returned)
*/
alloc_slice transform_url(slice inputURL, URLTransformStrategy strategy);

/**
* Transforms the URL passed in input using the provided strategy (note only URLs using
* standard ports 80 / 443 will be considered for transformation). This overload
* is simply to allow the optimization to not make copies in the AsIs strategy.
*
* @param inputURL The URL to transform
* @param strategy The strategy to use
* @returns The transformed URL. If the URL is not a candidate for AddPort or RemovePort
* (e.g. not a valid URL, or not using a standard port) then nullslice is returned)
*/
alloc_slice transform_url(const alloc_slice &inputURL, URLTransformStrategy strategy);
}

9 changes: 9 additions & 0 deletions Xcode/LiteCore.xcodeproj/project.pbxproj
Expand Up @@ -482,6 +482,7 @@
42030A402498444900283CE8 /* Error.cc in Sources */ = {isa = PBXBuildFile; fileRef = 27393A861C8A353A00829C9B /* Error.cc */; };
42030A412498445600283CE8 /* FilePath.cc in Sources */ = {isa = PBXBuildFile; fileRef = 27E89BA41D679542002C32B3 /* FilePath.cc */; };
42030A422498446000283CE8 /* LibC++Debug.cc in Sources */ = {isa = PBXBuildFile; fileRef = 27BF023C1FB61F5F003D5BB8 /* LibC++Debug.cc */; };
42B6B0E225A6A9D9004B20A7 /* URLTransformer.cc in Sources */ = {isa = PBXBuildFile; fileRef = 42B6B0E125A6A9D9004B20A7 /* URLTransformer.cc */; };
726F2B901EB2C36E00C1EC3C /* DefaultLogger.cc in Sources */ = {isa = PBXBuildFile; fileRef = 726F2B8F1EB2C36E00C1EC3C /* DefaultLogger.cc */; };
728EC54D1EC14611002C9A73 /* c4Listener.h in Headers */ = {isa = PBXBuildFile; fileRef = 728EC54C1EC14611002C9A73 /* c4Listener.h */; };
729272F52238DC0C00E7208E /* c4ExceptionUtils.cc in Sources */ = {isa = PBXBuildFile; fileRef = 729272F12238DB8500E7208E /* c4ExceptionUtils.cc */; };
Expand Down Expand Up @@ -1549,6 +1550,8 @@
27FDF13E1DA84EE70087B4E6 /* SQLiteFleeceUtil.hh */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = SQLiteFleeceUtil.hh; sourceTree = "<group>"; };
27FDF1421DAC22230087B4E6 /* SQLiteFunctionsTest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SQLiteFunctionsTest.cc; sourceTree = "<group>"; };
27FDF1A21DAD79450087B4E6 /* LiteCore-dylib_Release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = "LiteCore-dylib_Release.xcconfig"; sourceTree = "<group>"; };
42B6B0DD25A6A9D9004B20A7 /* URLTransformer.hh */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = URLTransformer.hh; sourceTree = "<group>"; };
42B6B0E125A6A9D9004B20A7 /* URLTransformer.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = URLTransformer.cc; sourceTree = "<group>"; };
720EA3F51BA7EAD9002B8416 /* libLiteCore.dylib */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.dylib"; includeInIndex = 0; path = libLiteCore.dylib; sourceTree = BUILT_PRODUCTS_DIR; };
726F2B8F1EB2C36E00C1EC3C /* DefaultLogger.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DefaultLogger.cc; sourceTree = "<group>"; };
7280F7F01E3AC9A600E3F097 /* libLiteCore.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libLiteCore.dylib; path = ../build_cmake/libLiteCore.dylib; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2473,6 +2476,8 @@
2779CC6E1E85E4FC00F0D251 /* ReplicatorTypes.hh */,
27FA569524B640E700B2F1F8 /* RemoteSequence.hh */,
2773FCFC1E67A64D00108780 /* RemoteSequenceSet.hh */,
42B6B0DD25A6A9D9004B20A7 /* URLTransformer.hh */,
42B6B0E125A6A9D9004B20A7 /* URLTransformer.cc */,
275CE1131E5BAC180084E014 /* Worker.cc */,
275CE1141E5BAC180084E014 /* Worker.hh */,
);
Expand Down Expand Up @@ -4030,6 +4035,7 @@
27469D07233D719800A1EE1A /* PublicKey.cc in Sources */,
27098AC421752A29002751DA /* SQLiteKeyStore+PredictiveIndexes.cc in Sources */,
278BD68B1EEB6756000DBF41 /* DatabaseCookies.cc in Sources */,
42B6B0E225A6A9D9004B20A7 /* URLTransformer.cc in Sources */,
27E3DD371DB450B300F2872D /* Logging.cc in Sources */,
27FC8DB622135BCE0083B033 /* ChangesFeed.cc in Sources */,
27E35AC81E942D6100E103F9 /* IncomingRev.cc in Sources */,
Expand Down Expand Up @@ -5308,6 +5314,7 @@
isa = XCBuildConfiguration;
baseConfigurationReference = 27DF7D6D1F4239A80022F3DF /* SQLite_Release.xcconfig */;
buildSettings = {
LLVM_LTO = NO;
};
name = Release;
};
Expand All @@ -5322,6 +5329,7 @@
isa = XCBuildConfiguration;
baseConfigurationReference = 273E9FBF1C519A1B003115A6 /* Tokenizer.xcconfig */;
buildSettings = {
LLVM_LTO = NO;
};
name = Release;
};
Expand All @@ -5336,6 +5344,7 @@
isa = XCBuildConfiguration;
baseConfigurationReference = 273E9FAA1C519A1B003115A6 /* LiteCore static.xcconfig */;
buildSettings = {
LLVM_LTO = NO;
};
name = Release;
};
Expand Down
1 change: 1 addition & 0 deletions cmake/platform_base.cmake
Expand Up @@ -99,6 +99,7 @@ function(set_litecore_source_base)
Replicator/Replicator.cc
Replicator/ReplicatorTypes.cc
Replicator/RevFinder.cc
Replicator/URLTransformer.cc
Replicator/Worker.cc
LiteCore/Support/c4ExceptionUtils.cc
LiteCore/Support/Logging.cc
Expand Down

0 comments on commit fc5e9f1

Please sign in to comment.