Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-2280

Serious bug in fsfs's implementation of svn_fs_apply_textdelta

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Blocker
    • Resolution: Fixed
    • all
    • unscheduled
    • libsvn_fs_fs
    • None

    Description

      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.
      

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            fitz Brian Fitzpatrick
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment