Fossil

View Ticket
Login
Ticket Hash: 99caf06e17bed849146e834a8341d40590c29bc5
Title: Error moving a file to a directory with ../..
Status: Fixed Type: Code_Defect
Severity: Important Priority:
Subsystem: Resolution: Fixed
Last Modified: 2011-01-12 16:04:34
Version Found In: [cf178577ec]
Description:
I had pwd in "tools/path", "../path2" existed, and I moved a file this way:
<verbatim>
fossil mv myfile ../path2
</verbatim>

Instead of renaming "tools/path/myfile" to "tools/path2/myfile", it renamed "tools/path/myfile" to "tools/path2", and the "fossil status" started showing "NOT A FILE".

I had to "fossil checkout" to fix that working directory, and specify the full rename path the next time, including the file name.

<hr /><i>viric added on 2010-11-27 23:41:03:</i><br />
Sorry, I think I wrote quite an uncomplete report before. You will understand from this example, where file_isdir() fails, and will make mv_cmd() think it's a file rename instead of moving to a directory.

The repository root is at 'repo':
<verbatim>
viric@bergamota:~/dir1/dir2/repo/tools/dir3$ LANG=C stat ../../include
  File: `../../include'
  Size: 200             Blocks: 0          IO Block: 4096   directory
Device: fe00h/65024d    Inode: 32905       Links: 7
Access: (0755/drwxr-xr-x)  Uid: ( 1000/   viric)   Gid: (  444/   viric)
Access: 2010-11-28 00:34:28.000000000 +0100
Modify: 2010-09-28 19:31:46.000000000 +0200
Change: 2010-09-28 19:31:46.000000000 +0200
viric@bergamota:~/dir1/dir2/repo/tools/dir3$ fossil test-canonical-name ../../include
/home/viric/dir1/dir2/repo/include
  file_size   = 200
  file_mtime  = 1285695106
  file_isfile = 0
  file_isexe  = 0
  file_isdir  = 0
</verbatim>

<hr /><i>anonymous claiming to be viric added on 2010-11-27 23:42:35:</i><br />
Sorry, I think I wrote quite an uncomplete report before. You will understand from this example, where file_isdir() fails, and will make mv_cmd() think it's a file rename instead of moving to a directory.

The repository root is at 'repo':
<verbatim>
viric@bergamota:~/dir1/dir2/repo/tools/dir3$ LANG=C stat ../../include
  File: `../../include'
  Size: 200             Blocks: 0          IO Block: 4096   directory
Device: fe00h/65024d    Inode: 32905       Links: 7
Access: (0755/drwxr-xr-x)  Uid: ( 1000/   viric)   Gid: (  444/   viric)
Access: 2010-11-28 00:34:28.000000000 +0100
Modify: 2010-09-28 19:31:46.000000000 +0200
Change: 2010-09-28 19:31:46.000000000 +0200
viric@bergamota:~/dir1/dir2/repo/tools/dir3$ fossil test-canonical-name ../../include
/home/viric/dir1/dir2/repo/include
  file_size   = 200
  file_mtime  = 1285695106
  file_isfile = 0
  file_isexe  = 0
  file_isdir  = 0
</verbatim>

<hr /><i>anonymous added on 2010-11-27 23:44:23:</i><br />
Just to clarify, there are no symlinks around. Only simple directories, so the canonical name is correct.
The same 'test-canonical-name' works perfect for "../dir4", where dir4 exists. But it fails for "../../include".

<hr /><i>anonymous claiming to be page added on 2010-12-27 14:36:41:</i><br />
Any updates on this front? I just stumbled upon the same problem, where fossil mv file ../../dirname moved file to dirname instead of dirname/file.

<hr /><i>anonymous claiming to be viric added on 2011-01-06 21:34:19 UTC:</i><br />
I investigated the problem further.

file_isdir() was made to use file_simplify_name() because of some bug in mingw [a7822bcc001e9]. Nevertheless, file_isdir() is called sometimes with a canonical filename, sometimes with a filename directly from program arguments.

The function file_simplify_name() works well only for canonical names, because it is meant to remove the ".." in absolute paths. So when file_isdir() is called without a canonical name with the sequence '../..' in it, it works bad.

As I don't know the mingw bug, I imagine that the file_simplify_name() call has to be there. So, I propose what comes to mind:
  *  Make file_simplify_name() work well when it's given a non-canonical name
  *  Make file_isdir() not call file_simplify_name() if the name is not canonical.

Any preference? As drh did the change [a7822bcc001e9], he may know the best option.

<hr /><i>anonymous claiming to be viric added on 2011-01-11 13:10:58 UTC:</i><br />
I attached a patch that always uses file_canonical_name() so file_isdir() works for cases of "../.." directories.

As it works with canonical names, maybe it has troubles with the ".." directories inside a symlinked directory, but I imagine there may be troubles for this situation in other parts of the code too.

Attachments: