Re: [PATCH] Fix potential crash in libsvn_repos when executing pre-commit hook

Thanks for the review! I really messed up the test.

> How is that going to work? "exit 0" is not a valid hook script. It
> might work with an additional "/bin/sh" but that is not portable.

This will actually work on Windows, but yes, it isn't portable.

> Ah! The fs layer does not run any hooks, you need to use the repos
> layer's svn_repos_fs_commit_txn to run hooks. But that will not work
> because the hook is not portable.

I reworked the patch (see the attachment). The hook script setup code is
now rewritten in more portable way; test also uses the appropriate API
(svn_repos_fs_commit_txn()) to commit the transaction. The test crashes
in lock_token_content() without the fix.

I tested this on Windows and Linux machines.

Log message:
[[[
Fix potential crash in libsvn_repos when executing pre-commit hook.

* subversion/subversion/libsvn_repos/hooks.c
  (lock_token_content): Add special handling for 'magic' value
   ((const char *) 1).

* subversion/subversion/tests/libsvn_repos/repos-test.c
  (deprecated_access_context_api): New.
  (test_funcs): Add new test.

Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]]

> I wonder if we should be checking the path? The modified test below
> still passes, is that what we want?

There is at least one place where mod_dav_svn reconstructs svn_fs_access_t in a
hackish way:

[[[
    do {
        /* Note the path/lock pairs are only for lock token checking
           in access, and the relative path is not actually accurate
           as it contains the !svn bits. However, we're using only
           the tokens anyway (for access control). */

        serr = svn_fs_access_add_lock_token2(access_ctx, relative,
                                             list->locktoken->uuid_str);

        if (serr)
          return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                      "Error pushing token into filesystem.",
                                      r->pool);
        list = list->next;

      } while (list);
]]]

So, basically, we can't change this behaviour without patching mod_dav_svn.
I'm not really sure if there is a way to get right paths for lock tokens in
this place, but maybe we could improve this place in a separate patch.