[m-rev.] for review: Improve Mercury's temporary files and directories predicates.
Julien Fischer
jfischer at opturion.com
Wed Apr 13 13:43:39 AEST 2016
Hi Paul,
Why have you posted this diff to the list twice?
Julien.
On Wed, 13 Apr 2016, Paul Bone wrote:
> For review by anyone.
>
> I don't really know C# and have even less knowledge of Erlang. I've not
> implemented these for the Erlang backend and a reviewer may wish to
> scrutinze the C# code in particular.
>
> Thanks.
>
> ---
>
> Improve Mercury's temporary files and directories predicates.
>
> + Add support for retriving the system/user's temporary directory.
> + Add support for creating temporary directories.
>
> library/io.m:
> Add temp_directory/3
>
> Add make_temp_directory/3 and make_temp_directory/5.
>
> make_temp_file/3 and make_temp_file/5 now return the full pathname on
> the Java backends.
>
> On the Java backend make_temp_file/3 and make_temp_file/5 now create
> files chmod 600, which is necessary when the temporary files are
> security sensative.
>
> Document the limitations of these calls for the C# and Erlang backends.
>
> configure.ac:
> runtime/mercury_conf.h.in:
> Check for mkdtemp(3).
>
> NEWS:
> Announce the new predicates.
> ---
> NEWS | 3 +
> configure.ac | 4 +-
> library/io.m | 319 ++++++++++++++++++++++++++++++++++++----------
> runtime/mercury_conf.h.in | 2 +
> 4 files changed, 258 insertions(+), 70 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 71c8764..bae0d9a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -123,6 +123,9 @@ Changes to the Mercury standard library:
> * io.print and string_writer.print now print arbitrary precision integers
> in their decimal form instead of printing their underlying representation.
>
> +* We have added temp_directory/3, make_temp_directory/3 and
> + make_temp_directory/5 predicates to the io module.
> +
> * We have added a module for discrete interval encoding trees, which are a
> highly efficient set implementation for fat sets. This module is a
> contribution from Yes Logic Pty. Ltd.
> diff --git a/configure.ac b/configure.ac
> index d282008..037a6bd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1316,8 +1316,8 @@ mercury_check_for_functions \
> grantpt unlockpt ptsname tcgetattr tcsetattr ioctl \
> access sleep opendir readdir closedir mkdir symlink readlink \
> gettimeofday setenv putenv _putenv posix_spawn sched_setaffinity \
> - sched_getaffinity sched_getcpu sched_yield mkstemp setrlimit \
> - fma
> + sched_getaffinity sched_getcpu sched_yield mkstemp mkdtemp
> + setrlimit fma
>
> mercury_check_for_stdio_functions \
> snprintf _snprintf vsnprintf _vsnprintf
> diff --git a/library/io.m b/library/io.m
> index b685315..3195532 100644
> --- a/library/io.m
> +++ b/library/io.m
> @@ -1312,8 +1312,65 @@
> % It is the responsibility of the program to delete the file when it is
> % no longer needed.
> %
> - % The file will reside in an implementation-dependent directory.
> - % For current Mercury implementations, it is determined as follows:
> + % The file is placed in the directory returned by temp_directory/3.
> + %
> + % This is insecure on the Erlang backend, file permissions are not set
> + % properly.
> + %
> +:- pred make_temp(string::out, io::di, io::uo) is det.
> +
> + % make_temp(Dir, Prefix, Name, !IO) creates an empty file whose
> + % name is different to the name of any existing file. The file will reside
> + % in the directory specified by `Dir' and will have a prefix using up to
> + % the first 5 characters of `Prefix'. Name is bound to the name of the
> + % file. It is the responsibility of the program to delete the file
> + % when it is no longer needed.
> + %
> + % The C# backend has the following limitations:
> + % + Dir is ignored.
> + % + Prefix is ignored.
> + %
> + % This is insecure on the Erlang backend, file permissions are not set
> + % properly.
> + %
> +:- pred make_temp(string::in, string::in, string::out, io::di, io::uo)
> + is det.
> +
> + % make_temp_directory(DirName, !IO) creates an empty directory whose name
> + % is different from the name of any existing directory.
> + %
> + % On the C# backend this is insecure as the file permissions are not set
> + % and this call does not test for an existing directory.
> + %
> + % This is unimplemented on the Erlang backend.
> + %
> +:- pred make_temp_directory(string::out, io::di, io::uo) is det.
> +
> + % make_temp_directory(Dir, Prefix, DirName, !IO) creates an empty directory
> + % whose name is different from the name of any existing directory. The new
> + % directory will reside in the existing directory specified by `Dir' and
> + % will have a prefix using up to the first 5 characters of `Prefix'.
> + % DirName is bound to the name of the new directory. It is the
> + % responsibility of the program to delete the directory when it is no
> + % longer needed.
> + %
> + % The C# backend has the following limitations:
> + % + It is insecure as as the file permissions are not set and the call
> + % does not test for an existing directory.
> + % + Prefix is ignored.
> + %
> + % This is unimplemented on the Erlang backend.
> + %
> +:- pred make_temp_directory(string::in, string::in, string::out,
> + io::di, io::uo) is det.
> +
> + % temp_directory(DirName, !IO)
> + %
> + % DirName is the name of a directory where applications should put
> + % temporary files.
> + %
> + % This is implementation-dependent. For current Mercury implementations,
> + % it is determined as follows:
> % 1. For the non-Java back-ends:
> % - On Microsoft Windows systems, the file will reside in
> % the current directory if the TMP environment variable
> @@ -1327,17 +1384,7 @@
> % value of this property is typically "/tmp" or "/var/tmp";
> % on Microsoft Windows systems it is typically "c:\\temp".
> %
> -:- pred make_temp(string::out, io::di, io::uo) is det.
> -
> - % make_temp(Dir, Prefix, Name, !IO) creates an empty file whose
> - % name is different to the name of any existing file. The file will reside
> - % in the directory specified by `Dir' and will have a prefix using up to
> - % the first 5 characters of `Prefix'. Name is bound to the name of the
> - % file. It is the responsibility of the program to delete the file
> - % when it is no longer needed.
> - %
> -:- pred make_temp(string::in, string::in, string::out, io::di, io::uo)
> - is det.
> +:- pred temp_directory(string::out, io::di, io::uo) is det.
>
> % remove_file(FileName, Result, !IO) attempts to remove the file
> % `FileName', binding Result to ok/0 if it succeeds, or error/1 if it
> @@ -1807,6 +1854,10 @@
> #endif
> ").
>
> +:- pragma foreign_decl("C#", "
> +using System.IO;
> +").
> +
> :- pragma foreign_code("C#", "
> // The ML_ prefixes here are not really needed,
> // since the C# code all gets generated inside a class,
> @@ -3770,7 +3821,7 @@ check_directory_accessibility_dotnet(_, _, _, Res, !IO) :-
> // isn't read-only doesn't mean we have permission to write to it.
> // The only way to test whether a directory is writable is to
> // write a file to it. The ideal way to do that would be
> - // io.make_temp, but currently the .NET backend version of that
> + // io.make_temp, but currently the C# backend version of that
> // ignores the directory passed to it.
> System.IO.FileAttributes attrs =
> System.IO.File.GetAttributes(FileName);
> @@ -10310,25 +10361,12 @@ io.setenv(Var, Value) :-
>
> %---------------------------------------------------------------------------%
>
> - % We need to do an explicit check of TMPDIR because not all
> - % systems check TMPDIR for us (eg Linux #$%*@&).
> -io.make_temp(Name, !IO) :-
> - Var = ( if dir.use_windows_paths then "TMP" else "TMPDIR" ),
> - io.get_environment_var(Var, Result, !IO),
> - (
> - Result = yes(Dir)
> - ;
> - Result = no,
> - ( if dir.use_windows_paths then
> - Dir = dir.this_directory
> - else
> - Dir = "/tmp"
> - )
> - ),
> - io.make_temp(Dir, "mtmp", Name, !IO).
> +make_temp(Name, !IO) :-
> + temp_directory(Dir, !IO),
> + make_temp(Dir, "mtmp", Name, !IO).
>
> -io.make_temp(Dir, Prefix, Name, !IO) :-
> - io.do_make_temp(Dir, Prefix, char_to_string(dir.directory_separator),
> +make_temp(Dir, Prefix, Name, !IO) :-
> + do_make_temp(Dir, Prefix, char_to_string(dir.directory_separator),
> Name, Err, Message, !IO),
> ( if Err = 0 then
> true
> @@ -10336,7 +10374,20 @@ io.make_temp(Dir, Prefix, Name, !IO) :-
> throw_io_error(Message)
> ).
>
> -%---------------------------------------------------------------------------%
> +make_temp_directory(DirName, !IO) :-
> + temp_directory(Dir, !IO),
> + make_temp_directory(Dir, "mtmp", DirName, !IO).
> +
> +make_temp_directory(Dir, Prefix, DirName, !IO) :-
> + do_make_temp_directory(Dir, Prefix,
> + char_to_string(dir.directory_separator), DirName, Err, Message, !IO),
> + ( if Err = 0 then
> + true
> + else
> + throw_io_error(Message)
> + ).
> +
> +%-----------------------------------------------------------------------%
>
> :- pred io.do_make_temp(string::in, string::in, string::in,
> string::out, int::out, string::out, io::di, io::uo) is det.
> @@ -10467,55 +10518,42 @@ io.make_temp(Dir, Prefix, Name, !IO) :-
> }
> }").
>
> -% For the Java implementation, io.make_temp/3 is overwritten directly,
> -% since Java is capable of locating the default temp directory itself.
> -
> -:- pragma foreign_proc("Java",
> - io.do_make_temp(_Dir::in, _Prefix::in, _Sep::in, _FileName::out,
> - _Error::out, _ErrorMessage::out, _IO0::di, _IO::uo),
> - [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe],
> +:- pragma foreign_decl("Java", local,
> "
> - // this function should never be called.
> -
> - throw new RuntimeException(""io.do_make_temp not implemented"");
> +import java.io.File;
> +import java.nio.file.Files;
> +import java.nio.file.Path;
> +import java.nio.file.Paths;
> +import java.nio.file.attribute.PosixFilePermissions;
> ").
>
> -:- pragma foreign_proc("Java",
> - io.make_temp(FileName::out, _IO0::di, _IO::uo),
> - [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
> - may_not_duplicate],
> -"
> - try {
> - java.io.File tmpdir = new java.io.File(
> - java.lang.System.getProperty(""java.io.tmpdir""));
> - FileName = java.io.File.createTempFile(""mtmp"", null, tmpdir).
> - getName();
> - } catch (java.lang.Exception e) {
> - io.ML_throw_io_error(e.getMessage());
> - FileName = null;
> - }
> -").
> +
> +% For the Java implementation, io.make_temp/3 is overwritten directly,
> +% since Java is capable of locating the default temp directory itself.
>
> :- pragma foreign_proc("Java",
> - io.make_temp(Dir::in, Prefix::in, FileName::out, _IO0::di, _IO::uo),
> + io.do_make_temp(Dir::in, Prefix::in, _Sep::in, FileName::out,
> + Error::out, ErrorMessage::out, _IO0::di, _IO::uo),
> [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
> may_not_duplicate],
> "
> try {
> - if (Prefix.length() < 3) {
> - // File.createTempFile() requires a Prefix which is
> - // at least 3 characters long.
> - Prefix = Prefix + ""tmp"";
> - } else if (Prefix.length() > 5) {
> + Path dir_path, new_file;
> +
> + if (Prefix.length() > 5) {
> // The documentation for io.make_temp says that we should only use
> // the first five characters of Prefix.
> Prefix = Prefix.substring(0, 5);
> }
> - FileName = java.io.File.createTempFile(Prefix, null,
> - new java.io.File(Dir)).getName();
> + dir_path = Paths.get(Dir);
> + new_file = Files.createTempFile(dir_path, Prefix, null);
> + FileName = new_file.toAbsolutePath().toString();
> + Error = 0;
> + ErrorMessage = """";
> } catch (java.lang.Exception e) {
> - io.ML_throw_io_error(e.getMessage());
> - FileName = null;
> + FileName = """";
> + Error = -1;
> + ErrorMessage = e.toString();
> }
> ").
>
> @@ -10591,6 +10629,151 @@ io.make_temp(Dir, Prefix, Name, !IO) :-
> end.
> ").
>
> +%-----------------------------------------------------------------------%
> +
> +:- pred io.do_make_temp_directory(string::in, string::in, string::in,
> + string::out, int::out, string::out, io::di, io::uo) is det.
> +
> +:- pragma foreign_proc("C",
> + io.do_make_temp_directory(Dir::in, Prefix::in, Sep::in, DirName::out,
> + Error::out, ErrorMessage::out, _IO0::di, _IO::uo),
> + [will_not_call_mercury, promise_pure, tabled_for_io,
> + does_not_affect_liveness],
> +"
> +#ifdef MR_HAVE_MKDTEMP
> + int err;
> +
> + DirName = MR_make_string(MR_ALLOC_ID, ""%s%s%.5sXXXXXX"",
> + Dir, Sep, Prefix);
> + DirName = mkdtemp(DirName);
> + if (DirName == NULL) {
> + ML_maybe_make_err_msg(MR_TRUE, errno,
> + ""error creating temporary directory: "", MR_ALLOC_ID,
> + ErrorMessage);
> + Error = -1;
> + } else {
> + ErrorMessage = MR_make_string_const("""");
> + Error = 0;
> + }
> +#else
> + #warning ""Your system does not have mkdtemp""
> + Error = -1;
> + ErrorMessage =
> + MR_make_string_const(""Your system does not have mkdtemp"");
> + DirName = MR_make_string_const("""");
> +#endif /* HAVE_MKDTEMP */
> +").
> +
> +:- pragma foreign_proc("C#",
> + io.do_make_temp_directory(Dir::in, _Prefix::in, _Sep::in, DirName::out,
> + Error::out, ErrorMessage::out, _IO0::di, _IO::uo),
> + [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe],
> +"{
> + try {
> + DirName = Path.Combine(Dir, Path.GetRandomFileName());
> + /*
> + * This is not secure:
> + * 1. We cannot set permissions
> + * 2. We cannot atomically test for and create a directory
> + */
> + Directory.CreateDirectory(DirName);
> + Error = 0;
> + ErrorMessage = """";
> + }
> + catch (System.Exception e)
> + {
> + DirName = """";
> + Error = -1;
> + ErrorMessage = e.Message;
> + }
> +}").
> +
> +:- pragma foreign_proc("Java",
> + io.do_make_temp_directory(Dir::in, Prefix::in, _Sep::in, DirName::out,
> + Error::out, ErrorMessage::out, _IO0::di, _IO::uo),
> + [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
> + may_not_duplicate],
> +"
> + try {
> + Path dir_path, new_file;
> +
> + if (Prefix.length() > 5) {
> + // The documentation for io.make_temp says that we should only use
> + // the first five characters of Prefix.
> + Prefix = Prefix.substring(0, 5);
> + }
> + dir_path = Paths.get(Dir);
> + new_file = Files.createTempDirectory(dir_path, Prefix);
> + DirName = new_file.toAbsolutePath().toString();
> + Error = 0;
> + ErrorMessage = """";
> + } catch (java.lang.Exception e) {
> + DirName = """";
> + Error = -1;
> + ErrorMessage = e.toString();
> + }
> +").
> +
> +%---------------------------------------------------------------------------%
> +
> +temp_directory(Dir, !IO) :-
> + % If using a Java or C# backend then use their API to get the location of
> + % temporary files.
> + system_temp_dir(Dir0, OK, !IO),
> + ( if OK = 1 then
> + Dir = Dir0
> + else
> + % Either this is not a Java or C# grade or the Java or C# backend
> + % couldn't determine the temporary directory.
> + %
> + % We need to do an explicit check of TMPDIR because not all
> + % systems check TMPDIR for us (eg Linux #$%*@&).
> + Var = ( if dir.use_windows_paths then "TMP" else "TMPDIR" ),
> + get_environment_var(Var, Result, !IO),
> + (
> + Result = yes(Dir)
> + ;
> + Result = no,
> + ( if dir.use_windows_paths then
> + Dir = dir.this_directory
> + else
> + Dir = "/tmp"
> + )
> + )
> + ).
> +
> +:- pred system_temp_dir(string::out, int::out, io::di, io::uo) is det.
> +
> +:- pragma foreign_proc("Java",
> + system_temp_dir(Dir::out, OK::out, _IO0::di, _IO::uo),
> + [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
> + may_not_duplicate],
> +"
> + try {
> + Dir = java.lang.System.getProperty(""java.io.tmpdir"");
> + OK = (Dir != null) ? 1 : 0;
> + } catch (Exception e) {
> + Dir = null;
> + OK = 0;
> + }
> +").
> +
> +:- pragma foreign_proc("C#",
> + system_temp_dir(Dir::out, OK::out, _IO0::di, _IO::uo),
> + [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
> + may_not_duplicate],
> +"
> + try {
> + Dir = System.IO.Path.GetTempPath();
> + OK = (Dir != null) ? 1 : 0;
> + } catch (System.Exception _) {
> + Dir = null;
> + OK = 0;
> + }
> +").
> +
> +system_temp_dir("", 0, !IO).
> +
> %---------------------------------------------------------------------------%
>
> :- pragma foreign_decl("C", "
> diff --git a/runtime/mercury_conf.h.in b/runtime/mercury_conf.h.in
> index d631cdf..343b71d 100644
> --- a/runtime/mercury_conf.h.in
> +++ b/runtime/mercury_conf.h.in
> @@ -296,6 +296,7 @@
> ** we have the pthread_mutexattr_setpshared()
> ** function.
> ** MR_HAVE_MKSTEMP we have the mkstemp() function.
> +** MR_HAVE_MKDTEMP we have the mkdtemp() function.
> ** MR_HAVE_SETRLIMIT we have the setrlimit() function.
> ** MR_HAVE_ISNAN we have the isnan() function.
> ** MR_HAVE_ISNANF we have the isnanf() function.
> @@ -345,6 +346,7 @@
> #undef MR_HAVE_DUP2
> #undef MR_HAVE_FILENO
> #undef MR_HAVE_MKSTEMP
> +#undef MR_HAVE_MKDTEMP
> #undef MR_HAVE_ISATTY
> #undef MR_HAVE_GRANTPT
> #undef MR_HAVE_UNLOCKPT
> --
> 2.8.0.rc3
>
> _______________________________________________
> reviews mailing list
> reviews at lists.mercurylang.org
> https://lists.mercurylang.org/listinfo/reviews
More information about the reviews
mailing list