1: 0bc540fbb7e ! 1: 62c795ef489 oauth: Report cleanup errors as warnings on stderr @@ Commit message print WARNINGs when they find themselves in similar situations, so follow their lead. + Reviewed-by: Zsolt Parragi + Discussion: https://postgr.es/m/CAOYmi%2BmEU_q9sr1PMmE-4rLwFN%3DOjyndDwFZvpsMU3RNJLrM9g%40mail.gmail.com + ## src/interfaces/libpq-oauth/oauth-curl.c ## -@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct async_ctx *actx) +@@ src/interfaces/libpq-oauth/oauth-curl.c: struct async_ctx + * Tears down the Curl handles and frees the async_ctx. + */ + static void +-free_async_ctx(PGconn *conn, struct async_ctx *actx) ++free_async_ctx(struct async_ctx *actx) + { + /* + * In general, none of the error cases below should ever happen if we have * no bugs above. But if we do hit them, surfacing those errors somehow * might be the only way to have a chance to debug them. * @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy } free_provider(&actx->provider); +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_cleanup_oauth_flow(PGconn *conn) + + if (state->async_ctx) + { +- free_async_ctx(conn, state->async_ctx); ++ free_async_ctx(state->async_ctx); + state->async_ctx = NULL; + } + 2: 074645e5b41 ! 2: b31a3406af2 libpq: Add PQgetThreadLock() to mirror PQregisterThreadLock() @@ Commit message currently relies on direct injection of pg_g_threadlock, and should ideally not. + Reviewed-by: Zsolt Parragi + Discussion: https://postgr.es/m/CAOYmi%2BmEU_q9sr1PMmE-4rLwFN%3DOjyndDwFZvpsMU3RNJLrM9g%40mail.gmail.com + ## src/interfaces/libpq/exports.txt ## @@ src/interfaces/libpq/exports.txt: PQgetAuthDataHook 207 PQdefaultAuthDataHook 208 @@ src/interfaces/libpq/libpq-fe.h: extern "C" +/* Features added in PostgreSQL v19: */ +/* Indicates presence of PQgetThreadLock */ -+#define LIBPQ_HAS_GET_THREAD_LOCK ++#define LIBPQ_HAS_GET_THREAD_LOCK 1 + /* * Option flags for PQcopyResult 3: b431ee7a1e0 ! 3: 61eeb16c49d libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 @@ src/interfaces/libpq/libpq-fe.h @@ src/interfaces/libpq/libpq-fe.h: extern "C" /* Features added in PostgreSQL v19: */ /* Indicates presence of PQgetThreadLock */ - #define LIBPQ_HAS_GET_THREAD_LOCK + #define LIBPQ_HAS_GET_THREAD_LOCK 1 +/* Indicates presence of the PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 authdata hook */ +#define LIBPQ_HAS_OAUTH_BEARER_TOKEN_V2 1 4: 59a5e8e27d6 ! 4: 3ec1933f6d4 libpq-oauth: Use the PGoauthBearerRequestV2 API @@ src/interfaces/libpq-oauth/exports.txt +pg_start_oauthbearer 2 ## src/interfaces/libpq-oauth/meson.build ## -@@ src/interfaces/libpq-oauth/meson.build: libpq_oauth_st = static_library('libpq-oauth', +@@ src/interfaces/libpq-oauth/meson.build: endif # 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) - + if build_shared_lib -libpq_oauth_so = shared_module(libpq_oauth_name, +libpq_oauth_so = shared_module('libpq-oauth', libpq_oauth_sources + libpq_oauth_so_sources, @@ src/interfaces/libpq-oauth/oauth-curl.c: enum OAuthStep enum OAuthStep step; /* where are we in the flow? */ int timerfd; /* descriptor for signaling async timeouts */ -@@ src/interfaces/libpq-oauth/oauth-curl.c: struct async_ctx - * Tears down the Curl handles and frees the async_ctx. - */ - static void --free_async_ctx(PGconn *conn, struct async_ctx *actx) -+free_async_ctx(PGoauthBearerRequestV2 *req, struct async_ctx *actx) - { - /* - * In general, none of the error cases below should ever happen if we have -@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct async_ctx *actx) +@@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(struct async_ctx *actx) if (actx->timerfd >= 0) close(actx->timerfd); @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy + + /* request->cleanup is only set after actx has been allocated. */ + Assert(actx); -+ -+ free_async_ctx((PGoauthBearerRequestV2 *) request, actx); + +- if (state->async_ctx) ++ free_async_ctx(actx); + request->user = NULL; + + /* libpq has made its own copy of the token; clear ours now. */ + if (request->token) -+ { + { +- free_async_ctx(state->async_ctx); +- state->async_ctx = NULL; + explicit_bzero(request->token, strlen(request->token)); + free(request->token); + request->token = NULL; + } +} -+} -+ + +- set_conn_altsock(conn, PGINVALID_SOCKET); +/* + * Builds an error message from actx and stores it in req->error. The allocation + * is backed by actx->work_data (which will be reset first). @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy + appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); + else + appendPQExpBufferStr(errbuf, actx->errbuf.data); - -- if (state->async_ctx) ++ + if (actx->curl_err[0]) - { -- free_async_ctx(conn, state->async_ctx); -- state->async_ctx = NULL; ++ { + appendPQExpBuffer(errbuf, " (libcurl: %s)", actx->curl_err); + + /* Sometimes libcurl adds a newline to the error buffer. :( */ @@ src/interfaces/libpq-oauth/oauth-curl.c: free_async_ctx(PGconn *conn, struct asy + errbuf->data[errbuf->len - 1] = '\0'; + errbuf->len--; + } - } - -- set_conn_altsock(conn, PGINVALID_SOCKET); ++ } ++ + req->error = errbuf->data; } @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn) +pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request) +{ + struct async_ctx *actx; ++ PQconninfoOption *conninfo = NULL; + + if (!initialize_curl(request)) + return -1; @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn) + initPQExpBuffer(&actx->errbuf); + + /* Pull relevant connection options. */ -+ { -+ PQconninfoOption *conninfo = PQconninfo(conn); -+ ++ conninfo = PQconninfo(conn); + if (!conninfo) + goto oom; + @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn) + } + + PQconninfoFree(conninfo); -+ } ++ conninfo = NULL; /* keeps `goto oom` safe */ + + actx->discovery_uri = request->v1.openid_configuration; + actx->issuer_id = request->issuer; @@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn) + return 0; + +oom: ++ if (conninfo) ++ PQconninfoFree(conninfo); ++ + request->error = libpq_gettext("out of memory"); + return -1; +} 5: 60c2a289390 = 5: 6cf4a1482ed libpq-oauth: Never link against libpq's encoding functions 6: b6974d5c8e2 ! 6: f4e787faccd WIP: Introduce third-party OAuth flow plugins? @@ Metadata ## Commit message ## WIP: Introduce third-party OAuth flow plugins? + [Not intended for PG19.] + This experimental commit promotes the pg_start_oauthbearer API to a public header (libpq-oauth.h) and adds a PGOAUTHMODULE environment variable that overrides the load path for the plugin, allowing users to @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st + fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror()); + + dlclose(state->flow_module); ++ state->flow_module = NULL; + + if (state->builtin) + return 0; @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st - libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr); - return 0; - } -+ libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr); -+ return 0; -+ } ++ appendPQExpBuffer(&conn->errorMessage, ++ "use_builtin_flow: failed to lock mutex (%d)\n", ++ lockerr); - if (!initialized) - { - init( ++ request->error = ""; /* satisfy report_flow_error() */ ++ return -1; ++ } ++ + if (!initialized) + { + init( @@ src/test/modules/oauth_validator/oauth_flow.c (new) + flag_count++; + } + ++ /* Slice OAUTH_TEST_FLAGS into a fake argv array. */ ++ env = strdup(env); + argc = flag_count + 1; + argv = malloc(sizeof(*argv) * (argc + 1)); -+ if (!argv) ++ ++ if (!env || !argv) + { + fprintf(stderr, "out of memory"); + exit(1);