danielsh_at_tigris.org writes:
> Log:
> In svnserve, tolerate unreadable passwd files.
>
> Review by: ghudson
> Found by: Kristian Kauper <kkauper_at_au.yahoo-inc.com>
>
> * subversion/svnserve/serve.c
> (load_configs): Skip reading the passwd file if it is unreadable (EACCES),
> logging the error but allowing other authentications (such as --tunnel)
> to continue.
+1, but a note about an "XXX" comment you left:
> --- trunk/subversion/svnserve/serve.c (r30867)
> +++ trunk/subversion/svnserve/serve.c (r30868)
> @@ -236,7 +236,14 @@ svn_error_t *load_configs(svn_config_t *
> if (server)
> /* Called by listening server; log error no matter what it is. */
> log_server_error(err, server, conn, pool);
> - if (err->apr_err != SVN_ERR_BAD_FILENAME)
> +
> + /* XXX: why do ignore SVN_ERR_BAD_FILENAME here? (see r16840)
> + *
> + * If the passwd file is unreadable, skip reading it and continue;
> + * some authentications (e.g. --tunnel) can proceed without it.
> + */
> + if (err->apr_err != SVN_ERR_BAD_FILENAME
> + && ! APR_STATUS_IS_EACCES(err->apr_err))
> {
> if (server)
> {
If we look at the larger block around that, it starts off with a comment
that reiterates what r16840's log message says:
/* Because it may be possible to read the pwdb file with some
* access methods and not others, ignore errors reading the pwdb
* file and just don't present password authentication as an
* option. */
err = svn_config_read(pwdb, pwdb_path, TRUE, pool);
if (err)
{
if (server)
/* Called by listening server; log error no matter what it is. */
log_server_error(err, server, conn, pool);
/* XXX: why do ignore SVN_ERR_BAD_FILENAME here? (see r16840)
*
* If the passwd file is unreadable, skip reading it and continue;
* some authentications (e.g. --tunnel) can proceed without it.
*/
if (err->apr_err != SVN_ERR_BAD_FILENAME
&& ! APR_STATUS_IS_EACCES(err->apr_err))
{
if (server)
{
/* Called by listening server: Now that we've logged
* the error, clear it and return a nice, generic
* error to the user
* (http://subversion.tigris.org/issues/show_bug.cgi?id=2271). */
svn_error_clear(err);
return svn_error_create(SVN_ERR_AUTHN_FAILED, NULL, NULL);
}
/* Called during startup; return the error, whereupon it
* will go to standard error for the admin to see. */
return err;
}
else
/* Ignore SVN_ERR_BAD_FILENAME and APR_EACCES and proceed. */
svn_error_clear(err);
}
Is the question you're really asking: "Why did r16840 allow for
SVN_ERR_BAD_FILENAME but not APR_STATUS_IS_EACCES?" ?
If so, I don't know why. But at this point, it might make sense to
rearrange the comments to explain the current state of the code in the
clearest way possible, which I've attempted to do in r30871. Please
review to make sure I didn't let anything drop.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-01 00:01:11 CEST