Thread

Commits

  1. oauth: Skip call-count test for libcurl 8.20.0

  2. libpq-oauth: Print libcurl version with OAUTHDEBUG_UNSAFE_TRACE

  1. [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-15T19:28:17Z

    Hi all,
    
    A couple of macOS testers pinged me last week with a newly failing
    test in oauth_validator/001_server:
    
        not ok 113 - call count is reasonably small
    
    It's a heuristic test, so I was looking into whether that heuristic
    needed to be tweaked. But this is actually a legitimate failure,
    caused by an upstream regression in Curl 8.20.0 [1]. I've tested the
    most recent 8.21.0 RC, due to release later this month, and the test
    now passes again.
    
    So that just leaves what to do about the current test failures. My
    current idea is to just skip the test if the curl binary reports that
    specific minor version. (A configure test probably won't help very
    much after this month: anyone consuming rolling releases of Curl in
    production will have already built against a past version, and if an
    upcoming LTS distro chose 8.20.0 as its base, they'd almost certainly
    backport the fix too, making the check worse than nothing.) An
    alternative would be to just let it ride for a couple weeks, but I
    don't really want to inflict that on our Homebrew testers.
    
    Either way, debugging this has inspired some improvements to
    OAUTHDEBUG_UNSAFE_TRACE that I'll propose after the 19 freeze is over.
    
    Thoughts?
    
    --Jacob
    
    [1] https://github.com/curl/curl/issues/21547
    
    
    
    
  2. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-15T19:47:42Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
    > So that just leaves what to do about the current test failures. My
    > current idea is to just skip the test if the curl binary reports that
    > specific minor version.
    
    Seems like a good solution.  We don't know how long such curl
    binaries will persist in the wild.
    
    			regards, tom lane
    
    
    
    
  3. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-15T22:56:52Z

    On Mon, Jun 15, 2026 at 12:47 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Jacob Champion <jacob.champion@enterprisedb.com> writes:
    > > So that just leaves what to do about the current test failures. My
    > > current idea is to just skip the test if the curl binary reports that
    > > specific minor version.
    >
    > Seems like a good solution.  We don't know how long such curl
    > binaries will persist in the wild.
    
    Cool. Unfortunately, I immediately ran into an obvious-in-retrospect
    problem: the Homebrew libcurl we're linked against is not what you see
    when you type `curl --version` at the command line, and I don't think
    we record the runtime version of libcurl anywhere today. I've attached
    a solution that should work well for PG20, but I don't feel as good
    about it for 19 (or a backport to 18).
    
    I need to switch to a different context for today, but tomorrow I'll
    try to find a solution that touches only the test code.
    
    Thanks,
    --Jacob
    
  4. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-16T19:46:31Z

    On Mon, Jun 15, 2026 at 3:56 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > I need to switch to a different context for today, but tomorrow I'll
    > try to find a solution that touches only the test code.
    
    Attached as v2-0002, which moves the version check into one of the
    OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is
    branched; it just implements the v1 strategy and reverts 0002.)
    
    I've tested this against a local Homebrew installation, but if anyone
    who's hit this in the wild has a chance to put 0001-3 through a smoke
    test, that'd be awesome. Barring any objections or bad test results,
    I'll plan to push tomorrow.
    
    Thanks!
    --Jacob
    
  5. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Daniel Gustafsson <daniel@yesql.se> — 2026-06-16T20:11:53Z

    > On 16 Jun 2026, at 21:46, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
    
    > Attached as v2-0002, which moves the version check into one of the
    > OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is
    > branched; it just implements the v1 strategy and reverts 0002.)
    
    +#if USE_LIBCURL
    +
    +/*
    + * XXX You may wonder why this test executable, which purposely does not make
    + * use of libcurl functionality, is printing out the version of Curl. This is
    + * needed to skip tests in 001_server when we see a known broken version of
    + * libcurl. (Querying the local Curl executable isn't good enough, because that
    + * may not use the same libcurl that we've been configured with.)
    
    If there is concern over the neatness of this (long term in the backbranches,
    for master it will be reverted as mentioned), I guess one option could be to
    introduce a new binary, oauth_curl_version, which only prints the version and
    exits instead oa adding a flag to an otherwise unrelated binary.  It would add
    a little compilation overhead though.
    
    --
    Daniel Gustafsson
    
    
    
    
    
  6. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-16T20:40:50Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
    > Attached as v2-0002, which moves the version check into one of the
    > OAuth test executables. (I'll hold 0004 until after REL_19_STABLE is
    > branched; it just implements the v1 strategy and reverts 0002.)
    
    > I've tested this against a local Homebrew installation, but if anyone
    > who's hit this in the wild has a chance to put 0001-3 through a smoke
    > test, that'd be awesome. Barring any objections or bad test results,
    > I'll plan to push tomorrow.
    
    I confirm that, with or without 0004, this fixes the oauth_validator
    failure on the machine where I saw that.
    
    However ... I don't love the plan of fixing this differently in v19
    and v20 just because of feature freeze.  Exposing more information
    for testing purposes isn't a user-visible feature IMO, so I would
    rather we go straight to 0004.
    
    CC'ing the RMT to see if they agree.  (I think the rmt@ alias is
    not functioning, so cc'ing members directly.)
    
    			regards, tom lane
    
    
    
    
  7. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-16T20:58:04Z

    On Tue, Jun 16, 2026 at 1:12 PM Daniel Gustafsson <daniel@yesql.se> wrote:
    > If there is concern over the neatness of this (long term in the backbranches,
    > for master it will be reverted as mentioned), I guess one option could be to
    > introduce a new binary, oauth_curl_version, which only prints the version and
    > exits instead oa adding a flag to an otherwise unrelated binary.  It would add
    > a little compilation overhead though.
    
    I considered that, but I think adding a second binary to the existing
    Makefile is likely to result in build system churn (to get it to
    behave like e.g. test_json_parser/Makefile). I can investigate that
    path if you think it'd be more maintainable that way, but let's make
    sure Tom's point below doesn't moot it:
    
    On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > I confirm that, with or without 0004, this fixes the oauth_validator
    > failure on the machine where I saw that.
    
    Thanks very much!
    
    > However ... I don't love the plan of fixing this differently in v19
    > and v20 just because of feature freeze.  Exposing more information
    > for testing purposes isn't a user-visible feature IMO, so I would
    > rather we go straight to 0004.
    
    Fair enough. If the RMT is okay with this for 19, were you thinking
    we'd also backpatch that code directly to 18?
    
    --Jacob
    
    
    
    
  8. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-16T21:02:28Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
    > On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> However ... I don't love the plan of fixing this differently in v19
    >> and v20 just because of feature freeze.  Exposing more information
    >> for testing purposes isn't a user-visible feature IMO, so I would
    >> rather we go straight to 0004.
    
    > Fair enough. If the RMT is okay with this for 19, were you thinking
    > we'd also backpatch that code directly to 18?
    
    Sure.
    
    			regards, tom lane
    
    
    
    
  9. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Nathan Bossart <nathandbossart@gmail.com> — 2026-06-17T15:40:04Z

    [RMT hat]
    
    On Tue, Jun 16, 2026 at 05:02:28PM -0400, Tom Lane wrote:
    > Jacob Champion <jacob.champion@enterprisedb.com> writes:
    >> On Tue, Jun 16, 2026 at 1:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >>> However ... I don't love the plan of fixing this differently in v19
    >>> and v20 just because of feature freeze.  Exposing more information
    >>> for testing purposes isn't a user-visible feature IMO, so I would
    >>> rather we go straight to 0004.
    > 
    >> Fair enough. If the RMT is okay with this for 19, were you thinking
    >> we'd also backpatch that code directly to 18?
    > 
    > Sure.
    
    This seems like a reasonable plan to me.
    
    -- 
    nathan
    
    
    
    
  10. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Melanie Plageman <melanieplageman@gmail.com> — 2026-06-17T15:50:57Z

    On Wed, Jun 17, 2026 at 11:40 AM Nathan Bossart
    <nathandbossart@gmail.com> wrote:
    >
    > >> Fair enough. If the RMT is okay with this for 19, were you thinking
    > >> we'd also backpatch that code directly to 18?
    > >
    > > Sure.
    >
    > This seems like a reasonable plan to me.
    
    +1
    
    
    
    
  11. Re: [oauth] Increased CPU usage during device flow with libcurl 8.20.0

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-17T20:57:05Z

    On Wed, Jun 17, 2026 at 8:51 AM Melanie Plageman
    <melanieplageman@gmail.com> wrote:
    > On Wed, Jun 17, 2026 at 11:40 AM Nathan Bossart
    > <nathandbossart@gmail.com> wrote:
    >
    > > This seems like a reasonable plan to me.
    >
    > +1
    
    Great, thanks everyone! I've pushed a patchset based on v1, after some
    additional testing.
    
    --Jacob