httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Covener" <cove...@gmail.com>
Subject Re: svn commit: r678179 - /httpd/httpd/branches/2.2.x/STATUS
Date Mon, 21 Jul 2008 12:51:54 GMT
On Mon, Jul 21, 2008 at 8:37 AM, Nick Kew <nick@webthing.com> wrote:
> On Mon, 21 Jul 2008 08:09:23 -0400
> "Eric Covener" <covener@gmail.com> wrote:
>> Is it kosher to pull the API extension from trunk? A less intrusive
>> (AuthnProviderAlias-only) solution occurred to me while reading your
>> note.  Would the (incorrect) minor bump be left in place or can it be
>> reverted?
>
> I'd say it wants a major bump that'll draw the line under likely
> breakage.  Fortunately that's allowed in /trunk/.
>

For clarification, you're saying a major bump to remove
has_realm_hash() right in favor of a simpler solution?

> I should add: my first thought on reviewing this was that has_* should
> be a bitfield so it could be generalised to other methods.  But I
> still don't see why just testing whether the get_realm_hash member
> is NULL isn't sufficient for you.

There was an existing test at initialization, but with
AuthnProviderAlias get_realm_hash() is always non-NULL.

The following didn't occur to me before the patch under review.  We
can confine the changes to AuthAliasProvider and still catch it at
initialization time (no real patch yet)

static const authn_provider authn_alias_provider =
{
    &authn_alias_check_password,
    &authn_alias_get_realm_hash,
};

static const authn_provider authn_alias_provider_nodigest =
{
    &authn_alias_check_password,
    NULL,
};

static const char *authaliassection(cmd_parms *cmd, void *mconfig,
const char *arg) {
...
ap_register_auth_provider( ...
                                  provider->get_realm_hash ?
&authn_alias_provider : &authn_alias_provider_nodigest,
...

This would satisfy the existing test performed in mod_auth_digest.

-- 
Eric Covener
covener@gmail.com

Mime
View raw message