CBL-8595: Add URLEndpointListener to the C++ API - #736
Open
jianminzhao wants to merge 8 commits into
Open
Conversation
CBL-8265: Unit tests of class URLEndpointListener in the C++ API Add URLEndpointListener, URLEndpointListenerConfiguration, and ListenerAuthenticator to the C++ API (include/cbl++/URLEndpointListener.hh), wrapping include/cbl/CBLURLEndpointListener.h, following the same pattern established by TLSIdentity's C++ wrapper (CBL 8594). - ListenerAuthenticator boxes password/certificate callbacks in a std::variant held by shared_ptr, bridging arbitrary capturing C++ callables across the C API's plain-function-pointer callback signature. - URLEndpointListener keeps its own copy of the configured ListenerAuthenticator so the callback's context stays alive for the listener's own lifetime, independent of the config used to construct it. - Adds Authenticator::certificateAuthenticator to Replicator.hh, filling a pre-existing gap (Authenticator only wrapped password/session auth). - Full Doxygen coverage, and a complete C++ test port (test/URLEndpointListenerTest_Cpp.cc/.hh) of URLEndpointListenerTest.cc's 21 test cases, plus completing three previously-stubbed C++ tests in TLSIdentityTest_Cpp.cc that needed this wrapper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an Enterprise Edition C++ wrapper for CBLURLEndpointListener (plus related configuration/authentication helpers) and ports/extends the C++ test coverage to match the existing C test suite, including previously-stubbed TLS identity handshake tests.
Changes:
- Introduces
cbl::URLEndpointListener,cbl::URLEndpointListenerConfiguration, andcbl::ListenerAuthenticatorin the public C++ API. - Adds
cbl::Authenticator::certificateAuthenticator(EE-only) to complete the Replicator C++ auth surface. - Adds/ports C++ unit tests for URL endpoint listeners and updates CMake/Xcode test source lists accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/URLEndpointListenerTest_Cpp.hh | Adds a C++ test fixture and helpers for listener/replicator-based tests. |
| test/URLEndpointListenerTest_Cpp.cc | Ports the C URLEndpointListener test suite to C++ (EE-only). |
| test/TLSIdentityTest_Cpp.cc | Updates TLSIdentity C++ tests to use the new C++ listener wrapper and completes previously-stubbed handshake tests. |
| test/cmake/test_source_files.cmake | Registers the new C++ listener test file in the CMake test target. |
| include/cbl++/URLEndpointListener.hh | Adds the new public C++ API wrapper for URL endpoint listeners and listener-side authentication. |
| include/cbl++/TLSIdentity.hh | Adds friendship needed for the new listener/authenticator wrappers to access internal adoption helpers. |
| include/cbl++/Replicator.hh | Adds EE-only client certificate authenticator factory and includes TLSIdentity. |
| include/cbl++/CouchbaseLite.hh | Exposes the new URLEndpointListener C++ header via the umbrella include. |
| CBL_C.xcodeproj/project.pbxproj | Adds the new public header and test sources to Xcode build phases. |
Comments suppressed due to low confidence (1)
include/cbl++/URLEndpointListener.hh:67
- certAuthenticator(callback) accepts an empty std::function; if the listener later invokes it, this will throw std::bad_function_call (or terminate) across the C callback boundary. Validate the callback and throw a Couchbase Lite invalid-parameter error up front.
static ListenerAuthenticator certAuthenticator(CertAuthCallback callback) {
return ListenerAuthenticator(std::move(callback));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Code Coverage Results:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
include/cbl++/URLEndpointListener.hh:72
- Typo in the thrown error message: "callbck" should be "callback".
static ListenerAuthenticator certAuthenticator(CertAuthCallback callback) {
if ( !callback ) {
throw Error{kCBLDomain, kCBLErrorInvalidParameter, "Falsy callbck"};
}
return ListenerAuthenticator(std::move(callback));
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
include/cbl++/URLEndpointListener.hh:61
- Typo in the thrown error message: "callbck" should be "callback" to keep error messages readable/searchable.
throw Error{kCBLDomain, kCBLErrorInvalidParameter, "Falsy callbck"};
include/cbl++/URLEndpointListener.hh:70
- Typo in the thrown error message: "callbck" should be "callback" to keep error messages readable/searchable.
throw Error{kCBLDomain, kCBLErrorInvalidParameter, "Falsy callbck"};
Comment on lines
+232
to
+235
| /** The TLS identity used by the listener for TLS communication, or a falsy TLSIdentity if the | ||
| listener is not started, or if TLS is disabled. | ||
| @note The returned identity remains valid only until the listener is stopped or released. | ||
| Assign it to a variable of your own if you need it to outlive that. */ |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
include/cbl++/URLEndpointListener.hh:123
- ListenerAuthenticator callback is invoked from the C API; if the user-supplied std::function throws, the exception will cross a C function-pointer boundary and can terminate the process. Catch exceptions in the trampoline and return false instead of letting them escape.
static bool _callCertAuth(void* context, CBLCert* cert) {
return std::get<CertAuthCallback>(*(Callback*)context)(Cert(cert));
}
include/cbl++/URLEndpointListener.hh:242
- urls() unconditionally calls FLMutableArray_Release(flUrls), but CBLURLEndpointListener_Urls returns null when the listener isn’t started. If FLMutableArray_Release is not null-safe, this can crash. Guard the release (and return a falsy MutableArray) when flUrls is null.
fleece::MutableArray urls() const {
FLMutableArray flUrls = CBLURLEndpointListener_Urls(ref());
fleece::MutableArray result(flUrls);
FLMutableArray_Release(flUrls);
return result;
Comment on lines
+117
to
+119
| static bool _callPasswordAuth(void* context, FLString username, FLString password) { | ||
| return std::get<PasswordAuthCallback>(*(Callback*)context)(slice(username), slice(password)); | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
include/cbl++/URLEndpointListener.hh:244
- The tlsIdentity() doc comment is internally contradictory: it says the returned identity is only valid until the listener is stopped/released, but also says assigning it to a variable can outlive that. The C API contract is that the identity is valid until stop/release, and can be kept longer by retaining; in C++ that corresponds to storing the returned TLSIdentity (it retains).
/** The TLS identity used by the listener for TLS communication, or a falsy TLSIdentity if the
listener is not started, or if TLS is disabled.
@note The returned identity remains valid only until the listener is stopped or released.
Assign it to a variable of your own if you need it to outlive that. */
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
include/cbl++/URLEndpointListener.hh:244
- The tlsIdentity() doc comment says the returned identity is only valid until the listener is stopped or released, but the C++ wrapper constructor retains the CBLTLSIdentity (via RefCounted), so a stored return value can outlive the listener. The note should match the actual ownership/lifetime semantics to avoid misleading API users.
/** The TLS identity used by the listener for TLS communication, or a falsy TLSIdentity if the
listener is not started, or if TLS is disabled.
@note The returned identity remains valid only until the listener is stopped or released.
Assign it to a variable of your own if you need it to outlive that. */
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
include/cbl++/URLEndpointListener.hh:252
urls()releasesflUrlsunconditionally even though the C API documents thatCBLURLEndpointListener_Urlscan return NULL when the listener isn’t started. Guard the release so this wrapper doesn’t rely onFLMutableArray_Release(NULL)being safe, and explicitly return a falsyMutableArraywhenflUrlsis null.
fleece::MutableArray urls() const {
FLMutableArray flUrls = CBLURLEndpointListener_Urls(ref());
fleece::MutableArray result(flUrls);
FLMutableArray_Release(flUrls);
return result;
include/cbl++/URLEndpointListener.hh:245
- The doc comment for
tlsIdentity()says the returned identity is only valid until the listener is stopped/released, but this wrapper constructsTLSIdentity(...)using the retaining wrapper constructor, so callers can keep the returnedTLSIdentityalive independently of the listener. Please update the note to match the actual C++ lifetime semantics.
/** The TLS identity used by the listener for TLS communication, or a falsy TLSIdentity if the
listener is not started, or if TLS is disabled.
@note The returned identity remains valid only until the listener is stopped or released.
Assign it to a variable of your own if you need it to outlive that. */
TLSIdentity tlsIdentity() const {return TLSIdentity(CBLURLEndpointListener_TLSIdentity(ref()));}
pasin
reviewed
Jul 29, 2026
| @param className The class the trampoline belongs to, used only for the log message. | ||
| @param what A short description of the operation, used only for the log message. */ | ||
| template <class Fn> | ||
| inline bool invokeSafely(const char* className, const char* what, Fn&& fn) noexcept { |
Contributor
There was a problem hiding this comment.
Based on the method name and where it is defined, it's a bit weird that the default error domain is Network. Wondering if the error domain should be passed in?
| using CertAuthCallback = std::function<bool(const Cert& cert)>; | ||
|
|
||
| /** Creates an empty (null) authenticator. */ | ||
| ListenerAuthenticator() = default; |
| // internal::invokeSafely helper) -- so any exception the user's callback throws is caught, | ||
| // logged, and turned into a plain authentication failure (false) instead. | ||
| static bool _callPasswordAuth(void* context, FLString username, FLString password) noexcept { | ||
| return internal::invokeSafely("ListenerAuthenticator", "passwordAuthCallback", [&] { |
Contributor
There was a problem hiding this comment.
The error should be logged in kCBLLogDomainListener domain instead of the default network domain.
| } | ||
|
|
||
| static bool _callCertAuth(void* context, CBLCert* cert) noexcept { | ||
| return internal::invokeSafely("ListenerAuthenticator", "certAuthCallback", [&] { |
Contributor
There was a problem hiding this comment.
The error should be logged in kCBLLogDomainListener domain instead of the default network domain.
| listenerConfig.disableTLS = false; | ||
|
|
||
| SECTION("Self-signed Cert") { | ||
| listenerConfig.tlsIdentity = createTLSIdentity(true, withExternalKey); |
Contributor
There was a problem hiding this comment.
do we need to set identityLabelsToDelete or is it set in createTLSIdentity() function?
| clientEndpoint(listenerConfig, listener)); | ||
| config.acceptOnlySelfSignedServerCertificate = true; | ||
|
|
||
| TLSIdentity clientIdentity = createTLSIdentity(false, withExternalKey); |
Contributor
There was a problem hiding this comment.
Same as above comment, do we need to set identityLabelsToDelete?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CBL-8265: Unit tests of class URLEndpointListener in the C++ API
Add URLEndpointListener, URLEndpointListenerConfiguration, and ListenerAuthenticator to the C++ API (include/cbl++/URLEndpointListener.hh), wrapping include/cbl/CBLURLEndpointListener.h, following the same pattern established by TLSIdentity's C++ wrapper (CBL 8594).