Index: subversion/include/private/svn_opt_private.h =================================================================== --- subversion/include/private/svn_opt_private.h (revision 30798) +++ subversion/include/private/svn_opt_private.h (working copy) @@ -34,7 +34,7 @@ * - check that only valid URI characters remain * - check that no back-path ("..") components are present * - canonicalize the separator ("/") characters - * URL_IN is in UTF-8 encoding and has no peg revision specifier. + * URL_IN is in UTF-8 encoding and can have a peg revision specifier. * Set *URL_OUT to the result, allocated from POOL. */ svn_error_t * @@ -50,7 +50,7 @@ * - If the path does not exist (which is valid) the given capitialization * is used. * - canonicalize the separator ("/") characters - * PATH_IN is in UTF-8 encoding and has no peg revision specifier. + * PATH_IN is in UTF-8 encoding and can have a peg revision specifier. * Set *PATH_OUT to the result, allocated from POOL. */ svn_error_t * Index: subversion/libsvn_subr/opt.c =================================================================== --- subversion/libsvn_subr/opt.c (revision 30798) +++ subversion/libsvn_subr/opt.c (working copy) @@ -946,40 +946,8 @@ for (i = 0; i < input_targets->nelts; i++) { const char *utf8_target = APR_ARRAY_IDX(input_targets, i, const char *); - const char *peg_start = NULL; /* pointer to the peg revision, if any */ const char *target; /* after all processing is finished */ - int j; - /* Remove a peg revision, if any, in the target so that it can - be properly canonicalized, otherwise the canonicalization - does not treat a ".@BASE" as a "." with a BASE peg revision, - and it is not canonicalized to "@BASE". If any peg revision - exists, it is appended to the final canonicalized path or - URL. Do not use svn_opt_parse_path() because the resulting - peg revision is a structure that would have to be converted - back into a string. Converting from a string date to the - apr_time_t field in the svn_opt_revision_value_t and back to - a string would not necessarily preserve the exact bytes of - the input date, so its easier just to keep it in string - form. */ - for (j = (strlen(utf8_target) - 1); j >= 0; --j) - { - /* If we hit a path separator, stop looking. This is OK - only because our revision specifiers can't contain - '/'. */ - if (utf8_target[j] == '/') - break; - if (utf8_target[j] == '@') - { - peg_start = utf8_target + j; - break; - } - } - if (peg_start) - utf8_target = apr_pstrmemdup(pool, - utf8_target, - peg_start - utf8_target); - /* URLs and wc-paths get treated differently. */ if (svn_path_is_url(utf8_target)) { @@ -987,13 +955,19 @@ } else /* not a url, so treat as a path */ { - const char *base_name; + char *base_name; + char *peg_rev; SVN_ERR(svn_opt__arg_canonicalize_path(&target, utf8_target, pool)); /* If the target has the same name as a Subversion working copy administrative dir, skip it. */ base_name = svn_path_basename(target, pool); + + /* "Delete" the peg revision */ + if (peg_rev = strrchr(base_name, '@')) + *peg_rev = '\0'; + /* FIXME: The canonical list of administrative directory names is maintained in libsvn_wc/adm_files.c:svn_wc_set_adm_dir(). @@ -1011,11 +985,6 @@ } } - /* Append the peg revision back to the canonicalized target if - there was a peg revision. */ - if (peg_start) - target = apr_pstrcat(pool, target, peg_start, NULL); - APR_ARRAY_PUSH(output_targets, const char *) = target; } @@ -1114,14 +1083,75 @@ return SVN_NO_ERROR; } +/* Extract the peg revision, if any, from UTF8_TARGET. Return the + * peg revision in *PEG_REVISION and the true target portion + * in *TRUE_TARGET. + * + * This is needed so that UTF8_TARGET can be properly canonicalized, + * otherwise the canonicalization does not treat a ".@BASE" as a "." + * with a BASE peg revision, and it is not canonicalized to "@BASE". + * If any peg revision exists, it is appended to the final + * canonicalized path or URL. Do not use svn_opt_parse_path() + * because the resulting peg revision is a structure that would have + * to be converted back into a string. Converting from a string date + * to the apr_time_t field in the svn_opt_revision_value_t and back to + * a string would not necessarily preserve the exact bytes of the + * input date, so its easier just to keep it in string form. + * + * All allocations are done in POOL. + */ +static svn_error_t * +split_arg_at_peg_revision(const char **true_target, + const char **peg_revision, + const char *utf8_target, + apr_pool_t *pool) +{ + const char *peg_start = NULL; /* pointer to the peg revision, if any */ + int j; + + for (j = (strlen(utf8_target) - 1); j >= 0; --j) + { + /* If we hit a path separator, stop looking. This is OK + only because our revision specifiers can't contain + '/'. */ + if (utf8_target[j] == '/') + break; + if (utf8_target[j] == '@') + { + peg_start = utf8_target + j; + break; + } + } + + if (peg_start) + { + *true_target = apr_pstrmemdup(pool, + utf8_target, + peg_start - utf8_target); + + *peg_revision = apr_pstrdup(pool, peg_start); + } + else + { + *true_target = utf8_target; + + *peg_revision = NULL; + } + + return SVN_NO_ERROR; +} + svn_error_t * svn_opt__arg_canonicalize_url(const char **url_out, const char *url_in, apr_pool_t *pool) { const char *target; + const char *peg_rev; + SVN_ERR(split_arg_at_peg_revision(&target, &peg_rev, url_in, pool)); + /* Convert to URI. */ - target = svn_path_uri_from_iri(url_in, pool); + target = svn_path_uri_from_iri(target, pool); /* Auto-escape some ASCII characters. */ target = svn_path_uri_autoescape(target, pool); @@ -1140,6 +1170,11 @@ /* strip any trailing '/' and collapse other redundant elements */ target = svn_path_canonicalize(target, pool); + /* Append the peg revision back to the canonicalized target if + there was a peg revision. */ + if (peg_rev) + target = apr_pstrcat(pool, target, peg_rev, NULL); + *url_out = target; return SVN_NO_ERROR; } @@ -1151,9 +1186,12 @@ const char *apr_target; char *truenamed_target; /* APR-encoded */ apr_status_t apr_err; + const char *peg_rev; + SVN_ERR(split_arg_at_peg_revision(&apr_target, &peg_rev, path_in, pool)); + /* canonicalize case, and change all separators to '/'. */ - SVN_ERR(svn_path_cstring_from_utf8(&apr_target, path_in, pool)); + SVN_ERR(svn_path_cstring_from_utf8(&apr_target, apr_target, pool)); apr_err = apr_filepath_merge(&truenamed_target, "", apr_target, APR_FILEPATH_TRUENAME, pool); @@ -1174,6 +1212,11 @@ SVN_ERR(svn_path_cstring_to_utf8(path_out, apr_target, pool)); *path_out = svn_path_canonicalize(*path_out, pool); + /* Append the peg revision back to the canonicalized target if + there was a peg revision. */ + if (peg_rev) + *path_out = apr_pstrcat(pool, *path_out, peg_rev, NULL); + return SVN_NO_ERROR; } Index: subversion/libsvn_client/cmdline.c =================================================================== --- subversion/libsvn_client/cmdline.c (revision 30798) +++ subversion/libsvn_client/cmdline.c (working copy) @@ -56,6 +56,10 @@ * REPOS_ROOT_URL is the absolute URL of the repository root. * All strings are in UTF-8 encoding. * Allocate *ABSOLUTE_URL in POOL. + * + * REPOS_ROOT_URL and RELATIVE_URL do not have to be properly URI-encoded, + * canonical, or valid in any other way. The caller is expected to perform + * canonicalization on *ABSOLUTE_URL after the call to the function. */ static svn_error_t * resolve_repos_relative_url(const char **absolute_url, @@ -68,7 +72,11 @@ _("Improper relative URL '%s'"), relative_url); - *absolute_url = svn_path_join(repos_root_url, relative_url + 2, pool); + /* No assumptions are made about the canonicalization of the input + * arguments, it is presumed that the output will be canonicalized after + * this function, which will remove any duplicate path seperator. + */ + *absolute_url = apr_pstrcat(pool, repos_root_url, relative_url + 1, NULL); return SVN_NO_ERROR; } @@ -188,39 +196,8 @@ for (i = 0; i < input_targets->nelts; i++) { const char *utf8_target = APR_ARRAY_IDX(input_targets, i, const char *); - const char *peg_start = NULL; /* pointer to the peg revision, if any */ const char *target; - int j; - /* Remove a peg revision, if any, in the target so that it can - be properly canonicalized, otherwise the canonicalization - does not treat a ".@BASE" as a "." with a BASE peg revision, - and it is not canonicalized to "@BASE". If any peg revision - exists, it is appended to the final canonicalized path or - URL. Do not use svn_opt_parse_path() because the resulting - peg revision is a structure that would have to be converted - back into a string. Converting from a string date to the - apr_time_t field in the svn_opt_revision_value_t and back to - a string would not necessarily preserve the exact bytes of - the input date, so its easier just to keep it in string - form. */ - for (j = (strlen(utf8_target) - 1); j >= 0; --j) - { - /* If we hit a path separator, stop looking. This is OK - only because our revision specifiers can't contain - '/'. */ - if (utf8_target[j] == '/') - break; - if (utf8_target[j] == '@') - { - peg_start = utf8_target + j; - break; - } - } - if (peg_start) - utf8_target = apr_pstrmemdup(pool, - utf8_target, - peg_start - utf8_target); /* Relative urls will be canonicalized when they are resolved later in * the function @@ -239,7 +216,8 @@ } else /* not a url, so treat as a path */ { - const char *base_name; + char *base_name; + char *peg_rev; SVN_ERR(svn_opt__arg_canonicalize_path(&target, utf8_target, pool)); @@ -248,6 +226,12 @@ working copy administrative dir, skip it. */ base_name = svn_path_basename(target, pool); + /* "Delete" the peg revision because svn_wc_is_adm_dir() cannot + * handle it. + */ + if (peg_rev = strrchr(base_name, '@')) + *peg_rev = '\0'; + if (svn_wc_is_adm_dir(base_name, pool)) { err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED, @@ -258,11 +242,6 @@ } } - /* Append the peg revision back to the canonicalized target if - there was a peg revision. */ - if (peg_start) - target = apr_pstrcat(pool, target, peg_start, NULL); - if (rel_url_found) { SVN_ERR(check_root_url_of_target(&root_url, target, Index: subversion/tests/cmdline/basic_tests.py =================================================================== --- subversion/tests/cmdline/basic_tests.py (revision 30798) +++ subversion/tests/cmdline/basic_tests.py (working copy) @@ -2291,7 +2291,81 @@ svntest.verify.AnyOutput, 'blame', '^/A/mu', iota_url_repo1, iota_url_repo2) +def basic_relative_url_non_canonical(sbox): + "basic relative url non-canonical targets" + sbox.build() + + # First, make a new revision of iota. + iota = os.path.join(sbox.wc_dir, 'iota') + svntest.main.file_append(iota, "New contents for iota\n") + svntest.main.run_svn(None, 'ci', + '-m', '', iota) + + # Now, make a new revision of A/mu . + mu = os.path.join(sbox.wc_dir, 'A', 'mu') + mu_url = sbox.repo_url + '/A/mu' + + svntest.main.file_append(mu, "New contents for mu\n") + svntest.main.run_svn(None, 'ci', + '-m', '', mu) + + + expected_output = [ + " 1 jrandom This is the file 'iota'.\n", + " 2 jrandom New contents for iota\n", + " 1 jrandom This is the file 'mu'.\n", + " 3 jrandom New contents for mu\n", + ] + + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'blame', + '^/iota/', mu_url) + + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'blame', + '^//iota/', mu_url) + +def basic_relative_url_with_peg_revisions(sbox): + "basic relative url targets with peg revisions" + + sbox.build() + + # First, make a new revision of iota. + iota = os.path.join(sbox.wc_dir, 'iota') + svntest.main.file_append(iota, "New contents for iota\n") + svntest.main.run_svn(None, 'ci', + '-m', '', iota) + + iota_url = sbox.repo_url + '/iota'; + + # Now, make a new revision of A/mu . + mu = os.path.join(sbox.wc_dir, 'A', 'mu') + mu_url = sbox.repo_url + '/A/mu' + + svntest.main.file_append(mu, "New contents for mu\n") + svntest.main.run_svn(None, 'ci', '-m', '', mu) + + # Delete the file from the current revision + svntest.main.run_svn(None, 'rm', '-m', '', iota_url) + + expected_output = [ + " 1 jrandom This is the file 'iota'.\n", + " 2 jrandom New contents for iota\n", + " 1 jrandom This is the file 'mu'.\n", + " 3 jrandom New contents for mu\n", + ] + + # Canonical version with peg revision + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'blame', '-r3', + '^/iota@3', mu_url) + + # Non-canonical version with peg revision + exit_code, output, error = svntest.actions.run_and_verify_svn(None, + expected_output, [], 'blame', '-r3', + '^//iota/@3', mu_url) + #---------------------------------------------------------------------- ######################################################################## @@ -2342,6 +2416,8 @@ basic_relative_url_using_current_dir, basic_relative_url_using_other_targets, basic_relative_url_multi_repo, + basic_relative_url_non_canonical, + basic_relative_url_with_peg_revisions, ] if __name__ == '__main__':