[m-rev.] diff: Don't allow directories to be opened as files in C grades.
Paul Bone
paul at bone.id.au
Tue Sep 6 14:14:26 AEST 2016
Don't allow directories to be opened as files in C grades.
Some OSs allow you fopen a directory, which could cause confusion. This
doesn't make sense for Mercury file streams so test for and raise an error
when this happens.
library/io.m:
fstat files as we open them to determine if they are directories, if
they are close them and return an error.
tests/general/read_dir_regression.m:
tests/general/read_dir_regression.exp:
tests/general/read_dir_regression.exp2:
tests/general/read_dir_regression.exp3:
tests/general/read_dir_regression.exp4:
Update test case, differentiate between errors opening the file, and
reading from the file.
---
library/io.m | 107 +++++++++++++++++++++++----------
tests/general/read_dir_regression.exp | 2 +-
tests/general/read_dir_regression.exp2 | 2 +-
tests/general/read_dir_regression.exp3 | 2 +-
tests/general/read_dir_regression.exp4 | 2 +-
tests/general/read_dir_regression.m | 4 +-
6 files changed, 82 insertions(+), 37 deletions(-)
diff --git a/library/io.m b/library/io.m
index 9e662c7..c0c562c 100644
--- a/library/io.m
+++ b/library/io.m
@@ -5088,68 +5088,80 @@ read_binary(Result, !IO) :-
open_input(FileName, Result, !IO) :-
do_open_text(FileName, "r", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't open input file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(input_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, input, text, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't open input file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
open_output(FileName, Result, !IO) :-
do_open_text(FileName, "w", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't open output file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(output_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, output, text, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't open output file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
open_append(FileName, Result, !IO) :-
do_open_text(FileName, "a", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't append to file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(output_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, append, text, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't append to file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
open_binary_input(FileName, Result, !IO) :-
do_open_binary(FileName, "rb", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't open input file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(binary_input_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, input, binary, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't open input file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
open_binary_output(FileName, Result, !IO) :-
do_open_binary(FileName, "wb", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't open output file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(binary_output_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, output, binary, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't open output file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
open_binary_append(FileName, Result, !IO) :-
do_open_binary(FileName, "ab", Result0, OpenCount, NewStream, !IO),
- ( if Result0 = -1 then
- make_err_msg("can't append to file: ", Msg, !IO),
- Result = error(io_error(Msg))
- else
+ ( if Result0 = 0 then
Result = ok(binary_output_stream(NewStream)),
insert_stream_info(NewStream,
stream(OpenCount, append, binary, file(FileName)), !IO)
+ else if Result0 = -2 then
+ Result = error(io_error("can't open directory as file"))
+ else
+ make_err_msg("can't append to file: ", Msg, !IO),
+ Result = error(io_error(Msg))
).
%---------------------------------------------------------------------------%
@@ -5907,7 +5919,7 @@ MercuryFilePtr mercury_current_binary_input(void);
MercuryFilePtr mercury_current_binary_output(void);
int mercury_next_stream_id(void);
MercuryFilePtr mercury_open(const char *filename, const char *openmode,
- MR_AllocSiteInfoPtr alloc_id);
+ MR_bool *is_dir, MR_AllocSiteInfoPtr alloc_id);
void mercury_io_error(MercuryFilePtr mf, const char *format, ...);
void mercury_output_error(MercuryFilePtr mf, int errnum);
void mercury_print_string(MercuryFilePtr mf, const char *s);
@@ -7157,10 +7169,13 @@ mercury_set_current_binary_output(Stream) ->
MercuryFilePtr
mercury_open(const char *filename, const char *openmode,
- MR_AllocSiteInfoPtr alloc_id)
+ MR_bool *is_dir, MR_AllocSiteInfoPtr alloc_id)
{
MercuryFilePtr mf;
FILE *f;
+#ifndef MR_WIN32
+ struct stat stat_info;
+#endif
#ifdef MR_WIN32
f = _wfopen(ML_utf8_to_wide(filename), ML_utf8_to_wide(openmode));
@@ -7168,9 +7183,28 @@ mercury_open(const char *filename, const char *openmode,
f = fopen(filename, openmode);
#endif
+ *is_dir = MR_FALSE;
if (f == NULL) {
return NULL;
}
+#if defined(MR_HAVE_FSTAT) && \
+ (defined(MR_HAVE_FILENO) || defined(fileno)) && defined(S_ISDIR)
+ /*
+ ** AFAIK this is only useful on Linux and FreeBSD. So if we don't have
+ ** fstat or fileno then fall back to just opening the file without this
+ ** check.
+ */
+ if (0 != fstat(fileno(f), &stat_info)) {
+ fclose(f);
+ return NULL;
+ }
+ if (S_ISDIR(stat_info.st_mode)) {
+ *is_dir = MR_TRUE;
+ fclose(f);
+ return NULL;
+ }
+#endif
+
MR_incr_hp_type_msg(mf, MercuryFile, alloc_id, ""MercuryFile"");
MR_mercuryfile_init(f, 1, mf);
return mf;
@@ -9593,10 +9627,17 @@ set_binary_output_stream(binary_output_stream(NewStream),
%
% Attempts to open a file in the specified mode.
% The Mode is a string suitable for passing to fopen().
- % Result is 0 for success, -1 for failure.
+ % Result is
+ % 0 -- success,
+ % -1 -- general failure,
+ % -2 -- failed because the file is a directory (C backends on some OSs)
% StreamId is a unique integer identifying the open.
% Both StreamId and Stream are valid only if Result == 0.
%
+ % The -2 result is separate because some OSs (Linux and FreeBSD) will
+ % willingly open a directory, and FreeBSD will allow you to read from it.
+ % This could cause confusion so we use fstat to detect the problem.
+ %
:- pred do_open_binary(string::in, string::in, int::out, int::out,
stream::out, io::di, io::uo) is det.
@@ -9609,12 +9650,14 @@ set_binary_output_stream(binary_output_stream(NewStream),
[will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
does_not_affect_liveness, no_sharing],
"
- Stream = mercury_open(FileName, Mode, MR_ALLOC_ID);
+ MR_bool is_dir;
+
+ Stream = mercury_open(FileName, Mode, &is_dir, MR_ALLOC_ID);
if (Stream != NULL) {
ResultCode = 0;
StreamId = mercury_next_stream_id();
} else {
- ResultCode = -1;
+ ResultCode = (MR_TRUE == is_dir) ? -2 : -1;
StreamId = -1;
}
").
@@ -9625,12 +9668,14 @@ set_binary_output_stream(binary_output_stream(NewStream),
[will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
does_not_affect_liveness, no_sharing],
"
- Stream = mercury_open(FileName, Mode, MR_ALLOC_ID);
+ MR_bool is_dir;
+
+ Stream = mercury_open(FileName, Mode, &is_dir, MR_ALLOC_ID);
if (Stream != NULL) {
ResultCode = 0;
StreamId = mercury_next_stream_id();
} else {
- ResultCode = -1;
+ ResultCode = (MR_TRUE == is_dir) ? -2 : -1;
StreamId = -1;
}
").
diff --git a/tests/general/read_dir_regression.exp b/tests/general/read_dir_regression.exp
index e804415..d9ab05c 100644
--- a/tests/general/read_dir_regression.exp
+++ b/tests/general/read_dir_regression.exp
@@ -1 +1 @@
-.: read failed: Is a directory
+open failed: can't open directory as file
diff --git a/tests/general/read_dir_regression.exp2 b/tests/general/read_dir_regression.exp2
index 12a191f..91a812a 100644
--- a/tests/general/read_dir_regression.exp2
+++ b/tests/general/read_dir_regression.exp2
@@ -1 +1 @@
-.: can't open input file: . (Is a directory)
+open failed: can't open input file: . (Is a directory)
diff --git a/tests/general/read_dir_regression.exp3 b/tests/general/read_dir_regression.exp3
index d8d124d..06cc5b2 100644
--- a/tests/general/read_dir_regression.exp3
+++ b/tests/general/read_dir_regression.exp3
@@ -1 +1 @@
-.: can't open input file: Access to the path '.' is denied.
+open failed: can't open input file: Access to the path '.' is denied.
diff --git a/tests/general/read_dir_regression.exp4 b/tests/general/read_dir_regression.exp4
index a46f850..449307c 100644
--- a/tests/general/read_dir_regression.exp4
+++ b/tests/general/read_dir_regression.exp4
@@ -1 +1 @@
-.: can't open input file: illegal operation on a directory
+open failed: can't open input file: illegal operation on a directory
diff --git a/tests/general/read_dir_regression.m b/tests/general/read_dir_regression.m
index b69e0a6..34608bb 100644
--- a/tests/general/read_dir_regression.m
+++ b/tests/general/read_dir_regression.m
@@ -21,10 +21,10 @@ main(!IO) :-
; LineRes = eof,
write_string("eof\n", !IO)
; LineRes = error(Error),
- format(".: %s\n", [s(error_message(Error))], !IO)
+ format("read failed: %s\n", [s(error_message(Error))], !IO)
),
close_input(File, !IO)
; FileRes = error(Error),
- format(".: %s\n", [s(error_message(Error))], !IO)
+ format("open failed: %s\n", [s(error_message(Error))], !IO)
).
--
2.7.4
More information about the reviews
mailing list