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

From: Victoria Dye <vdye@github.com>
Date: Feb 8, 2023, 21:29 UTC
Message-ID: <9012b8a3-4abe-d3c6-41ee-f28931869ad7@github.com>
In-Reply-To: <pull.1352.v8.git.1675711789.gitgitgadget@gmail.com>

Matthew John Cheetham via GitGitGadget wrote:

Show 14 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.
>

The core functionality change in this series (passing through 'WWW-Authenticate' headers to credential requests) didn't change much between v7 and v8, and what was updated was a net improvement [1]. Otherwise, the new test infrastructure is concise and made for an easy re-review; I had one small suggestion for patch 1 [2], but it's not critical enough on its own to hold this series up. I think this could probably be merged to 'next' as-is, but a final re-roll with some of the minor fixups suggested in other reviews wouldn't hurt either.

Thanks again for the time & effort you've put into perfecting these patches!

[1] https://lore.kernel.org/git/983fc35b-55e8-50df-5035-191a10b4ddac@github.com/ [2] https://lore.kernel.org/git/87f79e79-1591-ca28-4975-3bca5b8f7266@github.com/