Re: [PATCH v7 11/12] http: read HTTP WWW-Authenticate response headers

From: Jeff King <peff@peff.net>
Date: Feb 9, 2023, 13:12 UTC
Message-ID: <Y+TxRDatM9Iirwwu@coredump.intra.peff.net>
In-Reply-To: <DB9PR03MB983150E73B1C963C628CBE75C0DA9@DB9PR03MB9831.eurprd03.prod.outlook.com>

On Mon, Feb 06, 2023 at 11:25:49AM -0800, Matthew John Cheetham wrote:

Show 16 quoted lines

> > I guess I would have expected some level of abstraction here between the
> > credential subsystem and the http subsystem, where the latter is parsing
> > and then sticking opaque data into the credential to ferry to the
> > helpers.
> > 
> > But it probably isn't that big a deal either way. Even though there are
> > non-http credentials, it's not too unreasonable for the credential
> > system to be aware of http specifically.
> 
> I had considered possibly introducing an opaque property-bag style of
> 'protocol-specific properties' that, for example, http.c would add the
> WWW-Authenticate headers to as something like `http.wwwauth[]`.
> Other protocols (like smtp:// or cert://) could add their own properties
> if they needed or wanted to also.
> 
> Thoughts?

At the protocol level, I don't see much point. wwwauth sufficiently implies "http", and any helper is free to ignore or respect keys as appropriate to what it can handle. A flat namespace is fine.

Here I was more talking about the internal implementation. Mostly it was just funky that this internal http state flag was stuck into the credential struct. I think it could be removed with some minor pain, but it's probably not too big a deal (the pain at all is only because we are having to bring this state across multiple curl callbacks).

So let's go with it for now.

-Peff