Thank you! I love this fast turnaround.
Best,
Blair
kfogel@tigris.org wrote:
>
> Author: kfogel
> Date: 2002-09-09 21:15:47 -0500 (Mon, 09 Sep 2002)
> New Revision: 3150
>
> Modified:
> trunk/subversion/include/svn_error_codes.h
> trunk/subversion/include/svn_io.h
> trunk/subversion/libsvn_ra_local/split_url.c
> trunk/subversion/libsvn_subr/io.c
> trunk/subversion/tests/clients/cmdline/log_tests.py
> Log:
> Fix bug whereby 'svn log file:///nonexistent_path' resulted in an
> infinite loop. Reported by Blair Zajac.
>
> * subversion/libsvn_ra_local/split_url.c
> (svn_ra_local__split_URL): Handle "/" specially.
>
> * subversion/tests/clients/cmdline/log_tests.py
> (non_existent_repository): Regression test for the fix.
> (test_list): Run it.
>
> Unrelatedly, tighten up some code a bit:
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_BAD_VERSION_FILE_FORMAT): New error code.
>
> * subversion/include/svn_io.h, subversion/libsvn_subr/io.c
> (svn_io_read_version_file): Check file format more carefully,
> redocument.
>
> Modified: trunk/subversion/include/svn_error_codes.h
> ==============================================================================
> --- trunk/subversion/include/svn_error_codes.h (original)
> +++ trunk/subversion/include/svn_error_codes.h Mon Sep 9 21:15:50 2002
> @@ -155,6 +155,10 @@
> SVN_ERR_BAD_CATEGORY_START + 5,
> "Bogus revision number")
>
> + SVN_ERRDEF (SVN_ERR_BAD_VERSION_FILE_FORMAT,
> + SVN_ERR_BAD_CATEGORY_START + 6,
> + "Version file format not correct")
> +
> /* xml errors */
>
> SVN_ERRDEF (SVN_ERR_XML_ATTRIB_NOT_FOUND,
>
> Modified: trunk/subversion/include/svn_io.h
> ==============================================================================
> --- trunk/subversion/include/svn_io.h (original)
> +++ trunk/subversion/include/svn_io.h Mon Sep 9 21:15:50 2002
> @@ -554,9 +554,10 @@
>
> /*** Version/format files. ***/
>
> -/* Read the file at PATH as a textfile that contains a single line of
> - text, formatted as a integer followed by a newline, and return that
> - integer in *VERSION. Use POOL for all allocations. */
> +/* Set *VERSION to the integer that starts the file at PATH. If the
> + file does not begin with a series of digits followed by a newline,
> + return the error SVN_ERR_BAD_VERSION_FILE_FORMAT. Use POOL for
> + all allocations. */
> svn_error_t *
> svn_io_read_version_file (int *version, const char *path, apr_pool_t *pool);
>
>
> Modified: trunk/subversion/libsvn_subr/io.c
> ==============================================================================
> --- trunk/subversion/libsvn_subr/io.c (original)
> +++ trunk/subversion/libsvn_subr/io.c Mon Sep 9 21:15:50 2002
> @@ -22,6 +22,7 @@
> #include <string.h>
> #include <assert.h>
> #include <errno.h>
> +#include <apr_lib.h>
> #include <apr_pools.h>
> #include <apr_file_io.h>
> #include <apr_file_info.h>
> @@ -1921,7 +1922,20 @@
> return svn_error_createf (SVN_ERR_STREAM_UNEXPECTED_EOF, 0, NULL, pool,
> "reading `%s'", path);
>
> - /* ...string to integer conversion... */
> + /* Check that the first line contains only digits. */
> + {
> + char *c;
> +
> + for (c = version_str->data; *c; c++)
> + {
> + if (! apr_isdigit (*c))
> + return svn_error_createf
> + (SVN_ERR_BAD_VERSION_FILE_FORMAT, 0, NULL, pool,
> + "first line of '%s' contains non-digit", path);
> + }
> + }
> +
> + /* Convert to integer. */
> *version = atoi (version_str->data);
>
> /* And finally, close the file. */
>
> Modified: trunk/subversion/libsvn_ra_local/split_url.c
> ==============================================================================
> --- trunk/subversion/libsvn_ra_local/split_url.c (original)
> +++ trunk/subversion/libsvn_ra_local/split_url.c Mon Sep 9 21:15:50 2002
> @@ -107,9 +107,23 @@
> /* Attempt to open a repository at URL. */
> err = svn_repos_open (&repos, candidate_url, subpool);
>
> - /* Hey, cool, we were successfully. Stop loopin'. */
> + /* Hey, cool, we were successful. Stop looping. */
> if (err == SVN_NO_ERROR)
> break;
> +
> + /* It would be strange indeed if "/" were a repository, but hey,
> + people do strange things sometimes. Anyway, if "/" failed
> + the test above, then reduce it to the empty string.
> +
> + ### I'm not sure whether SVN_EMPTY_PATH and SVN_PATH_IS_EMPTY
> + in libsvn_subr/path.c is are supposed to be changeable. If
> + they are, this code could conceivably break. If they're not,
> + then we should probably make SVN_PATH_EMPTY_PATH a public
> + constant and use it here. But either way, note that the test
> + for '/' below is kosher, because we know it's the canonical
> + APR separator. */
> + if ((candidate_url[0] == '/') && (candidate_url[1] == '\0'))
> + candidate_url = "";
>
> /* If we're down to an empty path here, and we still haven't
> found the repository, we're just out of luck. Time to bail
>
> Modified: trunk/subversion/tests/clients/cmdline/log_tests.py
> ==============================================================================
> --- trunk/subversion/tests/clients/cmdline/log_tests.py (original)
> +++ trunk/subversion/tests/clients/cmdline/log_tests.py Mon Sep 9 21:15:50 2002
> @@ -313,6 +313,33 @@
>
>
> #----------------------------------------------------------------------
> +def non_existent_repository(sbox):
> + "'svn log file:///nonexistent_path' should fail"
> +
> + # The original bug was that
> + #
> + # $ svn log file:///nonexistent_path
> + #
> + # would go into an infinite loop, instead of failing immediately as
> + # it should. So this test _always_ operates on a file:/// path, to
> + # make sure that bug stays fixed. Note that if someone runs this
> + # test on a system that has "/nonexistent_path" in the root
> + # directory, the test could fail, and that's just too bad :-).
> +
> + output, errput = svntest.main.run_svn (1, 'log', 'file:///nonexistent_path')
> +
> + if not errput:
> + return 1
> +
> + for line in errput:
> + if re.match(".*Unable to open an ra_local session to URL.*", line):
> + return 0
> +
> + # Else never matched the expected error output, so the test failed.
> + return 1
> +
> +
> +#----------------------------------------------------------------------
> def plain_log(sbox):
> "'svn log', no args, top of wc."
>
> @@ -414,6 +441,7 @@
>
> # list all tests here, starting with None:
> test_list = [ None,
> + non_existent_repository,
> plain_log,
> versioned_log_message,
> log_with_empty_repos,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 10 07:35:18 2002