You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2011/07/26 18:48:51 UTC
svn commit: r1151165 - in /subversion/branches/1.7.x: ./ STATUS
subversion/bindings/swig/perl/native/t/8lock.t
subversion/include/svn_error.h subversion/libsvn_fs/fs-loader.c
subversion/libsvn_repos/hooks.c subversion/tests/cmdline/lock_tests.py
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__':