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

Serious bug in fsfs's implementation of svn_fs_apply_textdelta

From: Brian W. Fitzpatrick <fitz_at_collab.net>
Date: 2005-04-21 07:14:03 CEST

To reproduce this, call svn_fs_create_file(), svn_fs_apply_textdelta(),
svn_fs_change_node_prop(), then later on, pass NULL to the window
handler. When you pass NULL to the window handler, it causes the
property that you inserted with svn_fs_change_node_prop to vanish.

I found this while testing r14262 for merging into the 1.2.x branch.
It turns out that in mod_dav_svn (with autoversioning on) the code that
sets svn:mime-type from the request's content_type does not succeed
when using an fsfs backend.

I chatted with Greg Hudson in irc, and he said:

   "The obvious fix for fitz's bug is to stop caching the noderev in the
baton created by apply_textdelta.  It's a little sad that we'd lose the
efficiency gain of doing so, though, when the caller has no particular
motive to want to be able to tamper with the node during an
apply-textdelta sequence."

We agreed that that's not really necessary (to stop caching the
noderev) and that we'll be fine with doing the following:

1. Tweak the documentation for svn_fs_apply_textdelta to be clear that
you shouldn't interleave calls to svn_fs_apply_textdelta with another
change.

2. Call svn_fs_change_node_prop call (in
mod_dav_svn/repos.c:dav_svn_open_stream) before svn_fs_apply_textdelta.

Now doing #2 is going to require a small bit of work since
dav_svn_open_stream calls svn_fs_apply_textdelta() unconditionally,
then calls svn_fs_make_file if the apply fails and then calls
svn_fs_apply_textdelta again on the newly created file.

What we need to do here is call svn_fs_check_path, and if the path
doesn't exist, call svn_fs_make_file. Then we're ready to do our
svn_fs_change_node_prop mojo and move along to apply_textdelta.

One last improvement I'd like to see is to augment the conditional
around the call to svn_fs_change_node_prop and only add the mime type
if it doesn't already exist.

Filed as issue #2280.

-Fitz

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 21 07:15:02 2005

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