Re: svn commit: rev 5232 - in trunk/subversion: bindings/swig libsvn_ra_dav libsvn_ra_local

Philip Martin <philip@codematters.co.uk> writes:

> > if (err || ! creds)
> > - return -1;
> > + {
> > + svn_error_clear (err);
> > + return -1;
> > + }
> > + simple_creds = creds;
> >
> > /* ### silently truncates username/password to 256 chars. */
> > - apr_cpystrn(username, creds->username, NE_ABUFSIZ);
> > - apr_cpystrn(password, creds->password, NE_ABUFSIZ);
> > + apr_cpystrn(username, simple_creds->username, NE_ABUFSIZ);
> > + apr_cpystrn(password, simple_creds->password, NE_ABUFSIZ);
>
> While I was here I noticed that in this case there is no check for
> username/password being NULL, but below...

Hmmmm, interesting. At this point we know that creds is de-reffable,
but we don't know if the fields are NULL.

> ...in this case there is a check. I looked in svn_auth.h I could not
> determine whether it is the provider's responsibility to ensure that
> they are never NULL or the library's responsibility to check for NULL.

I think a provider that provides a non-NULL simple-creds structure
that contains NULL fields is broken. But that doesn't mean it
couldn't happen, if someone writes a sloppy provider. I think the
caller should double-check.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 7 16:17:22 2003