Thread overview
[Issue 5243] New: dmd -run potentially removes user files
Nov 20, 2010
Leandro Lucarella
Dec 18, 2012
Andrej Mitrovic
Jan 07, 2013
Leandro Lucarella
Jan 07, 2013
Andrej Mitrovic
Jan 07, 2013
Andrej Mitrovic
Jan 08, 2013
Leandro Lucarella
Jan 08, 2013
Andrej Mitrovic
Jan 08, 2013
Iain Buclaw
Jan 08, 2013
Leandro Lucarella
November 20, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5243

           Summary: dmd -run potentially removes user files
           Product: D
           Version: D1 & D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: critical
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: llucax@gmail.com


--- Comment #0 from Leandro Lucarella <llucax@gmail.com> 2010-11-19 16:09:43 PST ---
See this example:

$ mkdir x
$ echo 'void main() {}' > x/test.d
$ echo "my very important data that shouldn't be ereased" > test
$ ls test
test
$ cat test
my very important data that shouldn't be ereased
$ dmd -run x/test.d
$ ls test
ls: cannot access test: No such file or directory
$ cat test
cat: test: No such file or directory

I think this is a very serious bug. It's really unexpected that DMD removes the test file (I can understand why it happens, but it shouldn't). test.d being in another directory is just to point how much surprising could be that running a "script" in an unrelated directory removes files in the current directory.

If DMD wants to put D in the scripting world, this should be fixed ASAP, as no scripting language EVER will remove your files unexpectedly.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 18, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-12-18 12:35:00 PST ---
Created an attachment (id=1170)
patch

Possible patch added. I don't have a Posix build system to test it, and there should be a test-script (possibly in shell) to verify it works ok.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 07, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #2 from Leandro Lucarella <leandro.lucarella@sociomantic.com> 2013-01-07 10:46:33 PST ---
Can you(In reply to comment #1)
> Created an attachment (id=1170) [details]
> patch
> 
> Possible patch added. I don't have a Posix build system to test it, and there should be a test-script (possibly in shell) to verify it works ok.

Can you convert this into a pull request? It will make reviewing more easy.

I can write the test case based on the one in the report.

All your changes are POSIX only, is the overwriting not happening on Windows?

Also, is extremely unlikely, but you can still overwrite unintended files, a check to avoid overwriting anything should be done, but maybe that's much harder to do because you have to change how the file is opened, not just the filename.

Finally, any reason for declaring getuid() manually instead of including the
appropriate headers?

Thanks for addressing this issue!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 07, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #3 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-01-07 12:31:42 PST ---
(In reply to comment #2)
> Can you convert this into a pull request? It will make reviewing more easy.

I'll do it later tonight.

> All your changes are POSIX only, is the overwriting not happening on Windows?

No because executables on win32 always end with .exe. Even if you do "dmd -oftest", DMD will still append an .exe .

> Finally, any reason for declaring getuid() manually instead of including the
> appropriate headers?

The DMC header is unistd.h, but the front-end is also used by GDC and LDC, I don't know which headers they use for this function. It seemed easiest to just declare the function rather than to include an entire header (remember that includes are costly for compile-times in C++).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 07, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #4 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-01-07 13:14:03 PST ---
Here, have fun with it, I don't have the means to test this on posix: https://github.com/D-Programming-Language/dmd/pull/1437

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 08, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #5 from Leandro Lucarella <leandro.lucarella@sociomantic.com> 2013-01-08 07:31:29 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> > Can you convert this into a pull request? It will make reviewing more easy.
> 
> I'll do it later tonight.

Thanks!

> > All your changes are POSIX only, is the overwriting not happening on Windows?
> 
> No because executables on win32 always end with .exe. Even if you do "dmd -oftest", DMD will still append an .exe .

But you can accidentally overwrite another .exe file, maybe something you compiled before and now you want to do some temporary test without overwriting that file? I know it looks unlikely, but I think -run should never overwrite any files.

Anyway, your patch is certainly a progress :)

> > Finally, any reason for declaring getuid() manually instead of including the
> > appropriate headers?
> 
> The DMC header is unistd.h, but the front-end is also used by GDC and LDC, I don't know which headers they use for this function. It seemed easiest to just declare the function rather than to include an entire header (remember that includes are costly for compile-times in C++).

POSIX says it is in unistd.h, so I guess we can trust different platforms to provide it in that header :)

http://pubs.opengroup.org/onlinepubs/009695299/functions/getuid.html

I will add these comments to the pull request too, just for convenience.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 08, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-01-08 08:34:15 PST ---
(In reply to comment #5)
>> But you can accidentally overwrite another .exe file

I don't know of any compilers which warn you against this (it should probably be part of a build tool), I thought the enhancement was for when you can accidentally overwrite files which were not generated by the compiler (data files with no extension on posix, for example).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 08, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243


Iain Buclaw <ibuclaw@ubuntu.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ibuclaw@ubuntu.com


--- Comment #7 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-01-08 08:50:44 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> > Can you convert this into a pull request? It will make reviewing more easy.
> 
> I'll do it later tonight.
> 
> > All your changes are POSIX only, is the overwriting not happening on Windows?
> 
> No because executables on win32 always end with .exe. Even if you do "dmd -oftest", DMD will still append an .exe .
> 
> > Finally, any reason for declaring getuid() manually instead of including the
> > appropriate headers?
> 
> The DMC header is unistd.h, but the front-end is also used by GDC and LDC, I don't know which headers they use for this function. It seemed easiest to just declare the function rather than to include an entire header (remember that includes are costly for compile-times in C++).

GDC ifdef's pretty much all of mars.c, so this doesn't affect me.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 08, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=5243



--- Comment #8 from Leandro Lucarella <leandro.lucarella@sociomantic.com> 2013-01-08 09:07:05 PST ---
(In reply to comment #6)
> (In reply to comment #5)
> >> But you can accidentally overwrite another .exe file
> 
> I don't know of any compilers which warn you against this (it should probably be part of a build tool), I thought the enhancement was for when you can accidentally overwrite files which were not generated by the compiler (data files with no extension on posix, for example).

And I'm not suggesting for DMD to check that when compiling normally. The thing is -run, to the user, is perceived as not generating any files, so it's really unexpected if any file gets removed after running dmd -run somescript.d.

Specially if dmd want to be taken seriously in the scripting world.

But maybe Alex Rønne Petersen is right, dmd -run is an incomplete feature and maybe it should be removed altogether because it might be too hard to do it right and there is already rdmd.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------