since-v3.diff.txt

text/plain

Filename: since-v3.diff.txt
Type: text/plain
Part: 0
Message: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
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 <zsolt.parragi@percona.com>
    +    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 <zsolt.parragi@percona.com>
    +    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);