-: ----------- > 1: 5f87f11b18e Add minor-version counterpart to (PG_)MAJORVERSION 1: 942ad5391e2 ! 2: 4c9cc7f69af oauth: Move the builtin flow into a separate module @@ Commit message the search path came from a different build of Postgres. This ABI is considered "private". The module has no SONAME or version - symlinks, and it's named libpq-oauth-.so to avoid mixing and - matching across major Postgres versions. (Future improvements may - promote this "OAuth flow plugin" to a first-class concept, at which - point we would need a public API to replace this anyway.) + symlinks, and it's named libpq-oauth--.so to avoid mixing + and matching across Postgres versions, in case internal struct order + needs to change. (Future improvements may promote this "OAuth flow + plugin" to a first-class concept, at which point we would need a public + API to replace this anyway.) Additionally, NLS support for error messages in b3f0be788a was incomplete, because the new error macros weren't being scanned by @@ src/interfaces/libpq-oauth/Makefile (new) + +# This is an internal module; we don't want an SONAME and therefore do not set +# SO_MAJOR_VERSION. -+NAME = pq-oauth-$(MAJORVERSION) ++NAME = pq-oauth-$(MAJORVERSION)-$(MINORVERSION) + -+# Force the name "libpq-oauth" for both the static and shared libraries. ++# Force the name "libpq-oauth" for both the static and shared libraries. The ++# staticlib doesn't need version information in its name. +override shlib := lib$(NAME)$(DLSUFFIX) -+override stlib := lib$(NAME).a ++override stlib := libpq-oauth.a + +override CPPFLAGS := -I$(libpq_srcdir) -I$(top_builddir)/src/port $(LIBCURL_CPPFLAGS) $(CPPFLAGS) + +OBJS = \ -+ $(WIN32RES) \ -+ oauth-curl.o ++ $(WIN32RES) ++ ++OBJS_STATIC = oauth-curl.o + +# The shared library needs additional glue symbols. -+$(shlib): OBJS += oauth-utils.o -+$(shlib): oauth-utils.o ++OBJS_SHLIB = \ ++ oauth-curl_shlib.o \ ++ oauth-utils.o \ ++ ++oauth-utils.o: override CPPFLAGS += -DUSE_DYNAMIC_OAUTH ++oauth-curl_shlib.o: override CPPFLAGS_SHLIB += -DUSE_DYNAMIC_OAUTH ++ ++# Add shlib-/stlib-specific objects. ++$(shlib): override OBJS += $(OBJS_SHLIB) ++$(shlib): $(OBJS_SHLIB) ++ ++$(stlib): override OBJS += $(OBJS_STATIC) ++$(stlib): $(OBJS_STATIC) + +SHLIB_LINK_INTERNAL = $(libpq_pgport_shlib) +SHLIB_LINK = $(LIBCURL_LDFLAGS) $(LIBCURL_LDLIBS) @@ src/interfaces/libpq-oauth/Makefile (new) +# Shared library stuff +include $(top_srcdir)/src/Makefile.shlib + ++# Use src/common/Makefile's trick for tracking dependencies of shlib-specific ++# objects. ++%_shlib.o: %.c %.o ++ $(CC) $(CFLAGS) $(CFLAGS_SL) $(CPPFLAGS) $(CPPFLAGS_SHLIB) -c $< -o $@ ++ +# Ignore the standard rules for SONAME-less installation; we want both the +# static and shared libraries to go into libdir. +install: all installdirs $(stlib) $(shlib) @@ src/interfaces/libpq-oauth/Makefile (new) + rm -f '$(DESTDIR)$(libdir)/$(shlib)' + +clean distclean: clean-lib -+ rm -f $(OBJS) oauth-utils.o ++ rm -f $(OBJS) $(OBJS_STATIC) $(OBJS_SHLIB) ## src/interfaces/libpq-oauth/README (new) ## @@ @@ src/interfaces/libpq-oauth/README (new) += Load-Time ABI = + +This module ABI is an internal implementation detail, so it's subject to change -+across major releases; the name of the module (libpq-oauth-MAJOR) reflects this. ++across releases; the name of the module (libpq-oauth-MAJOR-MINOR) reflects this. +The module exports the following symbols: + +- PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn); @@ src/interfaces/libpq-oauth/README (new) + +At the moment, pg_fe_run_oauth_flow() relies on libpq's pg_g_threadlock and +libpq_gettext(), which must be injected by libpq using this initialization -+function before the flow is run. It also relies on libpq to expose -+conn->errorMessage, via the errmsg_impl. ++function before the flow is run. + -+This dependency injection is done to ensure that the module ABI is decoupled -+from the internals of `struct pg_conn`. This way we can safely search the -+standard dlopen() paths (e.g. RPATH, LD_LIBRARY_PATH, the SO cache) for an -+implementation module to use, even if that module wasn't compiled at the same -+time as libpq. ++It also relies on libpq to expose conn->errorMessage, via the errmsg_impl. This ++is done to decouple the module ABI from the offset of errorMessage, which can ++change positions depending on configure-time options. This way we can safely ++search the standard dlopen() paths (e.g. RPATH, LD_LIBRARY_PATH, the SO cache) ++for an implementation module to use, even if that module wasn't compiled at the ++same time as libpq. + += Static Build = + +The static library libpq.a does not perform any dynamic loading. If the builtin -+flow is enabled, the application is expected to link against libpq-oauth-*.a ++flow is enabled, the application is expected to link against libpq-oauth.a +directly to provide the necessary symbols. ## src/interfaces/libpq-oauth/exports.txt (new) ## @@ src/interfaces/libpq-oauth/meson.build (new) +libpq_oauth_so_sources = files( + 'oauth-utils.c', +) ++libpq_oauth_so_c_args = ['-DUSE_DYNAMIC_OAUTH'] + +export_file = custom_target('libpq-oauth.exports', + kwargs: gen_export_kwargs, @@ src/interfaces/libpq-oauth/meson.build (new) +# port needs to be in include path due to pthread-win32.h +libpq_oauth_inc = include_directories('.', '../libpq', '../../port') + -+# This is an internal module; we don't want an SONAME and therefore do not set -+# SO_MAJOR_VERSION. -+libpq_oauth_name = 'libpq-oauth-@0@'.format(pg_version_major) -+ -+libpq_oauth_st = static_library(libpq_oauth_name, ++libpq_oauth_st = static_library('libpq-oauth', + libpq_oauth_sources, + include_directories: [libpq_oauth_inc, postgres_inc], + c_pch: pch_postgres_fe_h, @@ src/interfaces/libpq-oauth/meson.build (new) + kwargs: default_lib_args, +) + ++# This is an internal module; we don't want an SONAME and therefore do not set ++# SO_MAJOR_VERSION. ++libpq_oauth_name = 'libpq-oauth-@0@-@1@'.format(pg_version_major, pg_version_minor) ++ +libpq_oauth_so = shared_module(libpq_oauth_name, + libpq_oauth_sources + libpq_oauth_so_sources, + include_directories: [libpq_oauth_inc, postgres_inc], ++ c_args: libpq_so_c_args, + c_pch: pch_postgres_fe_h, + dependencies: [frontend_shlib_code, libpq, libpq_oauth_deps], + link_depends: export_file, @@ src/interfaces/libpq/fe-auth-oauth-curl.c => src/interfaces/libpq-oauth/oauth-cu -#include "libpq-int.h" #include "mb/pg_wchar.h" +#include "oauth-curl.h" ++#ifdef USE_DYNAMIC_OAUTH +#include "oauth-utils.h" ++#endif /* * It's generally prudent to set a maximum response size to buffer in memory, @@ src/interfaces/libpq-oauth/oauth-curl.c: prompt_user(struct async_ctx *actx, PGc if (!res) { +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn) + { + fe_oauth_state *state = conn->sasl_state; + struct async_ctx *actx; ++ PQExpBuffer errbuf; + + if (!initialize_curl(conn)) + return PGRES_POLLING_FAILED; +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn) + + error_return: + ++ /* ++ * For the dynamic module build, we can't safely rely on the offset of ++ * conn->errorMessage, since it depends on build options like USE_SSL et ++ * al. libpq gives us a translator function instead. ++ */ ++#ifdef USE_DYNAMIC_OAUTH ++ errbuf = conn_errorMessage(conn); ++#else ++ errbuf = &conn->errorMessage; ++#endif ++ + /* + * Assemble the three parts of our error: context, body, and detail. See + * also the documentation for struct async_ctx. + */ + if (actx->errctx) + { +- appendPQExpBufferStr(&conn->errorMessage, +- libpq_gettext(actx->errctx)); +- appendPQExpBufferStr(&conn->errorMessage, ": "); ++ appendPQExpBufferStr(errbuf, libpq_gettext(actx->errctx)); ++ appendPQExpBufferStr(errbuf, ": "); + } + + if (PQExpBufferDataBroken(actx->errbuf)) +- appendPQExpBufferStr(&conn->errorMessage, +- libpq_gettext("out of memory")); ++ appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); + else +- appendPQExpBufferStr(&conn->errorMessage, actx->errbuf.data); ++ appendPQExpBufferStr(errbuf, actx->errbuf.data); + + if (actx->curl_err[0]) + { + size_t len; + +- appendPQExpBuffer(&conn->errorMessage, +- " (libcurl: %s)", actx->curl_err); ++ appendPQExpBuffer(errbuf, " (libcurl: %s)", actx->curl_err); + + /* Sometimes libcurl adds a newline to the error buffer. :( */ +- len = conn->errorMessage.len; +- if (len >= 2 && conn->errorMessage.data[len - 2] == '\n') ++ len = errbuf->len; ++ if (len >= 2 && errbuf->data[len - 2] == '\n') + { +- conn->errorMessage.data[len - 2] = ')'; +- conn->errorMessage.data[len - 1] = '\0'; +- conn->errorMessage.len--; ++ errbuf->data[len - 2] = ')'; ++ errbuf->data[len - 1] = '\0'; ++ errbuf->len--; + } + } + +- appendPQExpBufferChar(&conn->errorMessage, '\n'); ++ appendPQExpBufferChar(errbuf, '\n'); + + return PGRES_POLLING_FAILED; + } ## src/interfaces/libpq-oauth/oauth-curl.h (new) ## @@ @@ src/interfaces/libpq-oauth/oauth-utils.c (new) +#include "libpq-int.h" +#include "oauth-utils.h" + ++#ifndef USE_DYNAMIC_OAUTH ++#error oauth-utils.c is not supported in static builds ++#endif ++ +static libpq_gettext_func libpq_gettext_impl; -+static conn_errorMessage_func conn_errorMessage; + +pgthreadlock_t pg_g_threadlock; ++conn_errorMessage_func conn_errorMessage; + +/*- + * Initializes libpq-oauth by setting necessary callbacks. @@ src/interfaces/libpq-oauth/oauth-utils.h (new) + libpq_gettext_func gettext_impl, + conn_errorMessage_func errmsg_impl); + ++/* Callback to safely obtain conn->errorMessage from a PGconn. */ ++extern conn_errorMessage_func conn_errorMessage; ++ +/* Duplicated APIs, copied from libpq. */ +extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3); +extern bool oauth_unsafe_debugging_enabled(void); @@ src/interfaces/libpq/Makefile: ifeq ($(with_ssl),openssl) +# libpq.so doesn't link against libcurl, but libpq.a needs libpq-oauth, and +# libpq-oauth needs libcurl. Put both into *.private. +PKG_CONFIG_REQUIRES_PRIVATE += libcurl -+%.pc: override SHLIB_LINK_INTERNAL += -lpq-oauth-$(MAJORVERSION) ++%.pc: override SHLIB_LINK_INTERNAL += -lpq-oauth +endif + all: all-lib libpq-refs-stamp @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup_user_oauth_flow(PGconn *conn) + */ + const char *const module_name = +#if defined(__darwin__) -+ LIBDIR "/libpq-oauth-" PG_MAJORVERSION DLSUFFIX; ++ LIBDIR "/libpq-oauth-" PG_MAJORVERSION "-" PG_MINORVERSION DLSUFFIX; +#else -+ "libpq-oauth-" PG_MAJORVERSION DLSUFFIX; ++ "libpq-oauth-" PG_MAJORVERSION "-" PG_MINORVERSION DLSUFFIX; +#endif + + state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL); @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth - -#else - libpq_append_conn_error(conn, "no custom OAuth flows are available, and libpq was not built with libcurl support"); -+ libpq_append_conn_error(conn, "no custom OAuth flows are available, and the builtin flow is not installed"); ++ libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); goto fail; - -#endif @@ src/interfaces/libpq/meson.build: libpq = declare_dependency( + # libpq-oauth needs libcurl. Put both into *.private. + private_deps += [ + libpq_oauth_deps, -+ '-lpq-oauth-@0@'.format(pg_version_major), ++ '-lpq-oauth', + ] +endif + @@ src/test/modules/oauth_validator/t/002_client.pl: if ($ENV{with_libcurl} ne 'yes flags => ["--no-hook"], expected_stderr => - qr/no custom OAuth flows are available, and libpq was not built with libcurl support/ -+ qr/no custom OAuth flows are available, and the builtin flow is not installed/ ++ qr/no OAuth flows are available \(try installing the libpq-oauth package\)/ ); }