Re: [PATCH v8 0/3] Enhance credential helper protocol to include auth headers

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 6, 2023, 20:59 UTC
Message-ID: <230206.86357i5vki.gmgdl@evledraar.gmail.com>
In-Reply-To: <pull.1352.v8.git.1675711789.gitgitgadget@gmail.com>

On Mon, Feb 06 2023, Matthew John Cheetham via GitGitGadget wrote:

Show 13 quoted lines

> Updates in v8
> =============
>
>  * Drop custom HTTP test helper tool in favour of using a CGI shell script
>    and Apache; avoiding the need to implement an HTTP server.
>
>  * Avoid allocations in header reading callback unless we have a header we
>    care about; act on the char* from libcurl directly rather than create a
>    strbuf for each header.
>
>  * Drop st_mult overflow guarding function in curl callback functions; we're
>    not allocating memory based on the resulting value and just adds to
>    potential confusion in the future.

I just had some nit-y and other trivial comments spotted on a read-through, but with the caveat that I'm not too familiar with the credential infrastructure this looks good to me.

I'm rather neutral on the whole question of whether we eventually ship a httpd in-tree, but I think the v7 to v8 clearly demonstrates that whatever we do there, this topic is much improved by having that question un-tangled from the credential improvements here.

Thanks!