Re: [PATCH 0/8] [RFC] Enhance credential helper protocol to include auth headers

From: Derrick Stolee <derrickstolee@github.com>
Date: Sep 19, 2022, 16:44 UTC
Message-ID: <710c8868-800a-7140-0b47-74784b1e2b1d@github.com>
In-Reply-To: <452acd3b-14bd-a3c8-df58-39b5e9edfd0b@github.com>

On 9/19/2022 12:08 PM, Derrick Stolee wrote:

> On 9/13/2022 3:25 PM, Matthew John Cheetham via GitGitGadget wrote:

Show 18 quoted lines

>> protocol=https
>> host=example.com
>> wwwauth[0]=Bearer realm="login.example", scope="git.readwrite"
>> wwwauth[1]=Basic realm="login.example"
> 
> The important part here is that we provide a way to specify a multi-valued
> key as opposed to a "last one wins" key, right?
> 
> Using empty braces (wwwauth[]) would suffice to indicate this, right? That
> allows us to not care about the values inside the braces. The biggest
> issues I see with a value in the braces are:
> 
> 1. What if it isn't an integer?
> 2. What if we are missing a value?
> 3. What if they come out of order?
> 
> Without a value inside, then the order in which they appear provides
> implicit indices in their multi-valued list.

After looking at the code, it would not be difficult at all to make this
change in-place for these patches. But I won't push too hard if there is
some reason to keep the index values.

> Other than that, I support this idea and will start looking at the code
> now.

I took a look and provided feedback as I could. Patches 6 and 7 eluded me only because I'm so unfamiliar with the http.c code and don't have time to learn it today.

I mentioned that patches 1-3 could easily be picked up as a topic while the rest of the series is considered carefully.

I tried to add some mentions of testing, but you've already tested more than I expected, by adding the headers to the Apache output.

Thanks, -Stolee