Issue 6138 - Using dirEntries and chdir() can have unwanted results
Summary: Using dirEntries and chdir() can have unwanted results
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: Other All
: P2 major
Assignee: No Owner
URL:
Keywords: bootcamp
Depends on:
Blocks:
 
Reported: 2011-06-09 16:13 UTC by Andrej Mitrovic
Modified: 2024-12-01 16:14 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Andrej Mitrovic 2011-06-09 16:13:56 UTC
I can't officially classify this as a bug. Jonathan said it might be a good idea to put it here just so it doesn't get lost, and then we can later decide if it's a bug or if we should just be careful what we do. 

Here's some buggy code which might throw an exception (if you're lucky!):
   foreach (string entry; dirEntries(curdir, SpanMode.shallow))
   {
       if (entry.isdir)
       {
           foreach (string subentry; dirEntries(entry, SpanMode.shallow))
           {
               if (subentry.isdir)
               {
                   chdir(rel2abs(subentry));
               }
           }
       }
   }

This might throw something like this:
std.file.FileException@std\file.d(1124): .\foo\bar.d: The system cannot find the path specified.

The problem is chdir will modify curdir, and dirEntries doesn't store 'curdir' internally as an absolute address, so calling chdir() screws up the behavior of dirEntries.

A workaround is to call rel2abs as soon as possible when using 'curdir' (the first line changed here):

   foreach (string entry; dirEntries(rel2abs(curdir), SpanMode.shallow))
   {
       if (entry.isdir)
       {
           foreach (string subentry; dirEntries(entry, SpanMode.shallow))
           {
               if (subentry.isdir)
               {
                   chdir(rel2abs(subentry));
               }
           }
       }
   }

I've had a use for code like this where I was invoking a script file which was always relative to some subdirectory.
Comment 1 Timothee Cour 2016-03-23 05:33:36 UTC
sounds like a bug (and one that could cause harm); 
why not have dirEntries call rel2abs ? the cost of this should be dwarfed by system calls involved in dirEntries
Comment 2 RazvanN 2017-07-07 12:53:53 UTC
Is this still valid? The code seems to have changes significantly (rel2abs doesn't exist anymore) and I cannot reproduce with std.file.absolutePath.
Comment 3 RazvanN 2017-07-07 13:00:50 UTC
After analyzing the code I see that dirEntries does not call absolutePath on the path, which in my opinion is a bug. Even though I cannot reproduce the bug, I think a call to absolutePath should definitely be added
Comment 4 Vladimir Panteleev 2017-07-07 14:52:56 UTC
(In reply to RazvanN from comment #3)
> After analyzing the code I see that dirEntries does not call absolutePath on
> the path, which in my opinion is a bug. Even though I cannot reproduce the
> bug, I think a call to absolutePath should definitely be added

That might break things.

~ $ mkdir -p a/b/c
~ $ cd a/b/c
~/a/b/c $ touch a
~/a/b/c $ ls
a
~/a/b/c $ ls ~/a/b/c
a
~/a/b/c $ chmod 000 ~/a/b
~/a/b/c $ ls
a
~/a/b/c $ ls ~/a/b/c
ls: cannot access '/home/vladimir/a/b/c': Permission denied
~/a/b/c $ 

I think the correct solution would be to use fdopendir and openat (on POSIX).
Comment 5 Francesco Galla' 2018-01-29 13:14:54 UTC
(In reply to RazvanN from comment #3)
> After analyzing the code I see that dirEntries does not call absolutePath on
> the path, which in my opinion is a bug. Even though I cannot reproduce the
> bug, I think a call to absolutePath should definitely be added

I reproduced the bug with the following code:

   foreach (string entry; dirEntries((dir), SpanMode.shallow))
   {
       if (entry.isDir)
       {
	    foreach (string subentry; dirEntries(entry, SpanMode.shallow))
            {
                 if (subentry.isDir)
		 {
                     chdir(absolutePath(subentry));
		     writeln (absolutePath(subentry));
		 }
	    }
       }
   }

My directory tree is:

.
├── 1
│   └── 2
├── 3
│   ├── 4
│   └── 5
│       └── 6


* The result I obtained was the following:

std.file.FileException@/opt/dmd-2.075/import/std/file.d(1631): ./3: No such file or directory

* By calling:

foreach (string entry; dirEntries(absolutePath(dir), SpanMode.shallow))

The code was executed correctly. This seems to confirm the need for dirEntries() to call absolutePath(). Am I mistaken?
Comment 6 Seb 2018-02-04 19:43:16 UTC
FYI: Francesco Galla submitted a PR - https://github.com/dlang/phobos/pull/6125
Comment 7 Timothee Cour 2018-02-10 03:25:20 UTC
just hit that bug again, but differently:
```
chdir(foo);
auto files=dirEntries(dirSrc, "*.d", SpanMode.depth).filter!(a=>a.isFile).map!(a=>dir.buildPath(a.name)).array;
```
crashed similarly
Comment 8 Timothee Cour 2018-03-16 22:16:18 UTC
ping: this is a serious bug
note: i was OSX and the bug was reported on windows originally, so setting to All
Comment 9 dlangBugzillaToGithub 2024-12-01 16:14:15 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/phobos/issues/9584

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB