codeaudit / dkpro-core-asl

Automatically exported from code.google.com/p/dkpro-core-asl
0 stars 0 forks source link

Fails to run the binary, with "noexec" /tmp #107

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Run the TreeTagger wrapper in a linux system with /tmp mounted with 
"noexec". 
2. Tree tagger binary copied into the /tmp cannot be executed, and fails. 

A work-around: 
One can tell Java to use a different temporary directory, like followings. 

> java -Djava.io.tmpdir="somedir"   [the rest],
or in maven
> mvn -Djava.io.tmpdir="somedir" [build command] 

This makes Java to use a different directory, and it successfully runs. 

Possible Solution: 
A "fall back" code would be nice to end users; if the copy & call on /tmp 
fails, try to make a /tmp/ folder on current (eh, this might be ugly but...) 
PWD and use it and clean it ... 

Original issue reported on code.google.com by nohtae...@gmail.com on 7 Jan 2013 at 10:53

GoogleCodeExporter commented 9 years ago

Original comment by richard.eckart on 7 Jan 2013 at 4:21

GoogleCodeExporter commented 9 years ago
I think using PWD is a bad idea as that may be basically anything, but trying 
DKPRO_HOME and the user's home should work.

Original comment by richard.eckart on 7 Jan 2013 at 4:22

GoogleCodeExporter commented 9 years ago
At least on *nix systems, executable files should never be put in the system 
temp directory by default, nor should they be put in some arbitrary location in 
the user's home directory.  Instead we should follow the conventions of the XDG 
Base Directory Specification 
<http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html> – 
that is, check for the presence of an $XDG_RUNTIME_DIR environment variable, 
and if set, use that directory for the binary.  On systems implementing the XDG 
Base Directory Specification (which is pretty much every modern *nix), this 
directory is guaranteed to be readable, writable, and executable by the user 
(and only the user).  If this environment variable is missing or empty, only 
then should the application try falling back to the system temp directories.  
On POSIX/FHS systems, this is $TMPDIR (or failing that, /tmp).  Microsoft 
Windows probably has its own conventions but I don't know what those are.

Original comment by tristan.miller@nothingisreal.com on 9 Jan 2013 at 11:16

GoogleCodeExporter commented 9 years ago
An excellent idea. Thanks for pointing this out.

Original comment by richard.eckart on 9 Jan 2013 at 1:41

GoogleCodeExporter commented 9 years ago

Original comment by richard.eckart on 16 Feb 2013 at 11:01

GoogleCodeExporter commented 9 years ago

Original comment by pedrobss...@gmail.com on 26 Apr 2013 at 1:00

GoogleCodeExporter commented 9 years ago
/tmp directory is not used any more. Now it is checked if the
XDG_RUNTIME_DIR variable is set. If it is, this folder is used. Otherwise, it is
checked if the DKPRO_HOME environment is set. If it is, then it is used. If it
is not, then the USER.HOME folder is used.

Original comment by pedrobss...@gmail.com on 26 Apr 2013 at 3:49

GoogleCodeExporter commented 9 years ago
I think it's not quite fixed yet. E.g. there is no handling of DKPRO_HOME (or 
any of the other variables) being set but not having been created as a 
directory yet. The getClasspathAsFolder() isn't updated yet.

Original comment by richard.eckart on 26 Apr 2013 at 3:55

GoogleCodeExporter commented 9 years ago
I tested it without having the directory created, and it created on the fly. At 
least on ubuntu is not a problem. Do you know if it is problem in other OS? The 
getClasspathAsFolder() method was updated, thanks for noticing this.

Original comment by pedrobss...@gmail.com on 26 Apr 2013 at 5:05

GoogleCodeExporter commented 9 years ago
Since we do not use /tmp anymore at all now, we get test failures on the build 
server, because the home folder is actually not writable., e.g. in 
testGetUrlAsFile().

We also seem to have a problem that the temporary files created by getUrlAsFile 
can actually conflict with each other. E.g. if 
getUrlAsFile("http://lala/test.txt") and then 
getUrlAsFile("http://lolo/test.txt") is called, the temporary file created in 
the first call appears to be overwritten.

Original comment by richard.eckart on 26 Apr 2013 at 6:09

GoogleCodeExporter commented 9 years ago
The problem of conflicting file names was not there previously, because 
File.createTempFile(name, "."+suffix) creates unique names. The ability to 
create a unique name for each call of createTempFile() must be retained.

Original comment by richard.eckart on 26 Apr 2013 at 6:14

GoogleCodeExporter commented 9 years ago
I think your test did not what you think it did. I just ran a manual test 
pointing XDG_RUNTIME_DIR to a folder that does not exist and ran the 
testGetUrlAsFile(). The result was a FileNotFoundException because 
FileOutputStream could not write a file to a non-existing folder.

So we have several problems now:

- /tmp is not used anymore at all, even if it could be used
- createTempFile() does not create unique names like File.createTempFile() does
- the case that the temporary directory does not exist is not handled
- also the case that a temporary directory does exist but is not writable that 
it cannot be used for executable files is not handled

I'd suggest the following:

- revert getUrlAsFile() and getClasspathAsFolder() to the previous state and 
document that they must not be used for executable binaries
- create a new getUrlAsExecutable() method which makes sure that the temporary 
file is created in a location which permits executables
- getUrlAsExecutable() should try to create the executable file in the methods 
and order: 
-- File.createTempFile() then try to make temporary file executable
-- if this fails try creating the executable in XDG_RUNTIME_DIR under a unique 
name. Make sure no file which by accident may already have that name in that 
location is overwritten
-- if this fails, try DKPRO_HOME, but according to policy not DKPRO_HOME 
directly a subdirectory, I suggest DKPRO_HOME/temp. Again be careful about the 
unique name and about not overwriting
-- if this fails, try the users' home. Again not the home directly, but 
user.home/.dkpro/temp. (unique name/overwriting/yaddayadda)
-- if this also fails, produce an IOException with a useful error message 
explaining what was tried, what failed, why and what is suggested to fix the 
situation (e.g. make sure that user.home/dkpro/temp is writable and can be used 
for executables)

Making sure that the created file really has a unique name component may the 
the most difficult part. Apparrently the JDK uses a SecureRandom.nextLong() for 
this. Not fool-proof, but probably good enough for our purposes.

Original comment by richard.eckart on 26 Apr 2013 at 6:43

GoogleCodeExporter commented 9 years ago
It's probably also a good idea to use 
FileSystem.getFileSystem().createFileExcusively(file) to create the temporary 
file, like File.createTempFile() does. Best take a look at the sources of 
File.createTempFile() but don't just copy it ;)

Original comment by richard.eckart on 26 Apr 2013 at 6:49

GoogleCodeExporter commented 9 years ago
Apparrently FileSystem is an internal class of the JDK that we cannot use, so 
we'll probably have to resort to a simple loop which repeatedly tests if there 
already is a file with the unique name, try the next unique name if there is 
and tries to create the file if it's not there. Then we pray that the situation 
that two threads or processes try to create a file by the same name in almost 
the same moment is so rare that it is no problem.

Original comment by richard.eckart on 26 Apr 2013 at 7:09

GoogleCodeExporter commented 9 years ago

Original comment by pedrobss...@gmail.com on 3 May 2013 at 1:48

GoogleCodeExporter commented 9 years ago
I'm having trouble following what the problem is here.  You say there's a 
possibility of conflicting file names and in Comment #14 propose some sort of 
polling loop as a workaround.  What's wrong with just using 
File.createTempFile(String prefix, String suffix, File directory)?

Original comment by tristan.miller@nothingisreal.com on 6 May 2013 at 10:50

GoogleCodeExporter commented 9 years ago
Hum, inability to read the documentation on my side I guess ;) I didn't know of 
that signature.

Original comment by richard.eckart on 6 May 2013 at 10:52

GoogleCodeExporter commented 9 years ago
OK.  As a follow-up question, though, how do you intend to make the file 
executable?  DKPro uses Java 6, but support for changing POSIX file permissions 
first appears in Java 7.

Original comment by tristan.miller@nothingisreal.com on 6 May 2013 at 2:26

GoogleCodeExporter commented 9 years ago
If Java 7, then we can use setExecutable(). Otherwise, there are some 
provisions in the PlatformDetector class to help, in particular getChmodCmd(). 
I don't know if this is used anywhere right now.

Original comment by richard.eckart on 6 May 2013 at 3:38

GoogleCodeExporter commented 9 years ago
Actually, Java 6 already supports File.setExecutable(true). The provisions in 
PlatformDetector had been used when the stuff was still supporting Java 5.

Original comment by richard.eckart on 6 May 2013 at 3:43

GoogleCodeExporter commented 9 years ago

Original comment by pedrobss...@gmail.com on 8 May 2013 at 8:20