Joe Orton wrote: > On Sat, Sep 10, 2005 at 02:47:17AM +0100, David Reid wrote: > >>Following patch makes some changes to ssl_ext_lookup and changes it's >>API, hence the post for review. >> >>Add some more warnings when things don't go as advertised. > > > I don't think it's appropriate to log warnings (at least at > APLOG_WARNING level) from a function like this - only the caller knows > whether or not failures require user-visibile warnings or not.
OK. I think it makes sense to log them at some level though as otherwise there is no indication what's wrong. When testing these caught me out for a while, hence my additions. Maybe INFO would be a better level to log at? > > >>We now allow the "known" names to be used as extensions to lookup >>expanding the flexability of the function. >> >>Add an index to allow repeated calls to get different values to handle >>the case when the same extension is present multiple times (there is no >>restriction how often they can appear I'm aware of). > > > Use of the index seems fine though this is starting to overlap in > functionality with the ssl_extlist_by_oid function? Hmm, maybe. Maybe ssl_extlist could just call ssl_ext_lookup? > >>X509V3_EXT_print seems to have trouble printing some simple strings and >>despite having a default fallback it's still not able to decode them, so >>we allow a plain return of the data. This could also (concievably) be a >>small binary section, so we return the length to allow the caller to >>know how much data is provided. This can probably be improved on. > > > This is similar to what Dirk proposed recently, right? I'll reiterate > my concern with this: if OpenSSL cannot return a human-readable char * > representation of this extension value then this will instead produce > some unusable binary blob? This interface is supposed to return char * > strings not binary blobs. Right. I agree with you, but the function we're using fails to cope with even a simple string, so we need another solution. I'll try and come up with an alternative... david
