You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/03/27 12:55:11 UTC
svn commit: r1461542 -
/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Author: stsp
Date: Wed Mar 27 11:55:10 2013
New Revision: 1461542
URL: http://svn.apache.org/r1461542
Log:
Follow-up to r1461535:
* subversion/tests/libsvn_repos/repos-test.c
(filename_trailing_newline): Tweak this test to verify behaviour of
the repos layer, not the FS layer. Current consensus seems to be that
the FS API should allow such names, and the repos layer should reject
them. So expect a commit of the transaction via the repos API to fail,
rather than expecting the FS layer to reject the creation of filenames
with trailing newlines in transactions.
Modified:
subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Modified: subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c?rev=1461542&r1=1461541&r2=1461542&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Wed Mar 27 11:55:10 2013
@@ -3192,15 +3192,26 @@ filename_trailing_newline(const svn_test
svn_pool_clear(subpool);
/* Attempt to copy /foo to "/bar\n". This should fail. */
+ youngest_rev = 1;
SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
- err = svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool);
+ SVN_ERR(svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool));
+ err = svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn, subpool);
SVN_TEST_ASSERT(err != SVN_NO_ERROR);
+ svn_error_clear(err);
+ svn_pool_clear(subpool);
- /* Attempt to create a file /foo/baz\n. This should fail. */
- err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
+ /* Attempt to commit a file /foo/baz\n. This should fail. */
+ youngest_rev = 1;
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
+ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
+ SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
+ SVN_ERR(svn_fs_make_file(txn_root, "/foo/baz\n", subpool));
+ err = svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn, subpool);
SVN_TEST_ASSERT(err != SVN_NO_ERROR);
+ svn_error_clear(err);
+ svn_pool_clear(subpool);
return SVN_NO_ERROR;
}
Re: svn commit: r1461542 -
/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 27, 2013 at 04:55:46PM +0400, Ivan Zhakov wrote:
> I think easiest way to block these paths in svn_fs_make_file() not in
> commit_txn(). We already perform path validation in make_file().
Yes, that's right, and I'll do that now.
The reason I attempted to do this in repos_commit_txn() is that we
were planning to have the repos layer perform this check, instead
of having the FS layer do it.
See this post: http://svn.haxx.se/dev/archive-2013-03/0462.shtml
Re: svn commit: r1461542 - /subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Wed, Mar 27, 2013 at 3:55 PM, <st...@apache.org> wrote:
> Author: stsp
> Date: Wed Mar 27 11:55:10 2013
> New Revision: 1461542
>
> URL: http://svn.apache.org/r1461542
> Log:
> Follow-up to r1461535:
>
> * subversion/tests/libsvn_repos/repos-test.c
> (filename_trailing_newline): Tweak this test to verify behaviour of
> the repos layer, not the FS layer. Current consensus seems to be that
> the FS API should allow such names, and the repos layer should reject
> them. So expect a commit of the transaction via the repos API to fail,
> rather than expecting the FS layer to reject the creation of filenames
> with trailing newlines in transactions.
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c?rev=1461542&r1=1461541&r2=1461542&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Wed Mar 27 11:55:10 2013
> @@ -3192,15 +3192,26 @@ filename_trailing_newline(const svn_test
> svn_pool_clear(subpool);
>
> /* Attempt to copy /foo to "/bar\n". This should fail. */
> + youngest_rev = 1;
> SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
> SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
> SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
> - err = svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool);
> + SVN_ERR(svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool));
> + err = svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn, subpool);
> SVN_TEST_ASSERT(err != SVN_NO_ERROR);
> + svn_error_clear(err);
> + svn_pool_clear(subpool);
>
> - /* Attempt to create a file /foo/baz\n. This should fail. */
> - err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
> + /* Attempt to commit a file /foo/baz\n. This should fail. */
> + youngest_rev = 1;
> + SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
> + SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
> + SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
> + SVN_ERR(svn_fs_make_file(txn_root, "/foo/baz\n", subpool));
I think easiest way to block these paths in svn_fs_make_file() not in
commit_txn(). We already perform path validation in make_file().
--
Ivan Zhakov
Re: svn commit: r1461542 -
/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 27, 2013 at 01:16:58PM +0100, Bert Huijben wrote:
> > SVN_TEST_ASSERT(err != SVN_NO_ERROR);
> > + svn_error_clear(err);
> > + svn_pool_clear(subpool);
>
> I think there is some SVN_TEST_ macro that tests for a specific error code and then clears the error for you.
>
> That would be a more stable test than just assuming any error is ok.
Yes, my plan was to make the test check for a specific error code once
the implementation returns one.
RE: svn commit: r1461542 - /subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: woensdag 27 maart 2013 12:55
> To: commits@subversion.apache.org
> Subject: svn commit: r1461542 -
> /subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
>
> Author: stsp
> Date: Wed Mar 27 11:55:10 2013
> New Revision: 1461542
>
> URL: http://svn.apache.org/r1461542
> Log:
> Follow-up to r1461535:
>
> * subversion/tests/libsvn_repos/repos-test.c
> (filename_trailing_newline): Tweak this test to verify behaviour of
> the repos layer, not the FS layer. Current consensus seems to be that
> the FS API should allow such names, and the repos layer should reject
> them. So expect a commit of the transaction via the repos API to fail,
> rather than expecting the FS layer to reject the creation of filenames
> with trailing newlines in transactions.
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_re
> pos/repos-test.c?rev=1461542&r1=1461541&r2=1461542&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/tests/libsvn_repos/repos-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_repos/repos-test.c Wed Mar
> 27 11:55:10 2013
> @@ -3192,15 +3192,26 @@ filename_trailing_newline(const svn_test
> svn_pool_clear(subpool);
>
> /* Attempt to copy /foo to "/bar\n". This should fail. */
> + youngest_rev = 1;
> SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
> SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
> SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
> - err = svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool);
> + SVN_ERR(svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool));
> + err = svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn,
> subpool);
> SVN_TEST_ASSERT(err != SVN_NO_ERROR);
> + svn_error_clear(err);
> + svn_pool_clear(subpool);
>
> - /* Attempt to create a file /foo/baz\n. This should fail. */
> - err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
> + /* Attempt to commit a file /foo/baz\n. This should fail. */
> + youngest_rev = 1;
> + SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
> + SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
> + SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
> + SVN_ERR(svn_fs_make_file(txn_root, "/foo/baz\n", subpool));
> + err = svn_repos_fs_commit_txn(NULL, repos, &youngest_rev, txn,
> subpool);
> SVN_TEST_ASSERT(err != SVN_NO_ERROR);
> + svn_error_clear(err);
> + svn_pool_clear(subpool);
I think there is some SVN_TEST_ macro that tests for a specific error code and then clears the error for you.
That would be a more stable test than just assuming any error is ok.
Bert