You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rasmus Lerdorf <ra...@lerdorf.on.ca> on 1998/09/05 06:03:51 UTC
ap_md5() function is rather useless
The ap_md5() function is defined as:
API_EXPORT(char *) ap_md5(pool *p, unsigned char *string)
and it does a:
ap_MD5Update(&my_md5, string, strlen((const char *) string));
To put it politely, this sucks. Makes it useless for binary data. It
needs to have a string length argument added to it which is then passed to
ap_MD5Update().
Any objections to me fixing this? How many modules do you think I will
break?
-Rasmus
Re: ap_md5() function is rather useless
Posted by Martin Kraemer <ma...@mch.sni.de>.
On Sat, Sep 05, 1998 at 04:55:28PM +0100, Ben Laurie wrote:
> Rasmus Lerdorf wrote:
> > But it is a trivial change to make the current one binary safe.
>
> Sure, it makes sense, but the way to do it is this:
>
> API_EXPORT(char *) ap_md5_binary(pool *p, unsigned char *buf,int len)
> {
> ...
> }
>
> API_EXPORT(char *) ap_md5(pool *p, unsigned char *string)
> {
> return ap_md5_binary(p,string,strlen(string));
> }
+1 for this change. That even better documents that ap_md5 was never
meant to be called for binary data.
Martin
--
| S I E M E N S | <Ma...@mch.sni.de> | Siemens Nixdorf
| ------------- | Voice: +49-89-636-46021 | Informationssysteme AG
| N I X D O R F | FAX: +49-89-636-44994 | 81730 Munich, Germany
~~~~~~~~~~~~~~~~My opinions only, of course; pgp key available on request
Re: ap_md5() function is rather useless
Posted by Ben Laurie <be...@algroup.co.uk>.
Rasmus Lerdorf wrote:
>
> > Huh? ap_md5() is for strings. C strings. That's why the name of the
> > argument is "string". That's why the comment in the function says "Take
> > the Md5 hash of the string argument." That's why it uses ap_pstrdup. Which
> > also takes strings. I'd guess half that of C functions existing
> > in the world today work on strings. If you pass them binary data, they
> > don't work. That doesn't mean they are buggy. Just that they only work on
> > null-terminated C strings.
> >
> > If you want a version of ap_md5() that works on non-strings, then write
> > one. I don't see the big deal.
>
> But it is a trivial change to make the current one binary safe. Sort of
> silly to restrict it to strings only where there is no decent reason to.
> I don't really care though. It was more as a convenience to other module
> writers. I have my own binary-safe md5() function.
Sure, it makes sense, but the way to do it is this:
API_EXPORT(char *) ap_md5_binary(pool *p, unsigned char *buf,int len)
{
...
}
API_EXPORT(char *) ap_md5(pool *p, unsigned char *string)
{
return ap_md5_binary(p,string,strlen(string));
}
or even #define ap_md5 (but there's a small danger from side-effects if
you do that). Then you get the function you want _and_ retain upwards
compatibility _and_ avoid code duplication.
Cheeers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org/
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache/
WE'RE RECRUITING! http://www.aldigital.co.uk/
Re: ap_md5() function is rather useless
Posted by Rasmus Lerdorf <ra...@lerdorf.on.ca>.
> Huh? ap_md5() is for strings. C strings. That's why the name of the
> argument is "string". That's why the comment in the function says "Take
> the Md5 hash of the string argument." That's why it uses ap_pstrdup. Which
> also takes strings. I'd guess half that of C functions existing
> in the world today work on strings. If you pass them binary data, they
> don't work. That doesn't mean they are buggy. Just that they only work on
> null-terminated C strings.
>
> If you want a version of ap_md5() that works on non-strings, then write
> one. I don't see the big deal.
But it is a trivial change to make the current one binary safe. Sort of
silly to restrict it to strings only where there is no decent reason to.
I don't really care though. It was more as a convenience to other module
writers. I have my own binary-safe md5() function.
-Rasmus
Re: ap_md5() function is rather useless
Posted by Alexei Kosut <ak...@leland.Stanford.EDU>.
On Sat, 5 Sep 1998, Rasmus Lerdorf wrote:
> > Why not just add a new one and leave the current API as is?
>
> Like an ap_md5_binary_safe() function? That's like a
> ap_function_with_no_bugs() vs. ap_function_with_lots_of_bugs()
Huh? ap_md5() is for strings. C strings. That's why the name of the
argument is "string". That's why the comment in the function says "Take
the Md5 hash of the string argument." That's why it uses ap_pstrdup. Which
also takes strings. I'd guess half that of C functions existing
in the world today work on strings. If you pass them binary data, they
don't work. That doesn't mean they are buggy. Just that they only work on
null-terminated C strings.
If you want a version of ap_md5() that works on non-strings, then write
one. I don't see the big deal.
-- Alexei Kosut <ak...@stanford.edu> <http://www.stanford.edu/~akosut/>
Stanford University, Class of 2001 * Apache <http://www.apache.org> *
Re: ap_md5() function is rather useless
Posted by Rasmus Lerdorf <ra...@lerdorf.on.ca>.
> Why not just add a new one and leave the current API as is?
Like an ap_md5_binary_safe() function? That's like a
ap_function_with_no_bugs() vs. ap_function_with_lots_of_bugs()
I think I'll just turf the idea of using the Apache version of md5(). I
have my own implementation for the CGI version of PHP anyway. I'll just
use that for both CGI and module builds.
-Rasmus
Re: ap_md5() function is rather useless
Posted by Marc Slemko <ma...@worldgate.com>.
Mmm.
Why not just add a new one and leave the current API as is?
On Sat, 5 Sep 1998, Rasmus Lerdorf wrote:
> The ap_md5() function is defined as:
>
> API_EXPORT(char *) ap_md5(pool *p, unsigned char *string)
>
> and it does a:
>
> ap_MD5Update(&my_md5, string, strlen((const char *) string));
>
> To put it politely, this sucks. Makes it useless for binary data. It
> needs to have a string length argument added to it which is then passed to
> ap_MD5Update().
>
> Any objections to me fixing this? How many modules do you think I will
> break?
>
> -Rasmus
>