You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jon Foster <Jo...@cabot.co.uk> on 2009/12/03 14:19:41 UTC

Problems with mod_dav_svn and pre-revprop-change hook

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 
______________________________________________________________________

Re: Problems with mod_dav_svn and pre-revprop-change hook

Posted by Philip Martin <ph...@wandisco.com>.
"Jon Foster" <Jo...@cabot.co.uk> writes:

> 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.

> 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).

I can confirm that it happens with Subversion trunk.

-- 
Philip

Re: Problems with mod_dav_svn and pre-revprop-change hook

Posted by "C. Michael Pilato" <cm...@collab.net>.
Rawk!  Thanks, Jon.

Jon Foster wrote:
> Hi,
> 
> Done.  I split the report in two, as there are two separate issues.
> 
> Issue 3542
> pre-revprop-change hook executed twice by mod_dav_svn
> http://subversion.tigris.org/issues/show_bug.cgi?id=3542
> 
> Issue 3543
> mod_dav_svn: Error messages from pre-revprop-change are discarded
> http://subversion.tigris.org/issues/show_bug.cgi?id=3543 
> 
> Kind regards,
> 
> Jon
> 
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net] 
> Sent: 03 December 2009 15:32
> To: Jon Foster
> Cc: dev@subversion.apache.org
> Subject: Re: Problems with mod_dav_svn and pre-revprop-change hook
> 
> Jon Foster wrote:
>> [Resending as the company e-mail system is still stripping
>> non-ZIP attachments]
>>
>> Hi,
>>
>> I think there are 2 separate bugs in mod_dav_svn's pre-revprop-change
>> handling.  (Mostly due to problems with mod_dav).
> 
> Jon, would you be so kind as to file in issue in our tracker for this?
> Thanks!
> 


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: Problems with mod_dav_svn and pre-revprop-change hook

Posted by Jon Foster <Jo...@cabot.co.uk>.
Hi,

Done.  I split the report in two, as there are two separate issues.

Issue 3542
pre-revprop-change hook executed twice by mod_dav_svn
http://subversion.tigris.org/issues/show_bug.cgi?id=3542

Issue 3543
mod_dav_svn: Error messages from pre-revprop-change are discarded
http://subversion.tigris.org/issues/show_bug.cgi?id=3543 

Kind regards,

Jon

-----Original Message-----
From: C. Michael Pilato [mailto:cmpilato@collab.net] 
Sent: 03 December 2009 15:32
To: Jon Foster
Cc: dev@subversion.apache.org
Subject: Re: Problems with mod_dav_svn and pre-revprop-change hook

Jon Foster wrote:
> [Resending as the company e-mail system is still stripping
> non-ZIP attachments]
> 
> Hi,
> 
> I think there are 2 separate bugs in mod_dav_svn's pre-revprop-change
> handling.  (Mostly due to problems with mod_dav).

Jon, would you be so kind as to file in issue in our tracker for this?
Thanks!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand



**********************************************************************
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 
______________________________________________________________________

Re: Problems with mod_dav_svn and pre-revprop-change hook

Posted by "C. Michael Pilato" <cm...@collab.net>.
Jon Foster wrote:
> [Resending as the company e-mail system is still stripping
> non-ZIP attachments]
> 
> Hi,
> 
> I think there are 2 separate bugs in mod_dav_svn's pre-revprop-change
> handling.  (Mostly due to problems with mod_dav).

Jon, would you be so kind as to file in issue in our tracker for this?  Thanks!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: Problems with mod_dav_svn and pre-revprop-change hook

Posted by Jon Foster <Jo...@cabot.co.uk>.
[Resending as the company e-mail system is still stripping
non-ZIP attachments]

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 
______________________________________________________________________