Re: [PoC] Federated Authn/z with OAUTHBEARER

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: [PoC] Federated Authn/z with OAUTHBEARER
Дата
Msg-id 912516.1740329361@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: [PoC] Federated Authn/z with OAUTHBEARER  (Daniel Gustafsson <daniel@yesql.se>)
Ответы Re: [PoC] Federated Authn/z with OAUTHBEARER
Re: [PoC] Federated Authn/z with OAUTHBEARER
Список pgsql-hackers
Daniel Gustafsson <daniel@yesql.se> writes:
> I spent a few more hours staring at this, and ran it through a number of CI and
> local builds, without anything showing up.  Pushed to master with the first set
> of buildfarm animals showing green builds.

Coverity has a nit-pick about this:

/srv/coverity/git/pgsql-git/postgresql/src/interfaces/libpq/fe-auth-oauth.c: 784 in setup_token_request()
778             if (!request_copy)
779             {
780                 libpq_append_conn_error(conn, "out of memory");
781                 goto fail;
782             }
783
>>>     CID 1643156:  High impact quality  (WRITE_CONST_FIELD)
>>>     A write to an aggregate overwrites a const-qualified field within the aggregate.
784             memcpy(request_copy, &request, sizeof(request));
785
786             conn->async_auth = run_user_oauth_flow;
787             conn->cleanup_async_auth = cleanup_user_oauth_flow;
788             state->async_ctx = request_copy;
789         }

This is evidently because of the fields declared const:

    /* Hook inputs (constant across all calls) */
    const char *const openid_configuration; /* OIDC discovery URI */
    const char *const scope;    /* required scope(s), or NULL */

IMO, the set of cases where it's legitimate to mark individual struct
fields as const is negligibly small, and this doesn't seem to be one
of them.  It's not obvious to me where/how PGoauthBearerRequest
structs are supposed to be constructed, but I find it hard to believe
that they will all spring full-grown from the forehead of Zeus.
Nonetheless, this declaration requires exactly that.

(I'm kind of surprised that we're not getting similar bleats from
any buildfarm animals, but so far I don't see any.)

BTW, as another nitpicky style matter: why do PGoauthBearerRequest
etc. spell their struct tag names differently from their typedef names
(that is, with/without an underscore)?  That is not our project style
anywhere else, and I'm failing to detect a good reason to do it here.

            regards, tom lane



В списке pgsql-hackers по дате отправления: