[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Problems with mod_dav_svn and pre-revprop-change hook

From: Jon Foster <Jon.Foster_at_cabot.co.uk>
Date: Thu, 3 Dec 2009 14:19:41 -0000

Hi,

I think there are 2 separate bugs in mod_dav_svn's pre-revprop-change
handling. (Mostly due to problems with mod_dav).

I am using Subversion 1.6.6 and Debian's Apache httpd 2.2.9, but I
can't see any relevant changes in Apache httpd's SVN Trunk.

Reproduction recipes are attached - demo1.sh demonstratates both
bugs, and demo2.sh demonstrates another variant of bug 2.

---------- BUG 1 ----------

If the hook script blocks the change, then mod_dav_svn tries to set
the property back to it's original value, and it calls the hook
script again for this (with the old property value).

Expected behaviour: If the pre-revprop-change script blocks the
change, then mod_dav_svn shouldn't even try to revert it as the
revert is a no-op. (For DAV transactions with multiple prop
changes, if one of them fails then it shouldn't be asking the
pre-revprop-change hook scripts for permission to do the reverts,
it should just do the revert anyway and call the post-revprop-change
script).

Causes:
- subversion/mod_dav_svn/deadprops.c:db_apply_rollback() passes
  TRUE as the use_pre_revprop_change_hook parameter to
  svn_repos_fs_change_rev_prop3(). It should be FALSE in the
  revert case.
- httpd/modules/dav/main/props.c:dav_prop_exec() doesn't remove the
  rollback item if the property change fails. Therefore we "revert"
  changes that never happened.

---------- BUG 2 ----------

Any informative error messages from the first invocation of the
pre-revprop-change script are being lost. If the second invocation
of the hook script returns an error message, then this will be
returned to the user. If not, then the generic error message
"Could not execute PROPPATCH." is returned.

(I have Wireshark packet traces that show that the problem is at
the server end. The client just prints out the message the server
sends).

Expected behaviour: If the pre-revprop-change script fails with a
message, then that message should be shown to the user.

Causes:
- httpd/modules/dav/main/props.c:dav_prop_exec() wraps the useful
  error message in a useless "Could not execute PROPPATCH." message.
- httpd/modules/dav/main/props.c:dav_prop_rollback() takes any error
  messages from the rollback and uses them in preference to the
  (much more useful) original error. (The original error is
  preserved as an inner error, but...)
- httpd/modules/dav/main/mod_dav.c:dav_failed_proppatch() only
  reports the outer error, not any inner errors.

-------------

With careful hook script writing I can workaround these bugs, but
it would be good to get them fixed.

Kind regards,

Jon

**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.

If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.

Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232

Co. Registered in England number 02817269

Please contact the sender if you believe you have received this email in error.

**********************************************************************

______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Received on 2009-12-03 15:20:14 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.