On Tue, Aug 02, 2005 at 03:23:44PM +0200, Martin Kraemer wrote:
> On Tue, Aug 02, 2005 at 12:00:24PM +0100, Joe Orton wrote:
> >> 1) this is a pretty specific to way to code it. Is there no way to make
> >> it more general so that OID() is just a function like file() and can be
> >> used e.g. in regex matches too?
>
> The problem with the OID() "function" is that it where file() (or
> another file() like function) return a single value, what OID()
> stands for is an "array of zero or more values". But there is no
> syntax to deal with arrays in place of expressions. I tried to
> implement it as function first, but noticed that it would break when
> an OID was specified more than once. In the ASF scenario, the
> intention is to add multiple extensions with this OID, each one
> containing as value a group name of which the client is member.
>
> Because of the pre-existing syntax "<expr> in {value,value}", and
> because "{value,value}" is effectively an array, I chose to implement
> the OID() "function" as a special case of the "<expr> in" operator.
>
> Do you have a good idea how to use a function-like syntax, and still
> maintain the "is an array" property?
No, OK, that makes sense. Perhaps a more general implementation would
be to have a "peerextlist()" which evalutes to a "wordlist" array like
the current function, along with a "peerext()" which evaluates to a
"word" (just the first extension with given name).
> >> 3) oid() is a terrible name for this; it's simply the type of the
> >> parameter. It would be like calling malloc() "size()". The function
> >> expands (conceptually) to the values of an extension in the peer's
> >> certificate, identified by OID; so call it peerext() or something
> >> meaningful like that.
>
> Good point - Thanks a lot -- that is a *very* good idea, and (if
> nobody objects) I'd want to follow this suggestion. I had been a
> little unhappy with OID() myself. peerext() is especially good
> because it also documents where the certificate came from.
I'd prefer peerexts() or peerextlist() given the above, to emphasize the
the fact that it returns a list, and to allow a peerext() which doesn't
do that, in the future.
> >> > SetEnvIf OID("2.16.840.1.113730.1.13") "(.*) Generated (Certificate)"
> >> > ca=$1
> >>
> >> -1 on the naming since OID is completely entirely meaningless in this
> >> context.
>
> In the context of mod_setenvif, I'd even use "SSLPeerExt()" because
> it makes it clear that we are dealing with an SSL-related thing.
>
> Patch <<mod_setenvif.c.patch>> attached.
Likewise on naming; sslpeerexts() or sslpeerextlist() seems better
although it's perhaps getting a little long.
Also note that apr_array_pstrcat(pool, array, ',') can be used instead
of all that code to flatten the array to a string.
> In <<ssl_peerext.patch>> there is a patch which changes OID()
> to SSLPeerExt() for mod_ssl.
Other than the choice of name, this looks fine. Please split out the
unrelated change to match tokens case-insensitively to a separate
commit.
Regards,
joe
