Author: hwright Date: Tue Jul 26 16:48:50 2011 New Revision: 1151165 URL: http://svn.apache.org/viewvc?rev=1151165&view=rev Log: Merge r1147882, r1149343, r1149371, r1149372, r1149377, r1149398, r1149701, r1149713 from trunk:
* r1147882, r1149343, r1149371, r1149372, r1149377, r1149398, r1149701, r1149713 Validate consumer-supplied tokens in libsvn_fs. (This specifically applies to the output of pre-lock hook scripts.) Justification: Input validations are good; arbitrary lock tokens are bad, and can result in user-visible errors. Notes: r1147882 is reverted by r1149343. r1149343 reverts r1147882 and adds new code, which r1149371 rewrites. r1149371 is the change. r1149372 improves an error message. r1149377 is a typo fix. r1149398 fixes the bindings. r1149701 marks the test XFail for serf r1149713 correctly reports lock warnings and removes the XFail Votes: +1: danielsh, cmpilato, rhuijben Modified: subversion/branches/1.7.x/ (props changed) subversion/branches/1.7.x/STATUS subversion/branches/1.7.x/subversion/bindings/swig/perl/native/t/8lock.t subversion/branches/1.7.x/subversion/include/svn_error.h subversion/branches/1.7.x/subversion/libsvn_fs/fs-loader.c subversion/branches/1.7.x/subversion/libsvn_repos/hooks.c subversion/branches/1.7.x/subversion/tests/cmdline/lock_tests.py Propchange: subversion/branches/1.7.x/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Tue Jul 26 16:48:50 2011 @@ -54,4 +54,4 @@ /subversion/branches/tree-conflicts:868291-873154 /subversion/branches/tree-conflicts-notify:873926-874008 /subversion/branches/uris-as-urls:1060426-1064427 -/subversion/trunk:1146013,1146121,1146219,1146222,1146274,1146492,1146555,1146606,1146620,1146684,1146781,1146832,1146834,1146870,1146899,1146904,1147293,1147309,1148071,1148131,1148374,1148424,1148566,1148588,1148853,1148877,1148882,1148936,1149105,1149141,1149160,1149228,1149240,1149572,1149675,1151036 +/subversion/trunk:1146013,1146121,1146219,1146222,1146274,1146492,1146555,1146606,1146620,1146684,1146781,1146832,1146834,1146870,1146899,1146904,1147293,1147309,1147882,1148071,1148131,1148374,1148424,1148566,1148588,1148853,1148877,1148882,1148936,1149105,1149141,1149160,1149228,1149240,1149343,1149371-1149372,1149377,1149398,1149572,1149675,1149701,1149713,1151036 Modified: subversion/branches/1.7.x/STATUS URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/STATUS (original) +++ subversion/branches/1.7.x/STATUS Tue Jul 26 16:48:50 2011 @@ -183,24 +183,6 @@ Veto-blocked changes: Approved changes: ================= - * r1147882, r1149343, r1149371, r1149372, r1149377, r1149398, r1149701, r1149713 - Validate consumer-supplied tokens in libsvn_fs. (This specifically applies - to the output of pre-lock hook scripts.) - Justification: - Input validations are good; arbitrary lock tokens are bad, and can result - in user-visible errors. - Notes: - r1147882 is reverted by r1149343. - r1149343 reverts r1147882 and adds new code, which r1149371 rewrites. - r1149371 is the change. - r1149372 improves an error message. - r1149377 is a typo fix. - r1149398 fixes the bindings. - r1149701 marks the test XFail for serf - r1149713 correctly reports lock warnings and removes the XFail - Votes: - +1: danielsh, cmpilato, rhuijben - * r1150302 Avoid closing fs txns multiple times. Justification: Modified: subversion/branches/1.7.x/subversion/bindings/swig/perl/native/t/8lock.t URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/bindings/swig/perl/native/t/8lock.t?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/subversion/bindings/swig/perl/native/t/8lock.t (original) +++ subversion/branches/1.7.x/subversion/bindings/swig/perl/native/t/8lock.t Tue Jul 26 16:48:50 2011 @@ -54,10 +54,12 @@ print $stream 'orz'; } $txn->commit; -$fs->lock('/testfile', 'hate software', 'we hate software', 0, 0, $fs->youngest_rev, 0); +my $token = "opaquelocktoken:notauuid-$$"; + +$fs->lock('/testfile', $token, 'we hate software', 0, 0, $fs->youngest_rev, 0); ok(my $lock = $fs->get_lock('/testfile')); -is($lock->token, 'hate software'); +is($lock->token, $token); is($lock->owner, 'foo'); $acc = SVN::Fs::create_access('fnord'); @@ -65,7 +67,7 @@ is($acc->get_username, 'fnord'); $fs->set_access($acc); eval { -$fs->lock('/testfile', 'hate software', 'we hate software', 0, 0, $fs->youngest_rev, 0); +$fs->lock('/testfile', $token, 'we hate software', 0, 0, $fs->youngest_rev, 0); }; like($@, qr/already locked/); Modified: subversion/branches/1.7.x/subversion/include/svn_error.h URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/include/svn_error.h?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/subversion/include/svn_error.h (original) +++ subversion/branches/1.7.x/subversion/include/svn_error.h Tue Jul 26 16:48:50 2011 @@ -391,7 +391,8 @@ svn_error_t *svn_error_purge_tracing(svn #define SVN_ERR_IS_LOCK_ERROR(err) \ (err->apr_err == SVN_ERR_FS_PATH_ALREADY_LOCKED || \ err->apr_err == SVN_ERR_FS_NOT_FOUND || \ - err->apr_err == SVN_ERR_FS_OUT_OF_DATE) + err->apr_err == SVN_ERR_FS_OUT_OF_DATE || \ + err->apr_err == SVN_ERR_FS_BAD_LOCK_TOKEN) /** * Return TRUE if @a err is an error specifically related to unlocking Modified: subversion/branches/1.7.x/subversion/libsvn_fs/fs-loader.c URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_fs/fs-loader.c?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/subversion/libsvn_fs/fs-loader.c (original) +++ subversion/branches/1.7.x/subversion/libsvn_fs/fs-loader.c Tue Jul 26 16:48:50 2011 @@ -29,6 +29,7 @@ #include <apr_thread_mutex.h> #include <apr_uuid.h> +#include "svn_ctype.h" #include "svn_types.h" #include "svn_dso.h" #include "svn_version.h" @@ -1280,6 +1281,31 @@ svn_fs_lock(svn_lock_t **lock, svn_fs_t _("Lock comment contains illegal characters")); } + /* Enforce that the token be an XML-safe URI. */ + if (token) + { + const char *c; + + if (strncmp(token, "opaquelocktoken:", 16)) + return svn_error_createf(SVN_ERR_FS_BAD_LOCK_TOKEN, NULL, + _("Lock token URI '%s' has bad scheme; " + "expected '%s'"), + token, "opaquelocktoken"); + + for (c = token; *c; c++) + if (! svn_ctype_isascii(*c)) + return svn_error_createf(SVN_ERR_FS_BAD_LOCK_TOKEN, NULL, + _("Lock token '%s' is not ASCII " + "at byte %u"), + token, (unsigned)(c - token)); + + /* strlen(token) == c - token. */ + if (! svn_xml_is_xml_safe(token, c - token)) + return svn_error_createf(SVN_ERR_FS_BAD_LOCK_TOKEN, NULL, + _("Lock token URI '%s' is not XML-safe"), + token); + } + if (expiration_date < 0) return svn_error_create (SVN_ERR_INCORRECT_PARAMS, NULL, Modified: subversion/branches/1.7.x/subversion/libsvn_repos/hooks.c URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_repos/hooks.c?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/subversion/libsvn_repos/hooks.c (original) +++ subversion/branches/1.7.x/subversion/libsvn_repos/hooks.c Tue Jul 26 16:48:50 2011 @@ -617,8 +617,11 @@ svn_repos__hooks_pre_lock(svn_repos_t *r SVN_ERR(run_hook_cmd(&buf, SVN_REPOS__HOOK_PRE_LOCK, hook, args, NULL, pool)); + if (token) + /* No validation here; the FS will take care of that. */ *token = buf->data; + } else if (token) *token = ""; Modified: subversion/branches/1.7.x/subversion/tests/cmdline/lock_tests.py URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/tests/cmdline/lock_tests.py?rev=1151165&r1=1151164&r2=1151165&view=diff ============================================================================== --- subversion/branches/1.7.x/subversion/tests/cmdline/lock_tests.py (original) +++ subversion/branches/1.7.x/subversion/tests/cmdline/lock_tests.py Tue Jul 26 16:48:50 2011 @@ -1,4 +1,5 @@ #!/usr/bin/env python +# encoding=utf-8 # # lock_tests.py: testing versioned properties # @@ -1720,6 +1721,27 @@ def block_unlock_if_pre_unlock_hook_fail 1, 'unlock', pi_path) svntest.actions.run_and_verify_status(wc_dir, expected_status) +#---------------------------------------------------------------------- +def lock_invalid_token(sbox): + "verify pre-lock hook returning invalid token" + + sbox.build() + + hook_path = os.path.join(sbox.repo_dir, 'hooks', 'pre-lock') + svntest.main.create_python_hook_script(hook_path, + '# encoding=utf-8\n' + 'import sys\n' + 'sys.stdout.write("ÑеÑÑ")\n' + 'sys.exit(0)\n') + + fname = 'iota' + file_path = os.path.join(sbox.wc_dir, fname) + + svntest.actions.run_and_verify_svn2(None, None, + "svn: warning: W160037: " \ + ".*scheme.*'opaquelocktoken'", 0, + 'lock', '-m', '', file_path) + ######################################################################## # Run the tests @@ -1768,6 +1790,7 @@ test_list = [ None, cp_isnt_ro, update_locked_deleted, block_unlock_if_pre_unlock_hook_fails, + lock_invalid_token, ] if __name__ == '__main__':