Created attachment 2190 [details]
smb.conf configuration file

Created attachment 2191 [details]
Vista RC2 log file

This is the log level 10 log file for the delete folder attempt

http://validator.w3.org/check?uri=www.samba.org

doesn't really like it either..

(In reply to comment #3)
> http://validator.w3.org/check?uri=www.samba.org
> 
> doesn't really like it either..
> 

(In reply to comment #2)
> Created an attachment (id=2191) [edit]
> Vista RC2 log file
> 
> This is the log level 10 log file for the delete folder attempt
> 

I'm experiencing the same problem with samba 3.0.23d, but it only 
fails when the directory is not empty.

Setting "delete veto files" to yes and applying the following patch seems to 
fix the problem.


--- ../.build_orig/samba/source/smbd/reply.c    2007-01-03 13:21:54.000000000 -0500
+++ samba/source/smbd/reply.c   2007-01-03 13:25:26.000000000 -0500
@@ -3950,7 +3950,8 @@ BOOL rmdir_internals(connection_struct *

                if(dir_hnd != NULL) {
                        long dirpos = 0;
-                       while ((dname = ReadDirName(dir_hnd,&dirpos))) {
+                       if (!lp_recursive_veto_delete(SNUM(conn))) {
+                          while ((dname = ReadDirName(dir_hnd,&dirpos))) {
                                if((strcmp(dname, ".") == 0) || (strcmp(dname, "..")==0))
                                        continue;
                                if (!is_visible_file(conn, directory, dname, &st, False))
@@ -3959,6 +3960,7 @@ BOOL rmdir_internals(connection_struct *
                                        all_veto_files = False;
                                        break;
                                }
+                          }
                        }

In my test case I found that Vista is setting 'close on delete' on the directory.  When the directory handle is closed reply_close() calls rmdir_internals() which fails because the directory is not empty.  The solution presented above has a problem, if I set 'delete veto files=yes' then I can go into a DOS box and do 'rmdir some_dir', the directory and all of its contents are deleted instead of correctly giving me a 'directory not empty' error.

It seems that the Vista client is expecting an implicit recursive delete when the directory is closed.  Would it be correct to change reply_close() to do a recursive delete (rather than calling rmdir_internals()) in this case ?

Pardon, I meant 'delete on close', not 'close on delete' :)

I'd like to reproduce this as the patch doesn't look correct to me.
When you say "attempt to delete a folder" I need more info than that.

What *exactly* is in the folder ? Is this just a new folder you've created from scratch ?

Jeremy.

Thanks Jeremy for looking into this issue.

On a Vista workstation, I created a new folder (myfolder), then I created a subfolder under that (mysubfolder). Then I tried to delete the myfolder. Windows show that it is delete, but after refreshing the screen, myfolder comes back. And it is not actually deleted on Linux side too.

Note that this only happens when Samba and Windows are configured as WORKGROUP. When Samba is configured as Domain Controller or Domain Member modes, there is no such problem. I will attach my smb.conf for your reference.

Created attachment 2250 [details]
My smb.conf

By the way, I agree that my patch is hacky, but that was the quickest way for me to workaround it for now.

From my testing I have that I can easily reproduce this if the folder was configured to be a remote offline sync folder.  The sync offline folders option seems to be different in Vista than previous versions of windows and previous versions of windows do not exhibit this behaviour.  So it seems that samba might not handle these offline folder operations correctly.  

I don't see this problem anymore after upgrading to Windows Vista Business Release.

I have found this issue to be very intermittent in nature.  It was quite reproducible for me for some time, but now I can't reproduce it.  I am still running release candidate 2, the same version of Samba that I was when I reported this, but don’t see the problem at all.  IMO, we’ll need to wait and see what happens once Vista starts being used by the masses to really know if this is a bug or just a Vista hiccup.

Greg

(In reply to comment #14)
> I have found this issue to be very intermittent in nature.  It was quite
> reproducible for me for some time, but now I can't reproduce it.  I am still
> running release candidate 2, the same version of Samba that I was when I
> reported this, but don’t see the problem at all.  IMO, we’ll need to wait
> and see what happens once Vista starts being used by the masses to really know
> if this is a bug or just a Vista hiccup.
> 
> Greg
> 

My testing, as of late, is against the production version (MSDN) of the Ultimate Version and I have not seen this unless I am configured as Sync Offline Folder.  

Jeremy, should I open a new bug that is explicit that samba properly does not support the Sync Offline Folder option, or utilize this current bug?

This problem is consistently reproducible, as Lon discovered, by setting a folder to be 'available offline'.  Even if I have two folders in the same directory, folder A and folder B, and set folder A to 'offline' the problem shows up when I try to delete folder B (all done using Windows Explorer from a Vista client).  I have a possible solution that so far looks promising, described below along with my understanding of the problem.

Failure mode:
When this problem occurs it is because the Vista client sets delete_on_close on the root of the tree to be deleted.  If the directory is *not empty* a real windows client will return an error NT_STATUS_DIRECTORY_NOT_EMPTY but Samba simply sets the delete_on_close flag and returns NT_STATUS_OK which apparently leaves the client thinking that it is done once it closes the directory.  When the directory is closed Samba detects that it is not empty and doesn't delete the directory.

My patch forces an additional check in can_set_delete_on_close to verify that the directory is empty before setting the delete_on_close flag.  If the directory is found to be not empty then NT_STATUS_DIRECTORY_NOT_EMPTY is returned.  With this change in place I am cannot reproduce the problem (was consistent before) and the network capture shows NT_STATUS_DIRECTORY_NOT_EMPTY being returned after which the Vista client proceeds to do the delete recursively.

The patch (attached, against release 3.0.23d):
The part of the code that checks for an empty directory is essentially lifted from the beginning of rmdir_internal() into a function called deleteDirOk() which is called from can_set_delete_on_close().  

There is a build problem when using this patch as-is, swat will not link b/c it includes locking/locking.c (with part of my change) but not smbd/dir.c which has the functions ReadDirName, OpenDir and CloseDir.  I think this can be easily worked out if this proof of concept fix turns out to be viable.  Although swat fails to link smbd does link and I just copied the smbd binary into place for testing.

Created attachment 2263 [details]
Prevents setting delete_on_close on non-empty directory

Jeremy, I'm assigning this one to you.

I think this is already fixed in the SAMBA_3_0_25 and SAMBA_3_0 svn trees as I've added the following code into can_set_delete_on_close(). I'll look into porting this to 3.0.24.

Jeremy.

        /* Don't allow delete on close for non-empty directories. */
        if (fsp->is_directory) {
                long offset = 0;
                NTSTATUS status;
                SMB_STRUCT_STAT st;
                struct dptr_struct *dirptr;
                const char *name;

                status = dptr_create(fsp->conn,
                                        fsp->fsp_name,
                                        False,
                                        True,
                                        0,
                                        "*",
                                        True,
                                        0,
                                        &dirptr);
                if (!NT_STATUS_IS_OK(status)) {
                        return status;
                }

                /* Read 3 entries. Ignore first 2 (they're . and .. ) */
                name = dptr_ReadDirName(dirptr, &offset, &st);
                if (!name) {
                        dptr_CloseDir(dirptr);
                        return NT_STATUS_ACCESS_DENIED;
                }
                name = dptr_ReadDirName(dirptr, &offset, &st);
                if (!name) {
                        dptr_CloseDir(dirptr);
                        return NT_STATUS_ACCESS_DENIED;
                }
                name = dptr_ReadDirName(dirptr, &offset, &st);
                dptr_CloseDir(dirptr);
                if (name) {
                        DEBUG(10,("can_set_delete_on_close: got name %s - can't delete\n", name ));
                        return NT_STATUS_DIRECTORY_NOT_EMPTY;
                }
        }

Created attachment 2280 [details]
Patch for 3.0.24.

Here's the patch for 3.0.24. Thanks for the work Joe, it was nicer than mine :-).

Closed as fixed.

actually closing.