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 ______________________________________________________________________
